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

Add arm64 support to .NET snap #17

Merged
merged 3 commits into from
Oct 15, 2024
Merged

Add arm64 support to .NET snap #17

merged 3 commits into from
Oct 15, 2024

Conversation

mateusrodrigues
Copy link
Member

This PR adds arm64 support to the .NET snap.

  • Adds arm64 as a supported architecture in the snapcraft.yaml file and changes the build scripts accordingly.
  • Moves the CI builds to the Canonical self-hosted machines so that arm64 builds are also available.
  • Makes minor change to the Terminal class by exposing the InvocationResult return type to the callers, which helped debug issues running the snap on arm64.

@mateusrodrigues mateusrodrigues requested a review from a team as a code owner October 11, 2024 16:20
@canonical canonical deleted a comment from github-actions bot Oct 11, 2024
@canonical canonical deleted a comment from github-actions bot Oct 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 11, 2024 •
edited
Loading

Test Results (amd64)

73 tests  ±0   73 ✅ ±0   0s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 29cedc1. ± Comparison against base commit aaee004.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 11, 2024 •
edited
Loading

Test Results (arm64)

73 tests  ±0   73 ✅ ±0   0s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 29cedc1. ± Comparison against base commit aaee004.

♻️ This comment has been updated with latest results.

dviererbe
dviererbe approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@dviererbe dviererbe left a comment

Choose a reason for hiding this comment

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

I left some questions and nitpicks. I approve this now, because I am on vacation for the next week.

snap/snapcraft.yaml Outdated Show resolved Hide resolved
snap/snapcraft.yaml Outdated Show resolved Hide resolved
snap/snapcraft.yaml Outdated Show resolved Hide resolved
.github/workflows/build-on-main.yaml Show resolved Hide resolved
snap/snapcraft.yaml Show resolved Hide resolved
src/Dotnet.Installer.Core/Services/Implementations/SnapService.cs Show resolved Hide resolved
.github/workflows/build-on-main.yaml Show resolved Hide resolved
@mateusrodrigues
Copy link
Member Author

@dviererbe thank you for the review. I'll leave this open for a few more days while I run some more tests on arm64 machines, then I'll merge this.

@mateusrodrigues
Copy link
Member Author

Tests complete. Merging this.

@mateusrodrigues mateusrodrigues merged commit 49b4e6a into main Oct 15, 2024
7 checks passed
@mateusrodrigues mateusrodrigues deleted the feat/arm64-enablement branch October 15, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dviererbe dviererbe dviererbe approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@mateusrodrigues @dviererbe