-
-
Notifications
You must be signed in to change notification settings - Fork 937
Move versioning into a docker pre-build step, and out of lemmy. #6246
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
base: main
Are you sure you want to change the base?
Conversation
|
Instead of a file I would use an environment variable. That way its not necessary to make any changes to the source code, and will be easier for external people who want to package Lemmy without being familiar with the code. If the environment variable is set it should use that, and otherwise fallback to git-version. This should also be mentioned in the install from scratch documentation. |
|
Hrm. I think in this case a version file is much better than an env var, because this should be a compile (or really a pre-compile since the version should be filled before that) time step, not a changeable run-time variable. If we wanted it to be changeable, I'd recommend putting it in the Plus with using an env var, it'd have to get read every time, which doesn't make sense for something that's never going to change. If people are building custom lemmy versions, I can def update those from scratch instructions to point to this version file, so that they can write their own. The code changes here are also good because they solve a lot of weird re-compiling issues we're getting from lemmy's runtime trying to read the git version. |
|
Of course I meant a compile time env var using option_env, sorry if that wasnt clear. This way its not necessary to know any file path or include Rust/TS specific code. If the env var is not set it should still fallback git-version, even though we are not using that in our own builds. Its mainly useful for instances that are running their own forks with extra changes. In debug mode it should simply use the version from Cargo.toml like before. |
|
I suppose I'd be fine with an env var (although I really don't think its as explicit and clear as a version file), but I really don't think lemmy should be doing any internal logic trying to figure out which git tag or commit it is (and I also don't think using the version from Cargo.toml is a great idea, since there may be modifications). This should all be outside of lemmy, and all it should be doing is reading a file or env var. Otherwise we will run into recompile issues again, since git is always changing. For people running custom lemmy's, with the current file version, all we have to do is tell people to edit |
crates/utils/src/version.rs
Outdated
| @@ -0,0 +1 @@ | |||
| pub const VERSION: &str = "unknown version"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the release.bash script could just also edit this too, to the current version. Then anyone running non-custom lemmys from scratch would be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: this is done now.
|
I tried messing around with an env version, but it ended up being much worse and more verbose, due to this docker bug of not being able to easily set an env var from the result of a command: https://stackoverflow.com/questions/34911622/dockerfile-set-env-to-result-of-command You basically have to write a file no matter what if you want to persist the result of any command in docker. |
This moves versioning out of the lemmy codebase, and into the docker build, by overwriting a
version.rsfile based on theCI_PIPELINE_EVENTfrom woodpecker.Was also able to get rid of the
git_versiondependency in lemmy.I canceled the below tagged builds, but the links show that the
version.rsfile is being written correctly.Matching lemmy-ui PR