Conversation
address ocaml#12330
spelling
|
I wnated to fix the hygiene pb: But I can't find |
otherlibs/unix/unix.mli
Outdated
| Using {!create_process} or one of the [open_process_*] functions | ||
| instead is recommended. | ||
|
|
||
| The first element of the array [args] will often be [argv[0]] or |
There was a problem hiding this comment.
Using will often be doesn't really make sense for a specification, you are not documenting an optional behavior: programs will break if they are not send the correct number of arguments.
There was a problem hiding this comment.
An alternative wording might be:
On both unix and windows, the convention is that the first element of the array [args] is the name of the executed program.
otherlibs/unix/unix.mli
Outdated
|
|
||
| The first element of the array [args] will often be [argv[0]] or | ||
| [Sys.argv.(0)] in the new process [prog]. This is plateform | ||
| dependant and you should refer to the man page of execv, which for |
There was a problem hiding this comment.
man page are already Unix-only.
otherlibs/unix/unix.mli
Outdated
| The executable file [prog] is searched in the path. | ||
| The new process has the same environment as the current process. *) | ||
| The new process has the same environment as the current process. | ||
| The first element of the array [args] may or may not be required to |
There was a problem hiding this comment.
may or may not be required sounds mostly confusing.
There was a problem hiding this comment.
I was vague because of your comments on the same issue... I will rephrase as you suggest, but do you have a system in mind that does not follow the convention of first args being the process name ?
Maybe what is not clear is what happens when the first args is not the same as the process name.
In fact, my main concern is that if you do
Unix.open_process_args "cmd" [| "-l" |] it will not work as expected while
Unix.open_process_args "cmd" [| ""; "-l" |] probably will (I did not test), because most commands look for option from argv[1] ?
There was a problem hiding this comment.
What I have in mind is that the specification of program name may vary. Typically, the C17 standard reads as
If the value of argc is greater than zero, the string pointed to by argv[0]
represents the program name; argv[0][0] shall be the null character if the
program name is not available from the host environment.
but the represent verb is quite ambiguous.
Famously, the busybox executable mimics the behavior of the program which name in its argv[0].
Overall, we should focus on the important points: we want to warn users that argv[0] has a special meaning.
- rephrase by copying the doc of create_process which was ok to execve and open_process - also clarify process_pid and alike
|
@Octachron : I rephrased the PR and also applied your suggestion of documentation in issue #12329. |
otherlibs/unix/unix.mli
Outdated
| (** [execv prog args] execute the program in file [prog], with | ||
| the arguments [args], and the current process environment. | ||
| the arguments [args], and the current process environment. | ||
| Note that the first argument is by convention the filename of |
There was a problem hiding this comment.
Here, "first argument" feels slightly ambiguous to me: it may refer to prog. I would propose to be very explicit: "the first argument, [args.(0)], "
|
The |
And you need to compile ocaml to have it ! I got it, thanks. |
|
I do not understant why one test in the test suite failed while I only changed comments ? |
It is a known, unrelated issue (see #12345 for a few more details). |
otherlibs/unix/unix.mli
Outdated
| (** [execv prog args] execute the program in file [prog], with the arguments | ||
| [args], and the current process environment. Note that the first | ||
| argument, [args.(0)], is by convention the filename of the program being | ||
| executed, just like [Sys.argv.(0)] These [execv*] functions never return: |
There was a problem hiding this comment.
Missing dot: like [Sys.argv.(0)] These [execv*] ⇒ just like [Sys.argv.(0)]. These [execv*]
| program being executed, just like [Sys.argv.(0)]. | ||
|
|
||
| The pid of the new process is returned immediately; the new process | ||
| executes concurrently with the current process. The standard input and |
There was a problem hiding this comment.
If you are separating paragraphs by a newline, you should do so here too (and in the rest of the paragraph below).
Changes
Outdated
|
|
||
| - #12338: clarification of the documentation of process related function in | ||
| the unix module regarding the first element of args and shell's pid. | ||
| (Christophe Raffalli review by Florian Angeletti) |
Octachron
left a comment
There was a problem hiding this comment.
I agree that documenting those potential pitfalls is worthwhile (and not only for beginners). Thanks !
|
Note that the the documentation needs to be synced, and the Changes entry updated. |
|
Actually the "Resolve Conflict" button in github does a better job that git on my laptop ;-( |
* convention for first argument * process_pid and friend returns the pid of the shell for process function opening a shell.
address #12330