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.

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

Allow disabling buildkit for engines lacking support. #1032

Merged
merged 1 commit into from
Sep 29, 2022
Merged

Allow disabling buildkit for engines lacking support. #1032

merged 1 commit into from
Sep 29, 2022

Conversation

Alexhuszagh
Copy link
Contributor

No description provided.

@Alexhuszagh Alexhuszagh added enhancement A-container-engine Area: container engines labels Sep 27, 2022
@Alexhuszagh Alexhuszagh requested a review from a team as a code owner September 27, 2022 19:01
Alexhuszagh
Alexhuszagh commented Sep 27, 2022
View reviewed changes
@Alexhuszagh
Copy link
Contributor Author

This could also likely be fixed by either using an environment variable (to manually disable) or by checking if $engine buildx --help fails, since we've already checked $engine --version works at this point. This could use a slightly more comprehensive solution.

Emilgardis
Emilgardis approved these changes Sep 27, 2022
View reviewed changes
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

this looks good to me for now

adding more comprehensive checks for automatically doing this could be done in the client/server checks for platform I think, only issue is that the fields are not stable between implementations

@Alexhuszagh
Copy link
Contributor Author

bors r=Emilgardis

bors bot added a commit that referenced this pull request Sep 28, 2022
@bors @Alexhuszagh
Merge #1032
11ab0af 
1032: Allow disabling buildkit for engines lacking support. r=Emilgardis a=Alexhuszagh



Co-authored-by: Alex Huszagh <[email protected]>
@bors
Copy link
Contributor

bors bot commented Sep 28, 2022

Build failed:

@Emilgardis
Copy link
Member

Emilgardis commented Sep 29, 2022 •
edited
Loading

bors retry

possible spurious error on winehq

bors bot added a commit that referenced this pull request Sep 29, 2022
@bors @Alexhuszagh
Merge #1032
0805e1a 
1032: Allow disabling buildkit for engines lacking support. r=Emilgardis a=Alexhuszagh



Co-authored-by: Alex Huszagh <[email protected]>
@bors
Copy link
Contributor

bors bot commented Sep 29, 2022

Build failed:

Emilgardis
Emilgardis requested changes Sep 29, 2022
View reviewed changes
@Alexhuszagh
Copy link
Contributor Author

bors retry

possible spurious error on winehq

It looks like a signing key changed, I'll try to fix this and submit a PR:
https://github.com/cross-rs/cross/actions/runs/3145747924/jobs/5113458177#step:10:1640

@Emilgardis
Copy link
Member

It looks like a signing key changed, I'll try to fix this and submit a PR

sounds good!

The zig error is due to not using buildx anymore (because Engine::has_buildkit always returns false), and thus TARGETPLATFORM is not set correctly

https://github.com/cross-rs/cross/actions/runs/3149116522/jobs/5120408064#step:10:1398

@Alexhuszagh
Copy link
Contributor Author

It looks like a signing key changed, I'll try to fix this and submit a PR

sounds good!

The zig error is due to not using buildx anymore (because Engine::has_buildkit always returns false), and thus TARGETPLATFORM is not set correctly

https://github.com/cross-rs/cross/actions/runs/3149116522/jobs/5120408064#step:10:1398

Ah ok, so I need to fix that logic then.

@Alexhuszagh
Copy link
Contributor Author

This also will be helpful temporarily with nerdctl, since it seems BuildKit and Lima don't work nicely with --platform right now, unless I'm very much so mistaken.

Emilgardis
Emilgardis approved these changes Sep 29, 2022
View reviewed changes
@Alexhuszagh
Allow disabling BuildKit for container engines lacking BuildKit support.
7dbb514 
Setting the environment variable `CROSS_CONTAINER_ENGINE_NO_BUILDKIT=1`
will disable the use of BuildKit when building images (including custom
ones), allowing `cross` to be used with container engines lacking
BuildKit support.
@Alexhuszagh
Copy link
Contributor Author

This needs the bug fixes in #1036 for Wine before merging.

@Alexhuszagh
Copy link
Contributor Author

bors r=Emilgardis

@bors
Copy link
Contributor

bors bot commented Sep 29, 2022

Build succeeded:

@bors bors bot merged commit 5261b6a into cross-rs:main Sep 29, 2022
@Alexhuszagh Alexhuszagh deleted the buildkit branch September 29, 2022 19:42
@Emilgardis Emilgardis added this to the v0.3.0 milestone Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Emilgardis Emilgardis Emilgardis approved these changes

Assignees
No one assigned
Labels
A-container-engine Area: container engines enhancement
Projects
None yet
Milestone
v0.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Alexhuszagh @Emilgardis