Reorganise how MKEXE_VIA_CC is built to make it correct for MSVC#12909
Merged
Conversation
Contributor
|
A changes entry would be nice (in the "internal" category). Thanks! |
Contributor
Author
|
I added the Changes entry in the "Internal/Compiler-libs" section. Maybe it would fit better in the "Build system" section? |
Contributor
Yes, sorry, indeed. |
MSVC cl command-line option `/link` allows to pass options to the linker
using the following scheme:
`cl [cl arguments...] /link [link arguments...]`
In order to accomodate all our supported C compilers, this requires a
reshuffling of how the MKEXE_VIA_CC command line is built so that
`$oc_exe_ldflags` are indeed inserted after the `/link` for cl.
This commit also ensures that `/link` (`$mkexe_via_cc_ldflags_prefix`)
is inserted exactly once on the cl command line (rather than once per
argument, which is how flexlink expects these arguments).
Finally, this commit moves the usage of `mkexe_via_cc_extra_cmd` (which
is used only for the MSVC toolchain, to inject the manifest in the
executable) to the MKEXE_VIA_CC variable instead of hiding it into
`mkexe_via_cc_ldflags` where it doesn't belong.
For non-MSVC ports, this commit:
- removes the `$(OC_EXE_LDFLAGS)` Makefile variable as it is no longer
used,
- and reorders the `${oc_exe_ldflags}` after the `$(CFLAGS)`.
Co-authored-by: Samuel Hym <samuel.hym@rustyne.lautre.net>
Contributor
|
Thanks! |
dra27
added a commit
to dra27/ocaml
that referenced
this pull request
Feb 1, 2024
…ml#12909) MSVC cl command-line option `/link` allows to pass options to the linker using the following scheme: `cl [cl arguments...] /link [link arguments...]` In order to accomodate all our supported C compilers, this requires a reshuffling of how the MKEXE_VIA_CC command line is built so that `$oc_exe_ldflags` are indeed inserted after the `/link` for cl. This commit also ensures that `/link` (`$mkexe_via_cc_ldflags_prefix`) is inserted exactly once on the cl command line (rather than once per argument, which is how flexlink expects these arguments). Finally, this commit moves the usage of `mkexe_via_cc_extra_cmd` (which is used only for the MSVC toolchain, to inject the manifest in the executable) to the MKEXE_VIA_CC variable instead of hiding it into `mkexe_via_cc_ldflags` where it doesn't belong. For non-MSVC ports, this commit: - removes the `$(OC_EXE_LDFLAGS)` Makefile variable as it is no longer used, - and reorders the `${oc_exe_ldflags}` after the `$(CFLAGS)`. Co-authored-by: David Allsopp <david.allsopp@metastack.com> (cherry picked from commit f3f3a9c)
dra27
added a commit
to dra27/ocaml
that referenced
this pull request
Feb 1, 2024
…ml#12909) MSVC cl command-line option `/link` allows to pass options to the linker using the following scheme: `cl [cl arguments...] /link [link arguments...]` In order to accomodate all our supported C compilers, this requires a reshuffling of how the MKEXE_VIA_CC command line is built so that `$oc_exe_ldflags` are indeed inserted after the `/link` for cl. This commit also ensures that `/link` (`$mkexe_via_cc_ldflags_prefix`) is inserted exactly once on the cl command line (rather than once per argument, which is how flexlink expects these arguments). Finally, this commit moves the usage of `mkexe_via_cc_extra_cmd` (which is used only for the MSVC toolchain, to inject the manifest in the executable) to the MKEXE_VIA_CC variable instead of hiding it into `mkexe_via_cc_ldflags` where it doesn't belong. For non-MSVC ports, this commit: - removes the `$(OC_EXE_LDFLAGS)` Makefile variable as it is no longer used, - and reorders the `${oc_exe_ldflags}` after the `$(CFLAGS)`. Co-authored-by: David Allsopp <david.allsopp@metastack.com> (cherry picked from commit f3f3a9c)
dra27
added a commit
to dra27/ocaml
that referenced
this pull request
Feb 13, 2024
…ml#12909) MSVC cl command-line option `/link` allows to pass options to the linker using the following scheme: `cl [cl arguments...] /link [link arguments...]` In order to accomodate all our supported C compilers, this requires a reshuffling of how the MKEXE_VIA_CC command line is built so that `$oc_exe_ldflags` are indeed inserted after the `/link` for cl. This commit also ensures that `/link` (`$mkexe_via_cc_ldflags_prefix`) is inserted exactly once on the cl command line (rather than once per argument, which is how flexlink expects these arguments). Finally, this commit moves the usage of `mkexe_via_cc_extra_cmd` (which is used only for the MSVC toolchain, to inject the manifest in the executable) to the MKEXE_VIA_CC variable instead of hiding it into `mkexe_via_cc_ldflags` where it doesn't belong. For non-MSVC ports, this commit: - removes the `$(OC_EXE_LDFLAGS)` Makefile variable as it is no longer used, - and reorders the `${oc_exe_ldflags}` after the `$(CFLAGS)`. Co-authored-by: David Allsopp <david.allsopp@metastack.com> (cherry picked from commit f3f3a9c)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is joint work with @dra27. It brings changes that are required to restore the MSVC port and should make no difference to other ports.
MSVC cl command-line option
/linkallows to pass options to the linker using the following scheme:In order to accomodate all our supported C compilers, this requires a reshuffling of how the MKEXE_VIA_CC command line is built so that
$oc_exe_ldflagsare indeed inserted after the/linkfor cl. This commit also ensures that/link($mkexe_via_cc_ldflags_prefix) is inserted exactly once on the cl command line (rather than once per argument, which is how flexlink expects these arguments). Finally, this commit moves the usage ofmkexe_via_cc_extra_cmd(which is used only for the MSVC toolchain, to inject the manifest in the executable) to the MKEXE_VIA_CC variable instead of hiding it intomkexe_via_cc_ldflagswhere it doesn't belong.For non-MSVC ports, this commit:
$(OC_EXE_LDFLAGS)Makefile variable as it is no longer used,${oc_exe_ldflags}after the$(CFLAGS),all which are hopefully innocuous enough.