-
Notifications
You must be signed in to change notification settings - Fork 362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix configuration and build system for native Windows #2939
Conversation
1c164d6
to
fd3b0e5
Compare
310d485
to
79e90bd
Compare
OCamlMakefile
Outdated
@@ -112,7 +112,7 @@ endif | |||
|
|||
#################### variables depending on your OCaml-installation | |||
|
|||
SYSTEM := $(shell ocamlc -config 2>/dev/null | grep system | sed 's/system: //') | |||
SYSTEM := $(shell ocamlc -config 2>/dev/null | grep "^system:" | sed 's/system: //') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer a single | sed -n 's/^system: ?//p
pipe (but this is fine)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My way is sloppy - I've changed it!
Thanks! |
Ok to merge once the conflicts are fixed! |
configure can now identify i686-w64-mingw32-gcc. Code included to determine the value for AR from $OCAMLLIB/Makefile.config since OCamlMakefile insists on calling AR directly, rather than using ocamlmklib.
Fixed that pipe, and also 3 others! |
Do let this run through CI, therefore, just in case! |
gcc seems to be getting an option it doesn't like ( |
Test for ocamlc -config fails if your OCaml installation path includes "system"!
ocamldep needs to have the -slash option passed on Windows.
The Microsoft Linker is called link which has the potential to conflict with coreutils link, depending on the order of directories in PATH. By default, coreutils link will win, and compilation is broken on MSVC OCaml. Simply altering /etc/profile to put the Cygwin directories later isn't an option as commands like find and sort then get overridden by Microsoft's "alternative" implementations in system32. Introduce detection in configure so that if multiple link commands are found in PATH, identify the first one which is a Microsoft Linker and alter PATH in Makefile to include its directory first.
Autoconf's default prefix of /usr/local is inappropriate for Windows as it results in C:\usr\local being created by the opam-installer. When Windows is detected, the default is changed to C:/OPAM instead.
Oops - that's why we have CI! This should fix it - I'd put |
Can this one go in too, please? |
Yes, I had to re-run the AppVeyor builds but they seem OK now. |
This PR has some duplication with #2938 over the
$(EXE)
and$(WIN32)
variables.This PR corrects the build system to operate in a preconfigured Cygwin set-up for a native OCaml compiler (at present, autodetection of the Microsoft C Compiler is not merged - as for OCaml, Cygwin must be started from within a Microsoft Developer Tools prompt or PATH, INCLUDE and LIB must be correctly set). This PR fixes various things and when combined with #2938 should allow opam to be built using either mingw or msvc via the lib-ext route (note: the compiled opam will not work). This PR changes the following:
msvs-promote-path
is used to allow the Microsoft Linker (link.exe) to be called even when Cygwin'slink
appears before it inPATH
PREFIX
is selected for Windows-building (I got tired of accidentally finding C:\usr had been created...)