-
Notifications
You must be signed in to change notification settings - Fork 20
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
Use osx-arm64 packages on Apple silicon #379
base: master
Are you sure you want to change the base?
Conversation
53a96c5
to
b48c516
Compare
Previously the osx-64 packages were used because osx-arm64 was not supported. Rosetta 2 is no longer required for the conda runner on Apple silicon.
b48c516
to
e274a7c
Compare
@@ -610,9 +605,10 @@ def package_distribution(channel: str, package: str, version: str = None, label: | |||
|
|||
if (system, machine) == ("Linux", "x86_64"): | |||
subdir = "linux-64" | |||
elif (system, machine) in {("Darwin", "x86_64"), ("Darwin", "arm64")}: | |||
# Use the x86 arch even on arm (https://docs.nextstrain.org/en/latest/reference/faq.html#why-intel-miniconda-installer-on-apple-silicon) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this doc page go away too? (Later, non-blocking)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I haven't tested the changes, but trust you have.
Oh, wait. How does this interact with |
Oh good thinking, I'll check after lunch. |
First, I installed an earlier version so that
Then I ran All this to say, this is not a breaking change as long as we continue to provide both
Full output of `nextstrain update conda`
|
An edge case here is that |
We're gonna have to remember for a long while into the future that not all Conda runtimes on macOS will be arm64, if they've been long-lived and simply updated continually. I think it'd be simpler to reason about if we explicitly switched setups over from |
Good points. How about a deprecation message like this? 76543e8
The same code that detects "variant" of a Conda runtime setup (by inspecting installed packages, maybe there is a better way) can also be used to keep checking Rosetta 2 for |
Co-authored-by: Thomas Sibley <[email protected]>
9330a6e
to
3704648
Compare
For a deprecation message, that's good. But I'm wondering why we would "stop supporting Conda runtimes created before version X.X.X" (would it be merely to remove the subdir detection and conditional Rosetta 2 check?) and why we'd ask the user to recreate the runtime instead of doing it automatically on update? My thinking was we could add code to |
That makes sense, I'll look into updating this PR with that behavior. |
Description of proposed changes
Previously the osx-64 packages were used because osx-arm64 was not supported. Rosetta 2 is no longer required for the conda runner on Apple silicon.
Checklist
nextstrain setup --force conda
on an M1 Mac runs successfully, pulling fromosx-arm64
channelsCI / test-source
are unrelated.