Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

when building from tarball, no version number #4087

Closed
tessus opened this issue Nov 20, 2023 · 16 comments · Fixed by #4143
Closed

when building from tarball, no version number #4087

tessus opened this issue Nov 20, 2023 · 16 comments · Fixed by #4143
Labels
bug Something isn't working enhancement New feature or request

Comments

@tessus
Copy link
Contributor

tessus commented Nov 20, 2023

Subject of the issue

When I build vw from the tarball, e.g. https://github.com/dani-garcia/vaultwarden/archive/refs/tags/1.30.1.tar.gz, no version is shown in the diagnostics admin page.

build command: cargo build --features sqlite,mysql,postgresql --release

Generate Support String does not work either.

Deployment environment

Nothing happens, when I click on Generate Support String. This also seems to be a regression.

  • vaultwarden version: 1.30.1
  • Install method: built from source

  • Clients used: admin page

  • Reverse proxy and version: irrelevant (but it's Apache/2.4.58)

  • MySQL/MariaDB or PostgreSQL irrelevant (but it's MySQL 8.0.35)

  • Other relevant details:

Steps to reproduce

  1. download tarball
  2. extract
  3. build
  4. run
  5. go to https://vault.example.com/admin/diagnostics

Expected behaviour

Server Installed should show the version.
When clicking on Generate Support String, the support string should be generated.

Actual behaviour

Server Installed is empty.
No support string is generated.

Troubleshooting data

image
@BlackDex
Copy link
Collaborator

All is based upon the git repo metadata locally.
If that is not available it doesn't set this.

All tools, CI/CD, podman and docker bake script use the same git commands to extract this information.

If you fully build your self without any git metadata, i suggest to set VW_VERSION during the build. This will see that env variable and uses that as version number.

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

I can certainly do that. It's just that I haven't changed anything on my side.

I wrote a script that either builds from the tarball or the git repo at hash XXXXX. And until not too long ago, a version number was shown without having to set VW_VERSION at build time.

But I'll try to set VW_VERSION. But does this have anything to do with the generate support string? Anyway, I'll try and report back.

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

Hmm, there is something wrong even when building from git....

/--------------------------------------------------------------------\
|                        Starting Vaultwarden                        |
|                      Version 1.30.0-48836501                       |
|--------------------------------------------------------------------|
| This is an *unofficial* Bitwarden implementation, DO NOT use the   |
| official channels to report bugs/features, regardless of client.   |
| Send usage/configuration questions or feature requests to:         |
|   https://github.com/dani-garcia/vaultwarden/discussions or        |
|   https://vaultwarden.discourse.group/                             |
| Report suspected bugs/issues in the software itself at:            |
|   https://github.com/dani-garcia/vaultwarden/issues/new            |
\--------------------------------------------------------------------/
image

@BlackDex
Copy link
Collaborator

Can you see if an older version tarball still works? And if so provide which works and which doesn't?
Could be Github changed something?

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

I can try later today, but as mentioned in my previous comment, there is even an issue when building from git.
I usually build from git and thus this is more important and I will try to narrow that down.

@BlackDex
Copy link
Collaborator

Then i would still like to know at which point this failed, since nothing has changed in regards to this between 1.30.0 and 1.30.1.
Also, 1.30.0-48836501 is the current testing tagged version, and is correct. And it is also exactly the same as the 1.30.1 latest tagged version.

It would help if you can share the script/tooling you use to download/build this. Since all tooling i checked seems to work just fine. I even tested a git clone --depth=1, and that works fine too.

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

I was able to narrow down the incorrect version info when building from git.

So... If I built version 1.30.0 (at git main at that point) and then do a git pull to retrieve the latest version (at the time of this writing 1.30.1), cargo build does not seem to update all info. Which is weird, because the hash is correct, but the version is wrong. I have to do a cargo clean before the build and then all is good.
Before cargo clean: 1.30.0-48836501
After cargo clean: 1.30.1

If there is a tag, no git hash is shown in the version output.

So I am not entirely sure how it comes to 1.30.0-48836501 at all, because the version is generated as follows: use tag, iff it exists. otherwise use tag-current_git_hash
And at git hash 48836501 a tag 1.30.1 exists.

My build script either

  • downloads the tarball
  • does a git pull, git checkout <githash>

and then the cargo build. I never set any env vars or anything.

@dani-garcia
Copy link
Owner

We might need to invalidate the build script output by using https://doc.rust-lang.org/cargo/reference/build-scripts.html#rerun-if-changed and pointing it to some files in the .git directory?

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

btw, how do I retrieve the version via the API?

@BlackDex
Copy link
Collaborator

See

match get_json_api::<GitRelease>("https://api.github.com/repos/dani-garcia/vaultwarden/releases/latest")

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

no, I mean from the binary (vaultwarden server). isn't there a config endpoint or version? I can't finf it though?

./vaulwarden -v works but I want to see what is returned by the admin page. and this is done via an API, is it not?

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

Then i would still like to know at which point this failed

I tried 1.29.0, but also ran into the same issue. So I am not sure when that worked. Or I changed something in my build process, which I however do not recall. I certainly never set VW_VERSION. Still a mystery to me, but I am not going to waste any more time on this.

My solution is to only build from git moving forward. In case I will ever have to use the tarball, I'll remember to set VW_VERSION.

We might need to invalidate the build script output and pointing it to some files in the .git directory

I would suggest to point it to .git/HEAD

Shall I close this issue or do you want to keep it around for reference?

@BlackDex
Copy link
Collaborator

Let's keep it open for the fix to adjust build.rs

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

I just read up on rerun-if-changed. By default, cargo should rebuild build.rs every time something changes in the source tree. Which is the case when I do a git pull and the source tree changes from release A to release B.
So either there is a bug in cargo or the documentation is wrong.

But according to the documentation using rerun-if-changed will only restrict the recompilation to the change of a file or dir.

Thus I doubt that setting rerun-if-changed will improve anything. Unless there is a bug in cargo.

@BlackDex
Copy link
Collaborator

Well, there wasn't a change between the latest testing and stable release. Just a tag was created, maybe that is what is the issue here? Did you do a pull with --tags?

@tessus
Copy link
Contributor Author

tessus commented Nov 20, 2023

That's why I mentioned:

If I built version 1.30.0 (at git main at that point) and then do a git pull to retrieve the latest version (at the time of this writing 1.30.1)

There were 4 commits between those 2 versions. Definitely changes to the source tree.

But yea, your idea makes sense, but unfortunately I wasn't at HEAD.

BlackDex added a commit to BlackDex/vaultwarden that referenced this issue Dec 6, 2023
When a checked-out repo switches to a specific tag, and that tag does
not have anything else changed in the files except the tag, it could
happen that the build process doesn't see any changes, while it could be
that the version string needs to be different.

This commit ensures that if some specific paths are changed within the
.git directory, cargo will be triggered to rebuild.

Fixes dani-garcia#4087
@BlackDex BlackDex added bug Something isn't working enhancement New feature or request labels Dec 6, 2023
dani-garcia pushed a commit that referenced this issue Dec 9, 2023
* Fix BWDC when re-run with cleared cache

Using the BWDC with a cleared cache caused invited users to be converted
to accepted users.

The problem was a wrong check for the `restore` function.

Fixes #4114

* Remove useless variable

During some refactoring this seems to be overlooked.
This variable gets filled but isn't used at all afterwards.

Fixes #4105

* Check some `.git` paths to force a rebuild

When a checked-out repo switches to a specific tag, and that tag does
not have anything else changed in the files except the tag, it could
happen that the build process doesn't see any changes, while it could be
that the version string needs to be different.

This commit ensures that if some specific paths are changed within the
.git directory, cargo will be triggered to rebuild.

Fixes #4087

* Do not delete dir on file delete

Previously during a `delete_file` check we also tried to delete the
parent directory and ignored all errors, like not being empty for
example.

Since this function is called `delete_file` and does not mention
anything in regards to a directory i have removed that code and it will
now only delete the file and leave the rest as-is.

If this somehow is still needed or wanted, which i do not think we want,
then we should create a new function.

Fixes #4081

* Fix healthcheck when using an ENV file

If someone is using a `.env` file or configured the `ENV_FILE` variable
to use that as it's configuration, this was missed by the healthcheck.

So, `DOMAIN` and `ROCKET_TLS` were not seen, and not used in these cases.

This commit fixes this by checking for this file and if it exists, then
it will load those variables first.

Fixes #4112

* Add missing route

While there was a function and a derive, this endpoint wasn't part of
the routes. Since Bitwarden does have this endpoint ill add the route
instead of deleting it.

Fixes #4076
Fixes #4144

* Update crates to update the openssl crate

Because of a bug in the openssl-sys crate we pinned the version to an
older version. This issue has been fixed and was released 2 days ago.

This commit updates the openssl crates including others.
This should also fix the issues with building Vaultwarden using newer
versions of LibreSSL.

Fixes #4051
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants