-
Notifications
You must be signed in to change notification settings - Fork 290
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
Display the latest commit hash as version in the master Docker image #1676
Comments
That would be a nice feature, but I'm not sure what's the best approach to do so. It could be done through the CI but it would require that it manipulates the Git tree (i.e. create a commit and push/rebase it automatically), which I'm not a really in favor of. It can probably lead to other issues. Git hooks could be a solution but they aren't really reliable as they can't be enforced. I think I have seen this in other projects, I'll try to take a look at what's being done there. |
Oh, we do it just for the build at work, we don't push the modified file back. It's really simple, the |
yes the Dockerfile is probably the best place to do such a replacement at build time, using a branch name environment variable provided by the CI. or a custom entrypoint script to do it at runtime (may be overkill) |
Look at how RSS-bridge does it. At the bottom of every install is this:
|
What do you call the CI in this context? I don't understand how
This works fine but only works in Docker context.
Thanks, they parse a file in the |
For example, Gitlab CI provides these variables: https://docs.gitlab.com/ee/ci/variables/#list-all-environment-variables Drone also provides these: https://docs.drone.io/pipeline/environment/reference/ As CIs clone the repo locally, and you usually run within this context, you can even run |
true. The issue was about the Docker image but it should preferably work in any context.
Sure but what @ArthurHoaro meant (?), is how to provide the replacement function for people who install from source from the Since installation from source already uses a few In general I think most frequently used tools should be called from the Makefile, because it's relatively portable and can be called from any build/test context. Some CI steps would benefit from being moved to the Makefile (as I realized when working on https://github.com/nodiscc/Shaarli/pull/4/files). The Dockerfile could also leverage makefile targets to help dedu(tri)plicate the current tooling. |
Oh, I misunderstood that. Yes, the make approach could work as it's already in place for the source code install flavor. Git is also there, so the replacement can be made. The same could work with the Docker build, too, if it installs Shaarli the same way from the pre-cloned repo.
|
Yes that's what I meant, sorry if I was not clear. Adding a new make target is a good idea. I'd just add that it should be in a git ignored file. |
…mmit hash - explictly checkout and use . as build context to allow the changed file to be included in the build https://github.com/docker/build-push-action#git-context - fixes shaarli#1676
Proposed fix in #1945 |
…mmit hash - fixes shaarli#1676 - testing was successful using docker run --entrypoint /bin/cat nodiscc/shaarli:latest shaarli/shaarli_version.php (returns <?php /* c4a5ef5 */ ?>)
Relates to: #1651 (comment)
Although the Shaarli version number is displayed in the footer, the
master
Docker image always showsdev
as the version (hence thevdev
in the Material footer):This makes debugging harder as you have no clue which Shaarli source code version you have within the master image.
Please display the current git commit hash in the footer when using the master image. It can give visual aid of the current release on the master branch, and it could also make reporting errors and testing bugfixes easier.
The CI could replace the static
dev
string in this file to the latest commit hash: https://github.com/shaarli/Shaarli/blob/master/shaarli_version.php -> 544bbdaNote: The same version is displayed at
/admin/server
. This page doesn't have a template in the Material theme. kalvn/Shaarli-Material#124Note: The
v
prefix could be deleted from the Material theme, it's also not part of Semver (only the X.X.X). kalvn/Shaarli-Material#125The text was updated successfully, but these errors were encountered: