Document that Process::command will search the PATH#38018
Document that Process::command will search the PATH#38018bors merged 1 commit intorust-lang:masterfrom
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
I don’t get what “as for execvp“ part is supposed to mean. Why are you including references to POSIX specific functions in documentation of platform agnostic part of documentation?
There was a problem hiding this comment.
Yeah I was wondering whether that would help make the behavior clearer to people or if it was better style not to mention it. I can remove that phrase.
There was a problem hiding this comment.
I would just give a couple of examples - "For example, PATH will be searched for rustc but not bin/rustc".
There was a problem hiding this comment.
That isn't the case on Windows. It uses the child's PATH (badly: #37519) even if the program name contains multiple path components.
Windows and Linux have very different behaviour here.
There was a problem hiding this comment.
Yes, I saw that, what a mess.
The main thing I found missing here, as a reader, is that unqualified names will be searched along the PATH. (That's implied by some examples but I think worth having clear.)
I'd like to also say you can control that by setting the PATH environment, and it seems you can, except for various bugs on Windows. Perhaps this is too complex to get into here? It looks like general Rust style is not to mention bugs or caveats in the API docs?
There was a problem hiding this comment.
So we could leave this as just
If
programis not an absolute path, thePATHwill be searched in an OS-defined way.
we could optionally add
The search path to be used may be controlled by setting the
PATHenvironment variable on theCommand,
but this has some implementation limitations on Windows (see #37519).
There was a problem hiding this comment.
If program is not an absolute path, the PATH will be searched in an OS-defined way.
Something like that seems reasonable. I think the main thing that we need to include is that the behaviour is OS dependant.
There was a problem hiding this comment.
Nit:
... the `Command`'s environment...
|
How about this? |
|
Sounds reasonable to me! Looks like the tidy checks are failing though? |
|
whitespace fixed. |
|
Thanks! Could you also squash the commits? |
|
Done, commits squashed |
|
@bors: r+ |
|
📌 Commit db93677 has been approved by |
Document that Process::command will search the PATH
8 participants
No description provided.