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: more occurrences of macos12 #890

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vaeng
Copy link
Contributor

@vaeng vaeng commented Sep 17, 2024

Missed to files. Thanks @glennj

@vaeng Did you miss one? The merge into main is still using v12: https://github.com/exercism/configlet/actions/runs/10903920485/job/30259101645 image

@vaeng
Copy link
Contributor Author

vaeng commented Sep 17, 2024

Something is up with the nim installation on macOS and something else is happening with the windows tests in the nim scripts oO

@ErikSchierboom
Copy link
Member

You should just rerun the Windows test. It's flaky

@vaeng
Copy link
Contributor Author

vaeng commented Sep 17, 2024

You should just rerun the Windows test. It's flaky

Thanks, I will remember that. The MacOS tests are still not happy though

@ee7
Copy link
Member

ee7 commented Sep 17, 2024

Thanks, but I think we don't want the changes in this PR. The change from macos-12 to macos-14 is a change of architecture from x86_64 to arm64, which means:

  • The test workflow fails because Nim doesn't provide prebuilt binaries for arm64 macOS.
  • The configlet release process will either fail when producing a x86_64-macOS binary, or produce an arm64 macOS binary that is labelled as x86_64-macOS.

Furthermore, it is likely that commit 22c4cec broke the build.yml job, which runs at release time (and not at PR time). Changes to that workflow should be tested before being merged. And if we'll build on arm64-macos, we might as well build natively rather than using the cross-compiling machinery.

I think we should close this in favor of #879. That is, we should move to the non-deprecated macOS x86_64 runner first.

@vaeng
Copy link
Contributor Author

vaeng commented Sep 17, 2024

Thanks, but I think we don't want the changes in this PR. The change from macos-12 to macos-14 is a change of architecture from x86_64 to arm64, which means:

* The test workflow fails because Nim doesn't provide prebuilt binaries for arm64 macOS.

* The configlet release process will either fail when producing a x86_64-macOS binary, or produce an arm64 macOS binary that is labelled as x86_64-macOS.

Furthermore, it is likely that commit 22c4cec broke the build.yml job, which runs at release time (and not at PR time). Changes to that workflow should be tested before being merged. And if we'll build on arm64-macos, we might as well build natively rather than using the cross-compiling machinery.

I think we should close this in favor of #879. That is, we should move to the non-deprecated macOS x86_64 runner first.

I did not consider the architecture change in the different os releases, thank you for that pointer. We should probably roll the changes back first and then move on with your suggestion.

@glennj
Copy link
Contributor

glennj commented Sep 17, 2024

My apologies for stepping in without knowing the history and build status of this repo.

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