Skip to content

build: allow using cache explicitly with --squash-all using --layers #14320

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 27, 2022
Merged

build: allow using cache explicitly with --squash-all using --layers #14320

merged 3 commits into from
May 27, 2022
+842 −180

Conversation

@flouthoc
Copy link
Collaborator

@flouthoc flouthoc commented May 23, 2022 •
edited
Loading

Buildah already supports using --layers with --squash after containers/buildah#3674
if user wants to do so hence podman must honor similar configuration
in --squash-all behaviour if user wants to using cache.

PS: We cannot alter behaviour of podman build --squash for
docker-compat reasons hence this feature can be easily supported by
--squash-all.

Closes: containers/buildah#4011

Does this PR introduce a user-facing change?

* Adds support for build which allows using build-cache with --squash-all using --layers
* Adds support for new buildah feature --build-context where users can specify additional build context
* Adds support --cpp-flag in podman build which allows additional flags to pass to the C Preprocessor cpp with Containerfile.in

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 23, 2022

@flouthoc: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 23, 2022
@flouthoc flouthoc force-pushed the build-honor-squash-and-layers branch 2 times, most recently from 436d96d to c2c9b87 Compare May 23, 2022 11:49
@rhatdan
Copy link
Member

rhatdan commented May 23, 2022

LGTM but tests are not happy.

@mheon
Copy link
Member

mheon commented May 23, 2022

LGTM once tests go green

@flouthoc
Copy link
Collaborator Author

flouthoc commented May 23, 2022

Stumbled upon a regression following PR at buildah should fix this also added early test at buildah end to prevent this in future: containers/buildah#4013

@flouthoc flouthoc mentioned this pull request May 23, 2022
Closed
@flouthoc flouthoc force-pushed the build-honor-squash-and-layers branch 4 times, most recently from be60a9e to 07b7fd0 Compare May 25, 2022 09:38
@flouthoc
cmd, build: remove redundant squash processing logic
b730e73 
Same block contains similar lines above this is not needed as this looks
redundant.

[NO NEW TESTS NEEDED]
[NO TESTS NEEDED]

Signed-off-by: Aditya R <[email protected]>
@flouthoc flouthoc force-pushed the build-honor-squash-and-layers branch 2 times, most recently from 217e598 to 204510e Compare May 26, 2022 07:00
@flouthoc flouthoc removed the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label May 26, 2022
@flouthoc
Copy link
Collaborator Author

flouthoc commented May 26, 2022

@containers/podman-maintainers PTAL

flouthoc
flouthoc commented May 26, 2022
View reviewed changes
test/buildah-bud/apply-podman-deltas Outdated
Copy link
Collaborator Author

@flouthoc flouthoc May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'll open a followup PR to enable --build-context on podman-remote since change is big and not relevent to this PR and buildah vendor commit.

@flouthoc flouthoc requested a review from rhatdan May 26, 2022 10:25
giuseppe
giuseppe approved these changes May 26, 2022
View reviewed changes
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit otherwise LGTM

cmd/podman/images/build.go Outdated
Copy link
Member

@giuseppe giuseppe May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in togather

Copy link
Collaborator Author

@flouthoc flouthoc May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

flouthoc added 2 commits May 26, 2022 21:01
@flouthoc
build: allow using cache explicitly with --squash-all using --layers
6124b51 
Buildah already supports using `--layers` with `--squash` after containers/buildah#3674
if user wants to do so hence podman must honor similar configuration
in `--squash-all` behaviour if user wants to using cache.

PS: We cannot alter behaviour of `podman build --squash` for
docker-compat reasons hence this feature can be easily supported by
`--squash-all`.

Closes: containers/buildah#4011

Signed-off-by: Aditya R <[email protected]>
@flouthoc
vendor: bump buildah to v1.26.1-0.20220524184833-5500333c2e06
66a56ce 
Bump buildah to v1.26.1-0.20220524184833-5500333c2e06

Signed-off-by: Aditya R <[email protected]>
@flouthoc flouthoc force-pushed the build-honor-squash-and-layers branch from 204510e to 66a56ce Compare May 26, 2022 15:32
@flouthoc flouthoc requested a review from giuseppe May 26, 2022 17:57
@flouthoc
Copy link
Collaborator Author

flouthoc commented May 27, 2022

@giuseppe @vrothberg @containers/podman-maintainers PTAL

giuseppe
giuseppe approved these changes May 27, 2022
View reviewed changes
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 27, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, giuseppe

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhatdan
Copy link
Member

rhatdan commented May 27, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2022
@openshift-merge-robot openshift-merge-robot merged commit a72b240 into containers:main May 27, 2022
@flouthoc flouthoc mentioned this pull request Jun 2, 2022
Merged
@flouthoc flouthoc mentioned this pull request Jun 23, 2022
Closed
@edsantiago edsantiago mentioned this pull request Aug 4, 2022
Closed
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@giuseppe giuseppe giuseppe approved these changes

@rhatdan rhatdan Awaiting requested review from rhatdan

Assignees

@rhatdan rhatdan

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Building with "--squash" doesn't have any caching mechanism (again)

5 participants

@flouthoc @rhatdan @mheon @giuseppe @openshift-merge-robot