Skip to content

Always set SYSTEM, ARCH and MODEL#10044

Merged
xavierleroy merged 2 commits intoocaml:trunkfrom
dra27:always-system
Nov 28, 2020
Merged

Always set SYSTEM, ARCH and MODEL#10044
xavierleroy merged 2 commits intoocaml:trunkfrom
dra27:always-system

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Nov 24, 2020

While looking at something else, I encountered a subtle compilation fault in win32unix:

ifeq "$(SYSTEM)" "mingw"

Before 4.08 and autoconf, config/Makefile.mingw always set SYSTEM:

SYSTEM=mingw

so this worked. However, when we switched to autoconf, the build system adopted the usual ARCH=none, MODEL=default, SYSTEM=unknown if you configure without the native compiler.

This PR tentatively stops resetting ARCH, MODEL, and SYSTEM. The only place I could quickly see where these are used instead of the newer NATIVE_COMPILER=true variable in Makefile.config was the checknative target.

This has been wrong for a while, so there's no rush for 4.12

@dra27 dra27 force-pushed the always-system branch 2 times, most recently from ae4b021 to 2fbf4fe Compare November 26, 2020 14:10
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Nov 26, 2020

For this PR, I am certain that we should always report system: even for a bytecode-only build, because that's related to how the C compiler is called, and so forth.

The argument is less clear for arch: and model: but I would put it that it is surprising to be able to see the difference between ocamlc -config where configure had --enable-native-compiler and ocamlc -config on the same system where configure had --disable-native-compiler.

Copy link
Copy Markdown
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Besides, it only makes a difference with the --disable-native-compiler configure option, which is rarely used as far as I can imagine.

@xavierleroy xavierleroy merged commit 42efb99 into ocaml:trunk Nov 28, 2020
@xavierleroy
Copy link
Copy Markdown
Contributor

I'll let @dra27 and @Octachron decide whether to cherry-pick to 4.12.

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Nov 30, 2020

Thanks, @xavierleroy! It receives occasional use by me (which is how I discovered it) since I think we still don't have a way to tell ocamltest to do the old BYTECODE_ONLY=true in the testsuite, so I end up using this out of laziness.

Just for the proverbial record, we do ship a +bytecode-only switch to opam, and apparently it's used enough that a bug in the 4.12 experiment with having new package options was noticed (and fixed).

Entirely up to @Octachron where 4.12 is concerned.

@Octachron
Copy link
Copy Markdown
Member

A configuration fix seems fine for 4.12 .

dra27 added a commit that referenced this pull request Dec 4, 2020
Even if --disable-native-compiler is given.

(cherry picked from commit 42efb99)
@xavierleroy
Copy link
Copy Markdown
Contributor

If this is cherry-picked to 4.12, #10074 must be cherry-picked also.

@xavierleroy
Copy link
Copy Markdown
Contributor

xavierleroy commented Dec 18, 2022

The argument is less clear for arch: and model: but I would put it that it is surprising to be able to see the difference between ocamlc -config where configure had --enable-native-compiler and ocamlc -config on the same system where configure had --disable-native-compiler.

On second though, the "surprising" behavior was the correct one, given that arch = "none" was the traditional way to test whether ocamlopt is supported or not, see ocaml/opam-repository#22689 .

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Dec 18, 2022

Just for the proverbial record, though, the test has never been a (portably) reliable test for ocamlopt: the Windows config/Makefile.* used to set all three regardless:

ARCH=i386
ARCH64=false
### Name of architecture model for the native-code compiler.
MODEL=default
### Name of operating system family for the native-code compiler.
SYSTEM=mingw

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Dec 18, 2022

Actually, that argument is a bit specious for those manual Makefiles (if one were building a bytecode-only Windows compiler, one should have changed ARCH to none, of course). What isn't (and is more relevant now) is that num only selects the optimised versions of its C implementations (which are still used in the bytecode stubs) if ARCH is set. As things stand, we get num breaking noisily on the bytecode-only arches in 5.0, where for at least ppc64 (and i386/arm32 FWIW) they would otherwise have silently switched to the generic implementations 🤷

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.

3 participants