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

feat(remotebuild)!: filter build-plan with --build-for, drop --platform #5216

Merged
merged 8 commits into from
Jan 27, 2025
Merged

feat(remotebuild)!: filter build-plan with --build-for, drop --platform #5216

merged 8 commits into from
Jan 27, 2025

Conversation

mr-cal
Copy link
Collaborator

@mr-cal mr-cal commented Jan 23, 2025 •
edited
Loading

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run tox run -m lint?
  • Have you successfully run tox run -e test-py310? (supported versions: py39, py310, py311, py312)
  • Allows using --build-for to filter entries in the architectures or platforms keys in a project file.
  • Drop the --platform argument. For more information, see this comment.
  • Catches Launchpad errors, which were percolating up as internal errors.

There are still ways for remote builds to fail due to Launchpad's limited processing of the platforms key. That is out of scope here.

The 🎉 passing 🎉 remote spread tests are here.

Fixes #4992
(CRAFT-3277)

@mr-cal mr-cal force-pushed the work/CRAFT-3277-remote-build-filter branch from 5743349 to 43ac71a Compare January 23, 2025 21:55
@mr-cal
feat(remotebuild): filter build-plan with --platform or --build-for
c8345bc 
Allows using '--platform' or '--build-for' to filter entries in the
'architectures' or 'platforms' keys in a project file.

If the project file doesn't contain an 'architectures' or 'platforms'
entry, then '--platform' and '--build-for' define the architectures to
build for.

Signed-off-by: Callahan Kovacs <[email protected]>
@mr-cal mr-cal force-pushed the work/CRAFT-3277-remote-build-filter branch from 43ac71a to c8345bc Compare January 24, 2025 00:15
@mr-cal mr-cal requested review from tigarmo and lengau January 24, 2025 02:09
tigarmo
tigarmo reviewed Jan 24, 2025
View reviewed changes
@mr-cal
docs: clarify behavior
60a5f3e 
Signed-off-by: Callahan Kovacs <[email protected]>
@mr-cal
Copy link
Collaborator Author

mr-cal commented Jan 24, 2025 •
edited
Loading

After discussing with @tigarmo and @lengau, we've agreed to remove --platform from the remote-build command until Launchpad fully supports platforms.

Rationale

Launchpad isn't using craft-platforms and there isn't an API to pick which platforms to build. Even though we have restrictions in place to prevent platforms definitions that are ambiguous to Launchpad ([1] [2]), we aren't comfortable translating --platform to a Launchpad recipe.

--platform presents a complicated UX and prone to corner cases. Finally, we don't want to create an interim solution and then change the behavior again after LP181 is implemented.

The good news is that --build-for will be retained and can be used instead.

@mr-cal
chore: drop --platform
fb3c021 
Signed-off-by: Callahan Kovacs <[email protected]>
@mr-cal
chore: feedback from PR
a96ed9c 
Signed-off-by: Callahan Kovacs <[email protected]>
@mr-cal mr-cal changed the title feat(remotebuild): filter build-plan with --platform or --build-for feat(remotebuild)!: filter build-plan with --build-for, drop --platform Jan 24, 2025
@mr-cal mr-cal requested review from lengau and tigarmo January 24, 2025 19:22
mr-cal added 3 commits January 24, 2025 14:46
@mr-cal
tests: update snap name
ee3ebdd 
Signed-off-by: Callahan Kovacs <[email protected]>
@mr-cal
tests: rename directory
8c1a295 
Signed-off-by: Callahan Kovacs <[email protected]>
@mr-cal
tests: unique hashes for spread tests
eb2e9c8 
Signed-off-by: Callahan Kovacs <[email protected]>
tigarmo
tigarmo approved these changes Jan 27, 2025
View reviewed changes
@mr-cal mr-cal added the squash label Jan 27, 2025
@mr-cal
Copy link
Collaborator Author

mr-cal commented Jan 27, 2025

Spread test failure unrelated, merging

@mr-cal mr-cal merged commit 359a364 into main Jan 27, 2025
11 of 13 checks passed
@mr-cal mr-cal mentioned this pull request Jan 27, 2025
Closed
bepri pushed a commit that referenced this pull request Jan 30, 2025
@mr-cal @bepri
feat(remotebuild)!: filter build-plan with --build-for, drop --platfo…
c3d454d 
…rm (#5216)

* Allows using `--build-for` to filter entries in the `architectures` or
`platforms` keys in a project file.
* Drops the `--platform` argument.
* Catches Launchpad errors, which were percolating up as internal
errors.

Signed-off-by: Callahan Kovacs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tigarmo tigarmo tigarmo approved these changes

@lengau lengau lengau approved these changes

Assignees
No one assigned
Labels
squash
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

remote build: filter build plan with --build-for, drop --platform
3 participants
@mr-cal @tigarmo @lengau