-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
dockerTools.buildImage: add option to use nix output hash as tag #42781
Conversation
I think this could become the default value of |
@nlewo I was thinking about the same, but this would be a breaking change for everyone who currently relies on the default value |
If a breaking API change is not a problem then I’m happy to implement the suggestions from @nlewo. |
I don't think a API breaking change is a problem if it is an improvement and if we mention it in the release changelog. @Profpatsch @zimbatm WDYT about the default tag value? |
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.
👍 for the idea.
I don't mind if we break backward-compatibility so it could happen if tag
is null. That also makes it clearer what happens if tag
is specified, then it will override the default.
@@ -477,6 +479,13 @@ rec { | |||
passthru.buildArgs = args; | |||
passthru.layer = layer; | |||
} '' | |||
${lib.optionalString useOutputHashAsTag '' | |||
outName="$(basename "$out")" | |||
outHash=$(echo "$outName" | cut -c-32) |
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.
I think cut -d - -f 1
would be more resistant to change
Referring to the output hash by string splitting in bash seems kind of wrong. I could have sworn there was a builtin or library function that already does that, but I couldn’t find anything.
I’d prefer that, combined with good function docs. |
I built a pure nix function to do that in the past but it wouldn't work in that case as it depends on the derivation output to calculate the hash. |
Thanks for all the feedback. A nix builtin function would be nice indeed. I’ve experimented a little bit with |
I think it’s the best we can do at the moment, and it shouldn’t be a problem. |
Thanks, merged. |
hmm, maybe tests are failing because they are expecting the tag @GrahamcOfBorg test docker-tools |
Success on aarch64-linux Attempted: tests.docker-tools No partial log is available. |
1 similar comment
Success on aarch64-linux Attempted: tests.docker-tools No partial log is available. |
Failure on x86_64-linux (full log) Attempted: tests.docker-tools Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: tests.docker-tools Partial log (click to expand)
|
Fixed by #43119 |
Ah, I missed querying ofborg. Thanks! |
Motivation for this change
I want the tag of my docker image to only change when its content gets changed. Nix already tracks all the dependencies of the image and represents that in the nix-store path. I would like to re-use that value for the docker image tag.
When setting
useOutputHashAsTag
totrue
the giventag
will be ignored and the extracted hash of$out
will be used instead.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)