Ignore compatible --disable options in configure#10962
Conversation
- Ensure NO_NAKED_POINTERS is always defined as 1 in m.h - Propagate NAKED_POINTERS=false throughout the build and remove all uses of the variable - Remove the naked_pointers ocamltest action and the tests which relied on it
Passing --disable-spacetime, --disable-naked-pointers, --disable-naked-pointers-checker, --disable-vmthreads, --disable-graph-lib and --disable-bigarray-lib to configure merely make explicit the default for 5.00 and should not cause an error.
|
Thank you for the cleanup work! I fully agree with the first two commits. I'm neutral about the third commit (ignoring configure options that are now the default). Why should OCaml users keep passing these options? |
|
It's not so much OCaml users, as some of its developers! I cannot remember exactly what it was, but I have certainly done a huge bisect in the past for something which required So the rationale is that we want to keep the |
|
We need to make progress here. We have two unquestionable commits that are blocked by a third, opinionated commit. Could we get another opinion? Or just merge the first two commits right now. |
Reviewing is hard, but opinions: any day! I would agree with @dra27 that accepting irrelevant options that were relevant in the past is a good choice. (Of course, we should not silently accept options that are incompatible with the current state.) In general I fully trust @dra27's instincts on good UI for configuration across versions, tools, "windows core runtime libraries" and all that horrible stuff. |
|
Thanks, @gasche - I've gone with it as I'm just sorting out another part of FlexDLL for another task, and it'd be useful for that CI to work, too. |
There will never be naked pointers again - for compatibility, we should keep the define for
NO_NAKED_POINTERSinm.handNAKED_POINTERS=falseinMakefile.config.in, but there's no value in any of the rest of the codebase using it, which the first commit cleans.I believe the update to the comment for
Obj.reachable_wordsis correct?Finally, when an option is removed from configuration, we keep the
AC_ARG_ENABLEbecause it is clearer to explain to the user that the option requested no longer exists than to allow autoconf's default behaviour for an unrecognised option. This was done previously with the removal of vmthreads, spacetime and the graphics library.The code used displayed an error for both
--disable-fooand--enable-foo. However, there's no need to display an error for the--disable-fooparameter, since that's just being explicit about the new "default". This probably didn't come up with the previous three because graphics was trivially disabled on Unix by not having the libX11 headers (which are often not there by default anyway), vmthreads was unused and spacetime was off by default and so the--disable-forms were just less commonly used.However, this isn't so true for CI-cache-saving systems which use options like
--disable-bigarray-libto squeeze a few more kilobytes out of freely available cache quotas. This commit tweaks the m4sh so that--disable-spacetime,--disable-naked-pointers,--disable-naked-pointers-checker,--disable-vmthreads,--disable-graph-liband--disable-bigarray-libare now silently ignored.This completes #10831 (comment)