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: re-register dotnet for core22 #4841

Merged
merged 4 commits into from
Jun 10, 2024
Merged

Conversation

mr-cal
Copy link
Collaborator

@mr-cal mr-cal commented Jun 5, 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)

  • re-register dotnet for core22
  • add core22 and core24 dotnet spread tests
  • refactor Application to allow snapcraft to register/unregister plugins per-base
    • this includes renaming _known_core24 to _known_craft_app_base in preparation for core26

Fixes #4825
(CRAFT-2979)

@mr-cal mr-cal requested review from cmatsuoka and tigarmo and removed request for cmatsuoka June 5, 2024 20:41
snapcraft/application.py Outdated Show resolved Hide resolved
snapcraft/application.py Outdated Show resolved Hide resolved
@mr-cal mr-cal marked this pull request as draft June 6, 2024 23:09
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 marked this pull request as ready for review June 7, 2024 15:04
@mr-cal
Copy link
Collaborator Author

mr-cal commented Jun 7, 2024

Force-pushed because I dropped all changes related to running the ClassicFallback or craft-application based codepath.

It's too disruptive of a change for a hotfix.

snapcraft/application.py Show resolved Hide resolved
snapcraft/application.py Show resolved Hide resolved
Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>
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.

We can generalize the calls a bit more instead of being core24-specific in a future refactoring, but this solves the reported issue.

@mr-cal
Copy link
Collaborator Author

mr-cal commented Jun 7, 2024

We can generalize the calls a bit more instead of being core24-specific in a future refactoring, but this solves the reported issue.

Agreed. In a point release, we should flip the paradigm such that the craft-application codepath is used by default and Snapcraft only raises ClassicFallback if we know we should use the core20 or core22 feature set.

@mr-cal mr-cal requested a review from tigarmo June 7, 2024 17:47
@mr-cal mr-cal merged commit f8cf313 into hotfix/8.2 Jun 10, 2024
9 of 10 checks passed
@mr-cal mr-cal mentioned this pull request Jun 13, 2024
4 tasks
sergiusens added a commit that referenced this pull request Jun 14, 2024
This was a clean merge except for a minor conflict between #4841 and
#4732 in `snapcraft.application.Application._get_dispatcher()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants