Fix conf-openblas on macOS arm64 hardware, hopefully without breakages #25196
Conversation
|
(draft because I don't at present have access to my Mac to test it) |
|
I like this suggestion more than requiring |
|
Oh this lint was not there the last time right? If so, nice addition |
|
would you mind to increase the version of "conf-openblas" to 0.2.2? |
|
The lint issue was there last time - although it's at present removed in opam 2.2, because the issue which caused it be a warning has itself disappeared (cf. https://github.com/ocaml/opam/blob/5fa0ac762f4c8e6e2a45272adc6ef1fa4c6dcf28/src/state/opamFileTools.ml#L655-L661) |
|
For both changes, @hannesm, or just for the second env-related change? If for both, presumably the 0.2.1 should be marked as unavailable on macOS arm64 with Homebrew? |
|
I would just move |
|
OK, I don't mind doing it (and will push in a sec!), but it's that bit which feels strange for using a version number bump. It's a packaging error (we're not trying to install an older version of the thing, we're preserving older incorrect packaging instructions for the same thing), but we end up leaving opam-repository where if for any reason a solver on macOS happens to try 0.2.1 (lowerbounds, etc.), then it'll fail. |
pkg-config requires additional configuration after installing the openblas package.
dra27
force-pushed
the
macOS-conf-openblas
branch
from
6cf5424 to
d02bc49
Compare
This has to be done using a separate package since setenv cannot be filtered and opam 2.0.10/2.1.4 and earlier cannot handle a setenv with an empty string in the environment update.
dra27
force-pushed
the
macOS-conf-openblas
branch
from
d02bc49 to
7b242f7
Compare
|
You can make it unavailable for macos on arm64 with homebrew if that is completely uninstallable, I thought it was working but required manually setting the correct PKG_CONFIG_PATH all the time you wanted to install something |
|
As per the version number, I agree that at some point we should set on a more precise policy but bumping the number would follow what we did so far for the other conf- packages when changing the build steps so I think it is better to try to stay consistent |
The package only works if the user has pre-configured the environment - better to switch to 0.2.2 instead, which is hardened.
|
With |
|
I've now tested and verified this on both Intel and ARM Mac hardware |
|
Thanks |
Re-attempt at #25076, following the needed revert of the second part in #25195.
First commit is the same as #25076 - it changes macOS only, introducing a missing dependency (the package already used pkg-config without depending on its conf- package) and amending the call to
pkg-configfor Homebrew only to match Homebrew's recommendation.The second commit addresses the unintended breakage in #25194 (for which apologies again!). There are two issues at play:
setenvdoesn't support filters (ocaml/opam#5787) and empty updates to environment variables crash opam, which is fixed in 2.1.5, but not in earlier releases (ocaml/opam#5350).The fix I propose is to put the
setenvfield in a package of its own, and haveconf-openblasdepend on that package only for macOS with Homebrew. Note thatconf-openblas-macOS-envwill only install ifconf-openblasinstalled, which means that we know that the environment variable update will not be empty, hence there is no need to check for the opam-version itself.