Something in this is causing -g to be passed to msvc which is displaying a warning and so causing several tests to fail. I’m happy to look into this, @shindere?

Is this definitely what we want? It seems unquestionably correct that
the build of the system respects CFLAGS given at build time and
defaults to the CFLAGS at configure for building OCaml, but it
doesn’t seem like ocamlc -c should have to respect CFLAGS. If it
does, shouldn’t it be in the same way - i.e. shouldn’t ocamlc -c
always append the current value of CFLAGS, defaulting to the value
of CFLAGS when config.mlp was built if it’s not presently defined?

I am not sure I am following you, @dra27.

As far as I understand and could test, \utils/config.ml` will not
contain any reference to either CFLAGS or CPPFLAGS, so the values that
will be used by ocamlc and ocamlopt are thos that were passed to the
build system either at configure or at build time, the latter taking
precedence. Or am I misunderstanding your point here?

Well, I'm clearly not making my point properly! What I mean is that at configure-time, we store the value of CFLAGS and CPPFLAGS so that we can use them at build time if they're specified a second time. So at build time, you can override the values given at configure-time. I'm querying whether one of:

• ocamlc -c foo.c behaving the same way - if you specify CFLAGS (e.g. CFLAGS="-Wall -Werror" ocamlc -c foo.c) then you should get those CFLAGS and not the ones that config.ml was built with (i.e. in that example you'd get $common_cflags $sharedlib_cflags -Wall -Werror).
• ignoring CFLAGS and CPPFLAGS entirely in OCAMLC_CFLAGS and OCAMLC_CPPFLAGS. All of our C files are compiled directly by calling $(CC), not by calling ocamlc -c, so the whole of our build system respects CFLAGS and CPPFLAGS - do we definitely need to preserve those flags so that they get used for ocamlc -c whose purpose is to invoke the C compiler to allow C stubs to be compiled correctly?

is better than permanently storing what CFLAGS was in OCAMLC_CFLAGS at the time of the build.

@dra27: While going through the extra-checks CI script, I noticed that,
before running the sanitizers, it starts by oding an old-sytle
build that uses the world, opt and opt.opt targets.

It seems it was you who added that build.

May I ask what you wanted to test there?

Do you think there is a real need for this kind of build in addition to
those preformed in the main CI script?

@shindere - the explanation for that starts in #8845

What about MKEXE? At the moment MKEXE_BOOT does include CFLAGS and CPPFLAGS but MKEXE does not - I think it probably should?

It means that the runtime itself is not linked with those set, neither is tools/checkstack or yacc/ocamlyacc or testsuite/tests/output-complete-obj.

That's the only remaining question from me!

check-typo is grumbling about whitespace at EOL in configure.ac. I have one remaining piece of nervousness about MKEXE - normally, it calls a C compiler but for native Windows and shared-enabled Cygwin it calls flexlink. I wonder if CFLAGS and CPPFLAGS should not be included if mkexe is changed to flexlink? I'm musing if the error was in fact that MKEXE_BOOT should not use CFLAGS or CPPFLAGS (especially the latter, given that I don't think you can reliably use MKEXE with .c files). Maybe MKEXE should only be respecting LDFLAGS and never passing CFLAGS or CPPFLAGS because it should never be compiling C sources?

This is really useful on the BSDs, where we often pass custom CFLAGS and CPPFLAGS as part of ports invocations! Just one suggestion: perhaps also pass LDFLAGS though to OC_LDFLAGS.

It's fairly common practise to set both of these at the same time. For example, on OpenBSD, everything is installed in /usr/local by ports, and so:

CFLAGS="-I/usr/local/include" LDFLAGS="-L/usr/local/lib" 

is often used to both find the include files and also let the linker find the shared libraries for a port.

Reading for the nth time the GNU make documentation of CFLAGS, CPPFLAGS and LDFLAGS, LDFLAGS is definitely flags intended for the C compiler to pass on to the linker (so -L, etc.). flexlink should accept anything you'd expect the underlying compiler (either cl or gcc) to accept at this point too, even though flexlink actually calls the linker directly (we recently added -Wl support, for example, and flexlink automatically makes -lfoo portable between cl and gcc). So I think we should:

• Ensure that MKEXE is only ever used as a linker - that means that whatever is called is expected to produce an executable file from .o or .obj files only.
• Only pass $(OC_LDFLAGS) and $(LDFLAGS) at this point

I agree with @avsm that LDFLAGS should be getting the same treatment throughout (having finally arrived at that conclusion in my previous comment!)

I think the same consideration applies to LDLIBS, although for our purposes we can probably have OC_LDFLAGS as we do now including $(LDFLAGS) $(LDLIBS)? (TL;DR from https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html is that -L dir goes in LDFLAGS and -lfoo goes in LDLIBS)

I was wondering if you'd prefer to do that, yes, @shindere! In which case, to leave the tree in a good intermediate state prior to that PR, I think there are two things which should be done:

1. Because of the flexlink issue, I think we should remove OC_CFLAGS, CFLAGS, OC_CPPFLAGS and CPPFLAGS completely from MKEXE and MKEXE_BOOT
2. A follow-on from 1 is that tools/checkstack should call $(MKEXE) having previously built checkstack.o using the normal rule (i.e. it should not be $(MKEXE) ... checkstack.c) as MKEXE would no longer be providing the correct flags.

I think those changes are appropriate here for correct handling of CFLAGS and CPPFLAGS and then we're good to go?

Did you see my earlier question about the old-style build in
extra-checks?

Yes, I replied in #9824 (comment)! It was added in #8845 which includes the explanation - I think it should stay, because it tests that the build system doesn't mess up if the compiler is built in stages (in particular that you don't end up where the build system expects that .opt tools should be available)

Blame the heat! :)

Is that to say that the -link flags we add in front of each link flag
when passing them to flexlink has now become superfluous?

No, for two reasons - the first is that I wouldn't be confident that flexlink is really passing through everything one might want and the second because we then require a minimum of flexlink which we don't yet enforce... we'll get there, when I finally work on improving configures detection of flexdll, but it's not top of my list atm :)

It is sad I didn't think about htis in spite of these many readings of
make's doc.

I get this feeling every time Xavier simplifies my code 😉

lso, @dra27, you didn't comment the making checkstack a configure-time check bit. Is that to say you have no opinion on this?

Oh no, that was an error - I don't have a strong opinion, because I don't fully understand the history of the script/program - it looks like the original intent was "if we can build this test, then rely on it" which sounds more like it should be in the build system. I think it's meant to stop you running a build with settings which are clearly likely to fail (and written at a time nearly 20 years ago where that might have wasted a lot of time). If it moves to configure, would that be in order to run it? That sounds weird for cross-compilation... I wonder if for now it's best to leave it as-is? This PR otherwise I think is good to go - although a round of precheck might be sensible, just to check(stack)?

Sorry to bear bad news, but the "extra-checks" CI is broken. Hint: the -fsanitize=xxx option is needed at link-time too, not just at compile-time.

Darn - can that be fixed by a partial revert of the deletions to extra-checks and use the set_key trick on OC_LDFLAGS? The hack can then be completely removed when configure respects LDFLAGS too? We should all read the GNU make manual yet again to check, but presumably there are scenarios where you expect to add a flag to both CFLAGS and LDFLAGS?

I am pretty sure that CFLAGS must be passed to the C compiler even when linking. There's a number of options that are relevant for code generation and for linking as well, e.g. -g for debug builds.

Quoting the scriptures (GNU make manual):

'CFLAGS'
     Extra flags to give to the C compiler.
'LDFLAGS'
     Extra flags to give to compilers when they are supposed to invoke
     the linker, 'ld', such as '-L'.  Libraries ('-lfoo') should be
     added to the 'LDLIBS' variable instead.

"Extra flags to give to the C compiler" is not "extra flags to give to the C compiler unless it is supposed to invoke the linker".

I think the checkstack part remains fine. I think your suggestion is the way to go for now, @shindere, so that everything gets back to working properly - then we can figure out whether to change that, or to change how flexlink is invoked, as the long-term fix to MKEXE

I'd like to do the same with that bit and am wondering what a good name
for that job would be? I'm thinking about `sequential-build. Can
somebody think about a name that conveys more meaning?

Sequential feels like it implies non-parallel. "Stepwise"?

I referred to it as the "old school" build (idiomatically, it's very similar to old-fashioned, but with more respect and fondness). world.opt was introduced in OCaml 3.05 (which just predates me), but only for the Unix port, so it remained the Windows instruction (with a friendly pointer to RTFM in cc5a09e) until ab9616c in OCaml 4.00!

How about step-by-step-build ?

Sure!