-
Notifications
You must be signed in to change notification settings - Fork 118
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
Provide a way to apply upstream patches to the image #209
Conversation
/assign @hardys |
/assign @dtantsur |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dtantsur, elfosardo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test-integration |
afa4bfa
to
8c0fa2b
Compare
/test-integration |
8c0fa2b
to
09aaf9b
Compare
db3f157
to
7890a45
Compare
/retest |
/test-integration |
7890a45
to
8beb376
Compare
8beb376
to
013f80e
Compare
f4f06da
to
b958aa2
Compare
Dockerfile
Outdated
@@ -35,7 +35,7 @@ ENV PKGS_LIST=main-packages-list.txt | |||
ARG EXTRA_PKGS_LIST | |||
|
|||
COPY ${PKGS_LIST} ${EXTRA_PKGS_LIST} /tmp/ |
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.
A previous version of this PR included "${PATCH_LIST}" in this line, but its now gone, the weird thing is that it still works, this brought me down a bit of a rabbit hole. As far as I can see leaving "EXTRA_PKGS_LIST" blank results in the copy command treating it as a "*", so the entire contents of the local directory get copied into /tmp (including my PATCH_LIST file). So it works but isn't what we would want as the /tmp directory is getting cluttered in the image.
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.
@derekhiggins we saw this behavior in other situations and this is actually a bug in podman, although I can't find the exact reference now
the ${PATCH_LIST} is actually needed here, thanks for catching this
the alternative would be providing placeholders for empy files, but that seems to bring to other issues also related to podman behavior
this was actually working fine until version 1.6.4
This change allows users to apply patches to the images during build time directly from gerrit using the new arg PATCH. At the moment, it's limited to upstream projects with patches on gerrit; future changes will allow different sources, including local directories.
b958aa2
to
022363d
Compare
/lgtm |
/test-integration |
Bug 1998528: Sync latest bugfix code
This change allows users to apply patches to the images during build
time directly from gerrit using the new arg PATCH.
At the moment, it's limited to a single project per image; future
patches will allow applying patches from multiple projects, and
also from local directories.