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

refactor(application): commands, pre-run validation #5007

Merged
merged 4 commits into from
Sep 4, 2024

Conversation

mr-cal
Copy link
Collaborator

@mr-cal mr-cal commented Aug 29, 2024

  • 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)

Refactor

  1. Use craft-cli command groups
  2. Stop overriding craft-applications command_groups property, except for the LifecycleGroup
  3. Split the logic in _get_dispatcher() code into 2 places:
    • Check and raise ClassicFallback errors before creating the dispatcher
    • Check and raise CraftErrors if the wrong codepath or wrong version of snapcraft was chosen after creating the dispatcher

UX changes

  1. ESM errors show a properly formatted error instead of a traceback
  2. Remote build errors about SNAPCRAFT_REMOTE_BUILD_STRATEGY show a properly formatted error instead of a traceback
  3. SNAPCRAFT_REMOTE_BUILD_STRATEGY is only validated when executing snapcraft remote-build
  4. SNAPCRAFT_REMOTE_BUILD_STRATEGY is validated for core24 and newer snaps, rather than being ignored.
  5. snapcraft list-plugins --base=core20 no longer fails in a core24 project directory (snapcraft list-plugins --base=core20 fails in a core24 project directory #5008)

Fixes #4911
Fixes #5008
(CRAFT-3106)

@mr-cal mr-cal force-pushed the work/CRAFT-3106/command-refactor branch from 2c57004 to a4ca44f Compare August 29, 2024 13:46
@mr-cal mr-cal marked this pull request as draft August 29, 2024 14:31
@mr-cal mr-cal force-pushed the work/CRAFT-3106/command-refactor branch from a4ca44f to 5ab9475 Compare August 29, 2024 15:32
snapcraft/application.py Outdated Show resolved Hide resolved
@mr-cal mr-cal force-pushed the work/CRAFT-3106/command-refactor branch 3 times, most recently from 6279969 to 3cb2e1f Compare August 30, 2024 14:02
@mr-cal mr-cal marked this pull request as ready for review August 30, 2024 15:40
@mr-cal mr-cal added the rebase label Aug 30, 2024
snapcraft/application.py Outdated Show resolved Hide resolved
snapcraft/application.py Show resolved Hide resolved
@mr-cal mr-cal requested a review from tigarmo September 3, 2024 16:22
CommandGroups can be used now that craft-application will accept a
sequence of commands.
See canonical/craft-application#359

Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
@mr-cal mr-cal force-pushed the work/CRAFT-3106/command-refactor branch from 48d8d63 to 4403441 Compare September 4, 2024 19:39
Copy link
Contributor

@cmatsuoka cmatsuoka left a comment

Choose a reason for hiding this comment

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

Looks nice, thanks!

@mr-cal mr-cal merged commit 3508cf3 into main Sep 4, 2024
8 of 10 checks passed
@mr-cal mr-cal deleted the work/CRAFT-3106/command-refactor branch September 4, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

snapcraft list-plugins --base=core20 fails in a core24 project directory Refactor command_group usage
3 participants