-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Simplify the range of values used for Config.system
#12405
Conversation
The code is very well shaped as usually and the change does look very
reasonable to me.
Given that `abi` does get kind of an official status, shouldn't its
documentation be made slightly more precise and explain what ABI we are
talking about?
|
I would suggest to not bother with |
These two ports are only supported on Linux.
The various flavours of aarch32 now report system=linux and abi=eabi or abi=eabihf.
The 64-bit ports of OCaml have `Config.system` being freebsd, netbsd or openbsd, as appropriate but the 32-bit Intel ports have bsd for all of them. The x86_32 now reports the same value as for x86_64, which in particular means that BSDs matching *bsd but not {free,net,open}bsd are no longer identified.
Previously the 32-bit version reported linux_elf.
|
As an aside, I spotted that a possible i386 BSD which would have been matched before and isn't here is Debian GNU/kFreeBSD. I was tempted to make the i386 FreeBSD case |
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.
This looks like a useful clean-up and simplification. The amount of breakage in OPAM packages seems manageable. I think it's better to merge now than let this PR bit-rot.
Thank you! |
This fixes coq#18257 following the recommendations given in ocaml/dune#4895. Starting with OCaml 5.2, testing "linux" will be enough thanks to ocaml/ocaml#12405.
As @kit-ty-kate notes in #10613, the values used for
Config.system
(and displayed inocamlopt -config
) are quite inconsistent. At the time (in 2021), that felt a too bit tricky to address, but I've had cause to be looking at this as part of something else and I think two important things have changed:Note that since #10044,
ocamlc -config
displays the detected values forarchitecture
,model
andsystem
even when the native code compiler is not built.This PR proposes:
system=linux
for all Linux variants, in particular:system=elf
(@Octachron - given that s390x is only being restored to full native support in 5.1, might this commit be suitable for 5.1?)linux_elf
linux_eabi
/linux_eabihf
. Theeabi
/eabihf
is now preserved in a newConfig.abi
(abi:
inocamlc -config
). NB We don't use this (orSYS_linux_eabi
/SYS_linux_eabihf
) in the runtime, which uses ARM's own C preprocessor defines.system=freebsd
,system=netbsd
orsystem=openbsd
as appropriate; in particular:system=bsd
, butsystem=openbsd
(as aarch64 and x86_64 already did)system=bsd
for everything matching*bsd
(very pedantically, this means that a system matching*bsd
but not matchingfreebsd*|netbsd*|openbsd*
now reportsunknown
forsystem
(andnone
for architecture), but remember that these are bytecode only systems)That implements the suggestions in #10613, apart from the
bsd
and Windows ones. I'd prefer to leave the Windows ones as they are - the biggest inconsistency before was that both Cygwin and Cygwin64 reportedsystem=cygwin
, but the world has made that easy for us by eliminating 32-bit Cygwin completely. Apart from worrying about how many years it would take to remember the changed values, a more practical reason for leaving them be is that they are used in the compiler's codebase, and elsewhere - all the other values I'm changing here affect largely retired systems and the values themselves are not used at all in the compiler's codebase anymore.This then gives us:
system:
architecture:
values)amd64
,i386
amd64
,i386
1amd64
amd64
,arm64
,arm
,i386
amd64
,i386
amd64
,arm64
,arm
,i386
,power
,riscv
,s390x
amd64
,arm64
i386
amd64
amd64
,arm64
,arm
,i386
amd64
,arm64
,arm
,i386
amd64
i386
amd64
none
Footnotes
the x86 version of Cygwin was retired in 2022 ↩