docs: clarification for Unix.open_process_args_in

[rbjorklin]

I tripped myself up on this as can be seen here. Hoping this small change will help others in the future.

I used the Conventional Commits style, I hope that's okay.

[gasche]

Searching the web suggests that you are far from the only one to make this mistake. See for example this piece of Reason code

  let ic =
    Sys.win32
      // HACK: Not sure why this command doesn't work on Linux / macOS, and vice versa...
      ? Unix.open_process_args_in(
          "node",
          [|"node", "-e", "console.log(process.execPath)"|],
        )
      : Unix.open_process_in("node -e 'console.log(process.execPath)'");

It looks like Windows resolves the executable name passed to open_process_args*, adding to the confusion.

Could you maybe consider adding an example use in the documentation, which would make the recommendations visually obvious? Code blocks in ocamldoc use the {[...]} markup.

.

I wonder if we should consider changing open_process_args* to lookup the executable in the PATH (there is logic in the Unix module to implement this easily). My understanding of the function was that it was introduced to avoid the complexity of argument parsing by the shell, but there is not much in the original PR #1792 about resolution of the executable itself.

An alternative since OCaml 4.10 is to use Sys.command (Filename.quote_command cmd args). If I understand correctly, this lets us pass arguments as an unescaped list, but the cmd command will still be resolved through the shell.

[gasche]

(cc @xavierleroy @nojb)

[nojb]

I wonder if we should consider changing open_process_args* to lookup the executable in the PATH (there is logic in the Unix module to implement this easily). My understanding of the function was that it was introduced to avoid the complexity of argument parsing by the shell, but there is not much in the original PR #1792 about resolution of the executable itself.

Indeed, to summarize my recollection:

• open_process{,_in,_out,_full}: builds a shell command line and executes through shell; as a side-effect, the program is looked up in PATH
• open_process_args{,_in,_out,_full}: executes the program with the given arguments "verbatim". This uses execv on Unix so the program is not looked up in the PATH. However, on Windows (which requires an ad-hoc implementation), the program is looked up in the PATH, see

  ocaml/otherlibs/win32unix/createprocess.c

  Line 98 in 845ac6a

  exefile = caml_search_exe_in_path(wcmd);

So indeed it looks like there is a mismatch between Unix/Windows regarding the args functions. This may have been overlooked when the args functions where introduced because up until that point, only /bin/sh was used as "program" argument to the exec* function, which didn't need to be looked up in PATH.

So to summarize, looks like replacing execv by execvp in the Unix implementation of the args functions shwould be a plausible fix for the present issue.

[xavierleroy]

The proposed documentation is correct. However, the convention on argument arrays (that the first element matches the command name and the subsequent elements are the arguments proper) is consistently used in the Unix library, e.g. in execv, execve, execvp, execvpe, create_process, create_process_env, open_process_args_in, open_process_args_out, open_process_args, and open_process_args_full. So, documenting it for open_process_args_in and not for any other function is problematic.

[xavierleroy]

So to summarize, looks like replacing execv by execvp in the Unix implementation of the args functions shwould be a plausible fix for the present issue.

It is a plausible change, making these functions more usable, with a small backward incompatibility.

However it is not a "fix for the present issue", which also includes a misunderstanding of what an "argument array" is in the Unix world. This is a bit of information that should be put somewhere but not repeated 10 times for the 10 Unix functions that are affected.

[bobot]

And for the misunderstanding of what an "argument array" is in the Unix world, I think the proposed changed is misleading. The first argument doesn't have to match the name of the command ( the best explananation I found), it should be present but can be arbitrary, except if the command use it for modifying its behavior.

[frou]

I created a separate issue for the open_process_args* and PATH aspect. @gasche @nojb

[rbjorklin]

Superseded by #10084.