Fix Issue 20765 - Can't run processes with relative paths when specif…

[CyberShadow]

…ying a working directory

Remove the dubious check which attempted to duplicate the operating
system's behavior.

[dlang-bot]

Thanks for your pull request, @CyberShadow!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	20765	normal	Can't run processes with relative paths when specifying a working directory

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#7455"

[n8sh]

Remove the dubious check which attempted to duplicate the operating system's behavior.

Maybe the motivation was producing a better error message, or someone saw value in bailing out early when it can be determined early on that the process will fail (which is optimizing for a case that I assume is uncommon). The check could be modified to be:

    if (any!isDirSeparator(name))
    {	
        if (workDir.length == 0 && !isExecutable(name))	
            throw new ProcessException(text("Not an executable file: ", name));	
    }

Or it could be removed entirely as you did.

[CyberShadow]

It is dangerous to make assumptions such as those made by the removed code, because things such as ACLs, virtual filesystems, and not-quite-POSIX systems may cause the result to differ from what is expected. Generally in such cases it is more correct to try the action and handle the outcome, rather than attempt to predict it.

[CyberShadow]

It looks like the test for this fix uncovered an additional problem on FreeBSD.

[CyberShadow]

I can't figure out why it's failing on FreeBSD.

I tried the following C program:

#include <stdio.h>
#include <fcntl.h>
#include <unistd.h>

int main()
{
	int fd = open("dir", O_RDONLY);
	fchdir(fd);
	static const char *argv[] = {"./test.sh", 0};
	execve(argv[0], argv, NULL);
}

But, it works as expected.

@jmdavis perhaps might know?

[kyllingstad]

Remove the dubious check which attempted to duplicate the operating system's behavior.

Maybe the motivation was producing a better error message, or someone saw value in bailing out early when it can be determined early on that the process will fail (which is optimizing for a case that I assume is uncommon).

It was so the error could be handled in the parent, before the fork(). It predates the current pipe-based error transporting mechanism, which wasn't in there from the start. It can safely be removed now, and I agree that it should.

[CyberShadow]

Fixed the FreeBSD failure. The cause was embarrassingly mundane - I used #!/bin/bash in the test script, and bash isn't part of FreeBSD.

[schveiguy]

LGTM. Let the OS handle the error sounds good.