Only check that flexlink can be executed when building on a native windows machine.

[toots]

I'm in the process of updating https://github.com/ocaml-cross/opam-cross-windows/ to support OCaml 5.1.1 and there are very little changes to configure required now! Pretty exciting!

This one of two changes: flexlink needs to be called as a shell command, not a windows command when cross-compiling.

I took the approach of assuming that the cmd /c prefix needs to be added only when the build host is windows. That seems like a logical decision to me.

[gasche]

(This is for @dra27, @MisterDA)

[dra27]

Am I correct that this is being done in order to ensure that executable is being seen, rather than a shell script? (I was looking in particular at https://github.com/ocaml-cross/opam-cross-windows/tree/main/packages/flexdll-windows/flexdll-windows.0.42/files).

If that is the case, can this be fixed by instead always calling flexlink.exe -where in configure.ac?

(I don't mind the change as-is; I'm just trying to understand it more)

[toots]

Am I correct that this is being done in order to ensure that executable is being seen, rather than a shell script? (I was looking in particular at https://github.com/ocaml-cross/opam-cross-windows/tree/main/packages/flexdll-windows/flexdll-windows.0.42/files).

If that is the case, can this be fixed by instead always calling flexlink.exe -where in configure.ac?

(I don't mind the change as-is; I'm just trying to understand it more)

I can only speak for my use-case. I think calling flexlink.exe would work for us.

[dra27]

If that works, I think it's a little bit cleaner/clearer - perhaps just with a comment that it's done to ensure that it calls an executable and not any wrapper?

[dra27]

(I also can't see that it breaks any native system - Cygwin installs executables with .exe for maximal interop)

[toots]

If that works, I think it's a little bit cleaner/clearer - perhaps just with a comment that it's done to ensure that it calls an executable and not any wrapper?

Ok, program check switched to flexlink.exe, call switched to native call unconditionally.

[dra27]

I had managed to read two lines of the original diff the wrong way around - 38559e1 is not necessary (sorry!). I fully see what you were doing originally - the only issue is that it's not the correct value for $build... when building on native Windows, we're building with Cygwin or MSYS2, so the regex should instead be *-pc-msys|*-pc-cygwin*.

There is a possible alternative: the test itself is trying to be sure that the flexlink command which configure has seen (which, especially in Cygwin, might be a Cygwin-specific shebang script) can be executed by the native-Windows compiler which is about to produced. In theory, we could therefore do this cmd /c test when $build != $host, but I think looking at one of the logs you posted that opam-windows-cross still has to configure OCaml with $host = $target = x86_64-w64-mingw64, so this doesn't (yet) work.

[dra27]

The TL;DR is that I think this should be sufficient:

diff --git a/configure.ac b/configure.ac
index 1016470045..3ff128ff2d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -970,8 +970,11 @@ AS_IF([test x"$supports_shared_libraries" != 'xfalse'], [
     OCAML_TEST_FLEXLINK([$flexlink], [$flexdll_chain],
                         [$internal_cppflags], [$host])

-    AS_CASE([$host],
-      [*-w64-mingw32*|*-pc-windows],
+    # When building on Cygwin/MSYS2, flexlink may be a shell script which
+    # then cannot be executed by ocamlc/ocamlopt. Having located flexlink,
+    # ensure it can be executed from a native Windows process.
+    AS_CASE([$build],
+      [*-pc-msys|*-pc-cygwin*],
         [flexlink_where="$(cmd /c "$flexlink" -where 2>/dev/null)"
         AS_IF([test -z "$flexlink_where"],
           [AC_MSG_ERROR(m4_normalize([$flexlink is not executable from a native

[toots]

The TL;DR is that I think this should be sufficient:

diff --git a/configure.ac b/configure.ac
index 1016470045..3ff128ff2d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -970,8 +970,11 @@ AS_IF([test x"$supports_shared_libraries" != 'xfalse'], [
     OCAML_TEST_FLEXLINK([$flexlink], [$flexdll_chain],
                         [$internal_cppflags], [$host])

-    AS_CASE([$host],
-      [*-w64-mingw32*|*-pc-windows],
+    # When building on Cygwin/MSYS2, flexlink may be a shell script which
+    # then cannot be executed by ocamlc/ocamlopt. Having located flexlink,
+    # ensure it can be executed from a native Windows process.
+    AS_CASE([$build],
+      [*-pc-msys|*-pc-cygwin*],
         [flexlink_where="$(cmd /c "$flexlink" -where 2>/dev/null)"
         AS_IF([test -z "$flexlink_where"],
           [AC_MSG_ERROR(m4_normalize([$flexlink is not executable from a native

Works for me! Changes applied.

[dra27]

Excellent! As ever, thank you for continuing to push these changes up towards us. @shindere - please could you have a quick double-check of this and if you're happy, squash it?

[toots]

Thanks for the careful review!

[shindere]

this all looks very good to me yes, many thanks! I particularly appreciated the comment that has been added to `configure.ac`, which I found very clear. I took the liberty to force-push a squashed version to the PR branch because squashing is easier forme that way than from the web interface, I hope nobody minds but please feel free to let me know, should that be the case.