Skip to content

Comments

Fix for windows and OCaml 5#141

Merged
xavierleroy merged 2 commits intoocaml:masterfrom
hhugo:win-fix
Jul 18, 2023
Merged

Fix for windows and OCaml 5#141
xavierleroy merged 2 commits intoocaml:masterfrom
hhugo:win-fix

Conversation

@hhugo
Copy link
Contributor

@hhugo hhugo commented Jun 28, 2023

exported from #133, this patch was suggested by @dra27 for fixing the build with OCaml 5 on windows.

It'll be working on 4.x because of the CC hack in opam-repository-mingw packages. This works:

index 3aa538a..b8cea1e 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -2,3 +2,4 @@
 * text=auto

 configure text eol=lf
+config.guess text eol=lf
\ No newline at end of file
diff --git a/configure b/configure
index 6e87fe9..3acd7a6 100755
--- a/configure
+++ b/configure
@@ -161,7 +161,7 @@ checklib()
     rm -f tmp.ml tmp.out
     echo "" > tmp.ml
     r=1
-    $ocamlc -custom -ccopt "$ccopt $ldflags $cclib" tmp.ml -cclib -l$1 -o tmp.out >/dev/null 2>/dev/null || r=0
+    $ocamlc -custom -ccopt "$ldflags $cclib" tmp.ml -cclib -l$1 -o tmp.out >/dev/null 2>/dev/null || r=0
     if test ! -x tmp.out; then r=0; fi
     rm -f tmp.out tmp.ml tmp.cmi tmp.cmo
     if test $r -eq 0; then echo "not found"; else echo "found"; fi

The rationale is that we've already tested that $ccopt works when testing for the header (in checkinc). For the Windows ports, we can't pass -O3 -Wall -Wextra part because flexlink doesn't recognise those. I think it's OK just to drop $ccopt completely, although possibly that should only be done for Windows. I'm not able to do a PR for this at the moment, though, I'm afraid

I don't really understand the limitation regarding flexlink and what hack were presents in opam-repository-mingw packages.
I'm counting on @dra27 to answer any question regarding this PR.

Hugo Heuzard added 2 commits June 28, 2023 15:36
Copy link
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me. #132 proposes to skip this linking test if pkg_config is available, but I don't know if it's the case for the Windows ports of OCaml, so it's obviously better if the original linking test works on Windows too.

@xavierleroy xavierleroy merged commit 5a0e433 into ocaml:master Jul 18, 2023
@hhugo hhugo deleted the win-fix branch July 18, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants