check_executable -> is_executable

Return a boolean rather than raising an exception

src/core/opamCommonStubs.c

@@ -13,26 +13,38 @@
 /*                                                                        */
 /**************************************************************************/
 
+/* Needed for the Windows string conversion functions on older OCaml */
+#define CAML_INTERNALS
+
 #include <caml/mlvalues.h>
 #include <caml/alloc.h>
 #include <caml/memory.h>
 #include <caml/signals.h>
-#define CAML_INTERNALS
 #include <caml/osdeps.h>
 #include <caml/unixsupport.h>
+#include <caml/version.h>
+
+#ifndef _WIN32
+
 #include <fcntl.h>
+#include <unistd.h>
 
-#ifdef _WIN32
-#include <io.h>
 #else
-#include <unistd.h>
+
+#include <io.h>
+
+/* mingw-w64 defines R_OK */
+#ifndef R_OK
+#define R_OK 4
+#endif
+
 #endif
 
 #if OCAML_VERSION < 50000
-#define caml_uerror uerror
+#define caml_unix_access unix_access
 #endif
 
-CAMLprim value opam_check_executable(value path)
+CAMLprim value opam_is_executable(value path)
 {
   CAMLparam1(path);
   char_os * p;
@@ -42,15 +54,14 @@ CAMLprim value opam_check_executable(value path)
   p = caml_stat_strdup_to_os(String_val(path));
   caml_enter_blocking_section();
 #ifdef _WIN32
-  ret = _waccess(p, 04);
+  /* No execute bit on Windows */
+  ret = _waccess(p, R_OK);
 #else
   ret = faccessat(AT_FDCWD, p, X_OK, AT_EACCESS);
 #endif
   caml_leave_blocking_section();
   caml_stat_free(p);
-  if (ret == -1)
-    caml_uerror("faccessat", path);
-  CAMLreturn(Val_unit);
+  CAMLreturn(Val_bool(ret == 0));
 }
 
 /* This is done here as it simplifies the dune file */

src/core/opamStd.ml

@@ -1249,15 +1249,11 @@ module OpamSys = struct
   (* OCaml 4.05.0 no longer follows the updated PATH to resolve commands. This
      makes unqualified commands absolute as a workaround. *)
   let resolve_command =
-    let check_perms f =
-      try OpamStubs.check_executable f; true
-      with Unix.Unix_error _ -> false
-    in
     let resolve ?dir env name =
       if not (Filename.is_relative name) then begin
         (* absolute path *)
         if not (Sys.file_exists name) then `Not_found
-        else if not (check_perms name) then `Denied
+        else if not (OpamStubs.is_executable name) then `Denied
         else `Cmd name
       end else if is_external_cmd name then begin
         (* relative path *)
@@ -1266,7 +1262,7 @@ module OpamSys = struct
           | Some d -> Filename.concat d name
         in
         if not (Sys.file_exists cmd) then `Not_found
-        else if not (check_perms cmd) then `Denied
+        else if not (OpamStubs.is_executable cmd) then `Denied
         else `Cmd cmd
       end else
       (* bare command, lookup in PATH *)
@@ -1280,7 +1276,7 @@ module OpamSys = struct
          expected name but not the right permissions are skipped silently.
          Therefore, only two outcomes are possible in that case, [`Cmd ..] or
          [`Not_found]. *)
-      match List.find check_perms possibles with
+      match List.find OpamStubs.is_executable possibles with
       | cmdname -> `Cmd cmdname
       | exception Not_found ->
         if possibles = [] then

src/core/opamStubsTypes.ml

@@ -114,7 +114,7 @@ type win32_version_info = {
     (** Non-fixed string table. First field is a pair of Language and Codepage ID. *)
 }
 
-external check_executable : string -> unit = "opam_check_executable"
+external is_executable : string -> bool = "opam_is_executable"
 (** faccessat on Unix; _waccess on Windows. Checks whether a path is executable
     for the current process. On Unix, unlike Unix.access, this is checked using
     the EUID/EGID rather than RUID/RGID. *)