Thanks for diagnosing the problem, @shindere and sorry that my patch introduced them.

However, it's not the right fix:

• TARGETHEADERPROGRAM should be defined, and it should be being used - the target_ programs are not correct at the moment (my mistake).
• It was intentional to add Makefile as a prerequisite, since Makefile contains the code to construct the programs (echo statements, etc.). The alternative is to put the generation instructions for the headers into a Makefile fragment (e.g. Makefile.headers) and expressly depend on that which would stop it from rebuilding them when another part of Makefile is updated. In general, if you update the build system, you should invalidate most of what you've already built...

• TARGETHEADERPROGRAM should be defined, and it should be being used
• • the target_ programs are not correct at the moment (my mistake).
    Do you mean that these target programs should be built even if the
    TARGETBINDIR stuff is not enabled?

I believe that's correct, yes - or at least that's what the original version was doing. At the moment, I think it's building the target_ header files but using the wrong objects (so that'll have BINDIR instead of TARGET_BINDIR embedded in them).

• It was intentional to add Makefile as a prerequisite, since
• Makefile contains the code to construct the programs (echo
• statements, etc.). The alternative is to put the generation
• instructions for the headers into a Makefile fragment (e.g.
• Makefile.headers) and expressly depend on that which would stop it
• from rebuilding them when another part of Makefile is updated.
  Perhaps that would make more sense, yes.
  In general, if you update the build system, you should invalidate most
  of what you've already built...
  Well but then it would mean that we should add quite a lot of
  dependencies in a lot of places. We started to discuss this with
  @damiendoligez and I think he was not very warm for doing that.

I agree that to do it for everything would be excessive, but it seems worth having it for the few things where the code really is the Makefile (utils/config.ml, etc.)

In general, if you update the build system, you should invalidate most
of what you've already built...

If you update the build system, you need to do make clean and all developers should be aware of that. The alternative is to add Makefile as a prerequisite on absolutely all targets, which I rather dislike.

BTW, why define a TARGETHEADERPROGRAM variable when its value is always going to be target_$(HEADERPROGRAM) or indeed target_header (because we only cross-compile from Unix)? Why not simply use one of these in the Makefile rule?

And at line 227, shouldn't this be $(TARGETHEADERPROGRAM) instead of $(HEADERPROGRAM)?

@shindere:

• TARGETHEADERPROGRAM should be defined, and it should be being used
• • the target_ programs are not correct at the moment (my mistake).
    Do you mean that these target programs should be built even if the
    TARGETBINDIR stuff is not enabled?

I believe that's correct, yes - or at least that's what the original
version was doing. At the moment, I think it's building the target_
header files but using the wrong objects (so that'll have BINDIR
instead of TARGET_BINDIR embedded in them).
I'm sorry, but I'm unclear about how TARGETHEADERPROGRAM should be
defined, then?

I can have a look at this (once I'm done rehearsing)!

• It was intentional to add Makefile as a prerequisite, since
• Makefile contains the code to construct the programs (echo
• statements, etc.). The alternative is to put the generation
• instructions for the headers into a Makefile fragment (e.g.
• Makefile.headers) and expressly depend on that which would stop it
• from rebuilding them when another part of Makefile is updated.
  Perhaps that would make more sense, yes.
  In general, if you update the build system, you should invalidate most
  of what you've already built...
  Well but then it would mean that we should add quite a lot of
  dependencies in a lot of places. We started to discuss this with
  @damiendoligez and I think he was not very warm for doing that.

I agree that to do it for everything would be excessive, but it seems
worth having it for the few things where the code really is the
Makefile (utils/config.ml, etc.)
I think you also added dependencies when the header programs are
compiled from C, in which case the code itself is not really in the
Makefile, right?

Yes - but the recipe is also non-trivial. My original question with this was as to what bug removing that dependency was fixing, and it still appears to be none (it's just a consequence of the faulty pattern rule). I must confess I'm rather weary of having to argue against a change which makes the build system worse, and not at all convinced that having the build system wrong in every place is somehow superior to it being right only in some places. I added that dependency while developing the original PR because I needed it (while changing the recipe), it was correct, and because it meant I didn't have to temporarily break other parts of the build system in order to see that make all was doing the right thing, especially as the whole point was debugging parallel builds. It has also been useful that it was there in another awkward PR I've worked on since (see #8622). If the recipes and C files are moved to a separate Makefile fragment, does this argument go away? The same pattern is, for example, present in https://github.com/ocaml/ocaml/blob/trunk/utils/Makefile in the same way, added by me originally in the root Makefile in #1287.

@damiendoligez:
I think that when I wrote it, I was working on the basis that we would be cross-compiling other things soon! The error I think is twofold - TARGETHEADERPROGRAM at the moment I think is the same as HEADERPROGRAM and the object depended on when building tmptargetheader%exe should be target_$(HEADERPROGRAM)%$(O)

@shindere - Bach got in the way of my writing that comment and posting it, and I see your fix commit! I agree that for now it can safely stay on the assumption that the target and host use the same kind of header program, even though that's likely to change soon (non-trivially - I expect that when cross-compiling correctly lands, it will be normal to want Cygwin or WSL-compiled ocamlc generating native binaries)

AppVeyor is being strange today, which I don't think is related to this PR - but it needs to go through precheck for the code paths to be tested (since it's only Cygwin which builds this way)

(@shindere - is it is possible for me to start a precheck, but also cause you to be emailed the logs for the build, as I know collecting them separately is awkward?)

@shindere - I more meant that CI here (i.e. AppVeyor) doesn't build Cygwin at all, so the build recipe isn't tested either, since the native Windows ports still just duplicate those target header files. However, I tested Cygwin myself, so I'll go ahead and merge this one now.