• via Digitalmars-d-bugs (65/65) Sep 01 2015 https://issues.dlang.org/show_bug.cgi?id=15000

via Digitalmars-d-bugs <digitalmars-d-bugs puremagic.com> writes:

https://issues.dlang.org/show_bug.cgi?id=15000

          Issue ID: 15000
           Summary: Chanegs to userShell either need to be reverted or the
                    std.process documentation needs to be updated
           Product: D
           Version: D2
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P1
         Component: phobos
          Assignee: nobody puremagic.com
          Reporter: issues.dlang jmdavisProg.com


longer uses the SHELL variable. This means that userShell no longer gives you
the user's shell - so it's really not doing what its name implies - and its
documentation is incorrect, because it still says that it looks at the SHELL
variable, which in no longer does. Also, spawnShell's documentation (both on
the function itself and at the top of std.process) indicates that it uses the
user's chosen shell and not necessarily /bin/sh. Curiously, executeShell
doesn't say anything about that, but it seems to ultimately call spawnShell, so
it's certainly sharing the same implementation even if the docs aren't clear on
which shell it uses.

So, I see three options:

1. Change userShell back so that it actually returns the user's shell and not
always /bin/sh and then spawnShell/executeShell will use the user's shell,
since they use userShell to determine the shell.

2. Change userShell back so that it actually returns the user's shell and not
always /bin/sh but change spawnShell/executeShell to explicitly use /bin/sh or
create some other, private helper function similar to userShell which returns
the system's shell but not the necessarily the user's shell.

3. Change the documentation on userShell and spawnShell so that it makes it
clear that /bin/sh is always used rather than continuing to claim that SHELL is
used when it's not.

Personally, I find it a bit annoying that /bin/sh is used since it sucks, and
most folks use a much more sane shell (and honestly, if I'm using executeShell
or spawnShell, it's likely just for a program for myself rather than anything
serious, so I don't care about what it would do on other people's computers at
that point), but I can see why it would be considered to just always use
/bin/sh - particularly since it would be very easy to write a program that
worked on your computer but failed on someone else's due to a difference in
shells.

The change to userShell did break one of my programs (the one I used to build
dmd, druntime, and Phobos actually), and someone else in D.Learn ran afoul of
the change:

http://forum.dlang.org/post/ozueqlrtifopffmumbcl forum.dlang.org

So, in that sense, the change is a regression, but it may be for the better.
Regardless, if we do want to keep the change, then the documentation is going
to need to be updated. And honestly, I think that userShell is a horrible name
if it's not actually returning the user's shell, regardless of whether we want
spawnShell or executeShell to use SHELL or not, so I'd be very tempted to argue
that userShell should be changed back even if spawnShell still always uses
/bin/sh, though I'm not sure what the point of userShell is aside from using
spawnShell or executeShell. I'm actually tempted to argue that it probably
never should have been part of the API given its general uselessness, but maybe
it was there so that a program could check which shell spawnShell/executeShell
would use so that the command could be adjusted if need be? I don't know. That
doesn't seem very practical. But I don't see much point in userShell as its
currently implemented. I'm not sure that it's worth deprecating though. In any
case, its current implementation does not match its documentation.

Now, maybe a good solution would be to default to /bin/sh and have some way to
specify the shell, but given the forest of function parameters spawnShell
already has, that's probably not a good idea.

--