Skip to content

Fix parallel build on Cygwin#2267

Merged
shindere merged 1 commit intoocaml:trunkfrom
dra27:merge-headers
Apr 9, 2019
Merged

Fix parallel build on Cygwin#2267
shindere merged 1 commit intoocaml:trunkfrom
dra27:merge-headers

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Feb 26, 2019

The Unix and Cygwin build instructions in stdlib/Makefile are incorrect. They appear to have been written on the assumption that:

foo bar: baz
	command

means that foo and bar are generated by a single command, which is incorrect as that really defines two separate recipies. On Unix, with #! scripts, this just means that the headers are generated more times than necessary but for Cygwin it means that the for loop which was there is called multiple times and unsurprisingly in parallel those calls interfere with each other.

Unusually, the native Windows build instructions were better, so my approach has been to merge them both (there was a TODO note to this effect already)

I have also fixed a note that the build should not be done using flexlink - this has been done by formally promoting MKEXE_BOOT to the entire build system (it means "do MKEXE, but don't use flexlink"). At some point I may come up with a better name, I had simply started this fix until I realised that #2266 was the minimum 4.08 required fix and the lack of parallelism on Cygwin is annoying (at least, possibly uniquely, for me...)

The diff is very awkward to read - I'd recommend simply reviewing my resulting code in stdlib/Makefile as though there were nothing there before.

@dra27
Merge the generation of stdlib/camlheader*
cf4221e 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.
@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Feb 26, 2019

Precheck job 196

@shindere
Copy link
Copy Markdown
Contributor

shindere commented Feb 26, 2019 via email

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Feb 26, 2019

Thanks, @shindere! For the pattern rules comment, I was trying to explain that the more natural-looking:

camlheader%: header%.$(O)
	...

works for camlheaderd and camlheaderi but doesn't work for camlheader - i.e. the % in a pattern must have something to match so I do camlhead%: and know that the pattern will always include er even in camlheader but it involves the ugly $(subst er,,$*) later and the even stranger looking use of the . in .exe in the tmpheader files!

I wasn't planning on dealing with the FIXME now - it's been like that for decades, so it can wait for a bit longer! It will almost certainly be picked up when cross-compilation is being done, because strip is invoked without a prefix, so the tool name should be being picked up in configure (I expect that's also the case elsewhere in the build system, which is another reason I elected to leave it be for now)

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Feb 27, 2019

@shindere - I reworded the comment in stdlib/Makefile which I hope is clearer (my main concern was to ensure that no one is tempted to correct what initially looks like a typo, although anyone doing that would quickly be presented with a failing build!).

I have changed enough (not building flexlink, moving MKEXE_BOOT, etc.) that I don't propose this for 4.08. We could possibly choose to include it in a 4.08.1, just for the convenience of having parallel builds.

@shindere
Copy link
Copy Markdown
Contributor

shindere commented Feb 27, 2019 via email

@trefis
Copy link
Copy Markdown
Contributor

trefis commented Apr 9, 2019

Ping: is this PR ready to be approved or does more work need to happen?

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Apr 9, 2019 •
edited
Loading

Rebased and ready to go as far as I'm concerned, but note (again) that this is not for 4.08, at this stage.

@shindere shindere merged commit 278e5ab into ocaml:trunk Apr 9, 2019
@shindere
Copy link
Copy Markdown
Contributor

shindere commented Apr 18, 2019

This PR has introduced the TARGETHEADERPROGRAM variable which is used in
stdlib/Makefile but, I think, never defined.

Under some circumstances (hashbang script support not available and
stdlib/header.c and stdlib/headernt.c more recent than stdlib/.depend,
this causes the build system of the standard library to be broken.

#8626 has just been submitted to fix this.

@dra27 dra27 mentioned this pull request Feb 24, 2021
Merged
@dra27 dra27 deleted the merge-headers branch July 6, 2021 14:05
dra27 added a commit to dra27/ocaml that referenced this pull request Jul 28, 2024
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
a1ed101 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Aug 4, 2024
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
f095fe8 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Jun 28, 2025
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
d99afa9 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Jun 30, 2025
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
62c58ca 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Jul 2, 2025
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
667f420 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Jul 2, 2025
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
24f3496 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Jul 3, 2025
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
2eebb39 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Jul 3, 2025
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
8c9bce0 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Jul 6, 2025
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
cc1d8fb 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
dra27 added a commit to dra27/ocaml that referenced this pull request Jul 6, 2025
@dra27
Merge the generation of stdlib/camlheader* (ocaml#2267)
2df6287 
Windows and Unix build instructions for the program versions of the
header stubs unified. For Cygwin, this also fixes the parallel build.

(cherry picked from commit 278e5ab)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shindere shindere shindere approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dra27 @shindere @trefis