Skip to content
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

Fix build arg check #2967

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Fix build arg check #2967

merged 1 commit into from
Feb 4, 2021

Conversation

umohnani8
Copy link
Member

Signed-off-by: Urvashi Mohnani [email protected]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Fix the check on build args to be the length of the map
and not whether the map is nil. The nil check was causing
the cache layer to not be used as it would give a false
result.

How to verify it

Which issue(s) this PR fixes:

Fixes #2966

Special notes for your reviewer:

Does this PR introduce a user-facing change?


Fix the check on build args to be the length of the map
and not whether the map is nil. The nil check was causing
the cache layer to not be used as it would give a false
result.

Signed-off-by: Urvashi Mohnani <[email protected]>
@umohnani8
Copy link
Member Author

Cache is being used now

➜  buildah git:(arg-cache) time ./bin/buildah bud --layers -t test . 
STEP 1: FROM ubi8
Resolved "ubi8" as an alias (/home/umohnani/.config/containers/short-name-aliases.conf)
Getting image source signatures
Copying blob cca21acb641a done  
Copying blob d9e72d058dc5 done  
Copying config 3269c37eae done  
Writing manifest to image destination
Storing signatures
STEP 2: RUN sleep 3
STEP 3: COMMIT test
--> 992958b6748
992958b6748aca166baafb4124b9df269a8ba3216f310edf14e854e1e8d10eae
./bin/buildah bud --layers -t test .  7.79s user 1.93s system 72% cpu 13.387 total
➜  buildah git:(arg-cache) time ./bin/buildah bud --layers -t test1 .
STEP 1: FROM ubi8
STEP 2: RUN sleep 3
--> Using cache 992958b6748aca166baafb4124b9df269a8ba3216f310edf14e854e1e8d10eae
STEP 3: COMMIT test1
--> 992958b6748
992958b6748aca166baafb4124b9df269a8ba3216f310edf14e854e1e8d10eae
./bin/buildah bud --layers -t test1 .  0.15s user 0.10s system 26% cpu 0.957 total

@umohnani8 umohnani8 added the todo-backport-to-1.19 We should do or consider a backport to 1.19. Remove when we either decide not to, or open a PR. label Feb 4, 2021
@umohnani8
Copy link
Member Author

@rhatdan @nalind PTAL

@terceiro
Copy link
Contributor

terceiro commented Feb 4, 2021

confirming that this patchfixes it for me. Thanks!

@nalind
Copy link
Member

nalind commented Feb 4, 2021

LGTM

@rhatdan
Copy link
Member

rhatdan commented Feb 4, 2021

/lgtm
/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rhatdan, umohnani8
To complete the pull request process, please assign after the PR has been reviewed.
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@umohnani8
Copy link
Member Author

@rhatdan the docker build conformance failures look like the file permission issue you were talking about today, how do we plan on handling this?

@rhatdan
Copy link
Member

rhatdan commented Feb 4, 2021

I am going to ignore them for now. And force merge.

@rhatdan rhatdan merged commit 1040f10 into containers:master Feb 4, 2021
@TomSweeneyRedHat
Copy link
Member

This looks like it should be cherry picked into release-1.19 too.

@umohnani8 umohnani8 mentioned this pull request Feb 4, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm locked - please file new issue/PR todo-backport-to-1.19 We should do or consider a backport to 1.19. Remove when we either decide not to, or open a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman 3.0rc2: podman build does not use the cache
6 participants