-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Lift the restriction on installing 5.3.0+BER on macos+arm64 #27887
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
base: master
Are you sure you want to change the base?
Conversation
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! This style of constraint was added in #17716 for all versions of OCaml which didn't support Apple silicon (i.e. everything before 4.12.0 apart from 4.10.1).
FWIW the same can be done for 4.14.1+BER:
diff --git a/packages/ocaml-variants/ocaml-variants.4.14.1+BER/opam b/packages/ocaml-variants/ocaml-variants.4.14.1+BER/opam
--- a/packages/ocaml-variants/ocaml-variants.4.14.1+BER/opam
+++ b/packages/ocaml-variants/ocaml-variants.4.14.1+BER/opam
@@ -77,4 +77,4 @@ depopts: [
"ocaml-option-nnp"
"ocaml-option-nnpchecker"
]
-available: !(os = "macos" & arch = "arm64") & os != "win32"
+available: os != "win32"
@@ -76,4 +76,4 @@ depopts: [ | |||
"ocaml-option-nnp" | |||
"ocaml-option-nnpchecker" | |||
] | |||
available: !(os = "macos" & arch = "arm64") & os != "win32" & arch != "arm32" & arch != "x86_32" | |||
available: os != "win32" & arch != "arm32" & arch != "x86_32" |
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.
Orthogonal to this PR, but this:
available: os != "win32" & arch != "arm32" & arch != "x86_32" | |
conflicts: [ | |
"ocaml-option-32bit" | |
"ocaml-option-bytecode-only" | |
] | |
available: os != "win32" |
and removing them from the list above is more correct.
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.
The arm32-
and x86_32-
variants are failing in CI. I wonder if it's due to this change.
Thanks. I've made that change in 20f0163 |
Various changes in ocaml-compiler.5.3.0 which need to be present, in particular the ocaml-option-bytecode-only changes.
I've pushed two additional commits which will hopefully improve the CI. As it's written at the moment, the 4.14.1 and 5.3.0 packages don't support bytecode-only builds (I'm not sure if MetaOCaml actually supports that itself?), so I added the conflict to the 4.14.1 version as well (that happened to be being caught by CI). The second commit brings a few of the changes which are in ocaml-compiler.5.3.0 into the BER version - hopefully the 32-bit buillds will be correctly rejected this time. |
The lint error is incorrect (as for compiler packages in general). The failures on Arch I think can be ignored, although there's trouble on the horizon! I think that's GCC 15. For the 4.14.1 version, it's missing ocaml/ocaml#12577 (from 4.14.2). For the 5.3.0 version, I'm wondering if the BER patches have accidentally backed out ocaml/ocaml#12509 and ocaml/ocaml#11764 (which are the "trunk" parts of the 4.14.2 PR), but I haven't looked in detail. |
The outstanding CI errors: archlinux-ocaml-4.14 IIUC, @dra27 you think we should hold off until these are fixed? (The other option being we mark the package unavailable or in conflict?). |
At a glance, it doesn't look that way: the commit metaocaml/ber-metaocaml@d6b868c (which seems to be the reworking of those PRs in 5.3.0) is on the |
@kayceesrk reports that MetaOCaml 5.3.0+BER can be installed successfully on macos+arm64, so the current constraint is apparently unnecessary.