fix the -runtime-variant option for bytecode#8537
Conversation
6deb0af to
a8b0952
Compare
bytecomp/bytelink.ml
Outdated
| Location.print_filename file | ||
| | Required_module_unavailable s -> | ||
| fprintf ppf "Required module `%s' is unavailable" s | ||
| fprintf ppf "Required module '%s' is unavailable" s |
There was a problem hiding this comment.
Why the elimination of the back-tick here? FWIW, I'd prefer to see other instances of '%s' changed to `%s' rather than this way around?
There was a problem hiding this comment.
@dra27 some people from your faculty may disagree ;-)
There was a problem hiding this comment.
Quite possibly - but let's remove them wholesale, rather than piecemeal with other changes 🙂
There was a problem hiding this comment.
(I'd also prefer them either to be post-processed to the correct Unicode character before going to the console, or for us finally to allow the correct Unicode characters directly in the message, both of which would keep Markus happy!)
|
I have a nagging concern that the flexlink bootstrap relies on the "silent fail" behaviour when the header is missing - I just want to check what's going on there before this is merged (I'm suspicious that in bytecode-only mode, it may result in a corrupt |
|
Just to confirm - this PR will break the flexdll bootstrap build the next time someone commits a bootstrap. Do you mind if I push fixes directly? |
|
I've rebased and added commits to fix the flexdll bootstrap in my fork |
2d7eef4 to
98c598d
Compare
|
@dra27 I've applied all your suggestions and integrated your flexdll fixes (tested by bootstrapping flexdll on mingw64). I've also moved the Change entry to 4.09, as I intend to cherry-pick this patch. |
|
@damiendoligez - squash or merge, and still good for 4.09? |
|
This is a bugfix so it should be good for 4.09. |
… under the wrong names, which made -runtime-variant fail for bytecode.
If OCaml was already installed, camlheader would be used from the installation when building the bytecode flexdll/flexlink.exe
98c598d to
5231ae1
Compare
|
FWIW, I just squashed the Changes bit - the flexdll and other changes are a bit subtle, so seem worth retaining cleanly for bisecting |
|
I'll merge and cherry-pick once CI passes |
fix the -runtime-variant option for bytecode (cherry picked from commit 8153041)
|
Thanks! |
The
-runtime-variantoption has been broken since 4.03.0 because thecamlheaderdandcamlheaderifiles are installed under the wrong names. The first commit fixes this problem.The failure is silent and a bytecode file is produced that has the executable flag but doesn't work because the header is missing. The second commit adds some error handling for this case (in particular when the user specifies a runtime variant that doesn't exist).
Note: the first bug was introduced by myself in commit 0225ca0