-
-
Notifications
You must be signed in to change notification settings - Fork 78
Dockerfiles: reuse build arguments #257
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: master
Are you sure you want to change the base?
Dockerfiles: reuse build arguments #257
Conversation
Signed-off-by: Timo Reichl <[email protected]>
Signed-off-by: Timo Reichl <[email protected]>
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.
Thanks for opening this pull request! Be sure to follow the pull request template!
I am a bot, here are the test results for this PR:
|
I'm not rejecting or ignoring this but it's not something that can just be applied to a single branch of a single base image, it would need to be replicated across the board and that takes a lot of thought and planning. I'll probably look at the options for implementing some or all of this PR as part of the Alpine 3.22 base image branch when that's released in the next month or so. |
@thespad yeah sorry about bluntly posting this PR without a related issue first. I didn't know how to start this kind of conversation the best way. Either way I thought about the other base images as well. I could provide PRs for those as soon as the changes of this specific PR are deemed mergeable. What do you think? |
PRs for other branches and bases will likely come when there's some other reason to touch them as there's not a huge benefit to retrofitting changes like this to older images, but I'll have a better sense of what's needed once 3.22 is up and running. |
I am a bot, here are the test results for this PR:
|
Description:
This PR puts all image configuration and convenience variables at the top-level of the respective Dockerfile (before any stage) as
ARG <key>[=<value>]
instructions. Top-levelARG
s can be imported by following stages viaARG <key>
(i.e., without setting a value).See below for further details.
Benefits of this PR and context:
Putting all "living" build arguments at the top-level makes it more convenient and less error prone to copy/paste and typos when changing values in the future, as those can now be set in one single place only and are imported into all stages which require them.
Take the current master branch for example. This line is defined in the stage
rootfs-stage
:ENV ROOTFS=/root-out
However, a couple lines later,
/root-out
is explicitly stated again:sed -i -e 's/^root::/root:!:/' /root-out/etc/shadow
This PR just defines
ROOTFS
at the top level and uses it throughout the whole file.How Has This Been Tested?
I'll just paste this log of my terminal inputs/outputs here:
building the image from master branch
building the image from branch dockerfiles/reuse-build-arguments
running lscr.io/linuxserver/jenkins-builder:latest
Rinse and repeat for
Dockerfile.aarch64
.Testing environment
Source / References:
N/A