Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gix-command on Unix can use non-POSIX /bin/sh even if POSIX sh is first in PATH #1869

Open
EliahKagan opened this issue Mar 1, 2025 · 4 comments
Labels
help wanted Extra attention is needed

Comments

@EliahKagan
Copy link
Member

EliahKagan commented Mar 1, 2025

Current behavior 😯

On Unix-like systems, /bin/sh is usually a POSIX-compatible shell, but POSIX does not require that anything at /bin/sh even exist. It requires that sh run a POSIX-compatible shell (so long as PATH has not been changed to a value that prevents this) and that the shell language that interprets commands in system() and popen() calls is POSIX-compatible. But sh need not resolve to /bin/sh.

Because a lot of software assumes /bin/sh is widespread, it usually does exist, even in systems that don't otherwise use traditional locations for executables (#1810). But some Unix-like operating systems have a Bourne-style but substantially POSIX-incompatible sh implementation as /bin/sh. If the system does not also provide a POSIX-compatible sh elsewhere, then there is probably nothing we can or should do about it. More likely on a system used today, though, is that a POSIX-compatible sh does also exist, at another location.

An informative (i.e. official but supplementary and non-binding) section of the POSIX standard on the sh command includes this recommendation:

Applications should note that the standard PATH to the shell cannot be assumed to be either /bin/sh or /usr/bin/sh, and should be determined by interrogation of the PATH returned by getconf PATH , ensuring that the returned pathname is an absolute pathname and not a shell built-in.

For example, to determine the location of the standard sh utility:

command -v sh

On some implementations this might return:

/usr/xpg4/bin/sh

I think, though I am not sure, that in practice today this is mainly important on Solaris and illumos systems.

Solaris

On Solaris 11, /bin/sh is ksh93, which is POSIX-compatible.

In Solaris 10 and earlier, /bin/sh is a legacy Bourne shell implementation, as described in User Environment Feature Changes for Solaris 11.

Solaris 10 remains in extended support until January 2027.

illumos?

For now, I am having trouble finding definitive information about /bin/sh on illumos. It is a family of operating systems, and while I think /bin/sh is the same on all illumos distributions of the same version of illumos, I am not sure about that either. However, OmniOS is an illumos system. On my OmniOS r151050 virtual machine, /bin/sh is POSIX-compatible:

$ ls -l /bin/sh
lrwxrwxrwx   1 root     root           9 Jun 23  2024 /bin/sh -> i86/ksh93

So the issue doesn't affect my OmniOS system. I think there are older supported versions of OmniOS and of other illumos operating systems, and I haven't researched or examined /bin/sh in them.

Expected behavior 🤔

It may be that we cannot change the behavior in a way that produces an overall improvement:

  • We could use sh instead of /bin/sh. This moderately decreases consistency with Git, which I suspect should work this way but does not (described below). This would apply to cases where /bin/sh -c ... is used, not for scripts with shebangs, so there is no problem with using the wrong shell when /bin/sh is actually called for. But having a shell command, such as the value of core.sshCommand, be interpreted differently for the same user on the same system in the same environment between Git and gitoxide seems like it might lead to strange problems.
  • We could use sh when present and fall back to /bin/sh. This has the same effect as just using sh on most systems, including the decreased consistency with Git and increased difficulty to users who want to configure nontrivial shell commands on systems with non-POSIX /bin/sh that are compatible with both Git and gitoxide. It might cover the case where PATH is unset better than just using sh, though.
  • Using /bin/sh first and falling back to sh on a Unix-like system is probably the least worthwhile change, because I don't think any such systems without any /bin/sh are in common use.

We could do any of those just on systems that are known to have non-POSIX /bin/sh. Detecting the system makes things more complicated but probably not excessively so. The tradeoffs still apply on the affected systems, though.

If on some systems downstream builds of Git have a different shell, then that would be a compelling case for covering them separately. Otherwise it may not be worthwhile.

If this is not to be changed, then I think it is worth documenting in gix-command, and in gix_path::env::shell().

(Either a change to the behavior or the documentation seems like it would conflict with #1862 and #1864. I have no objection to such changes being made at any time and I can rebase to solve the conflict. But if I do them, I may wait until after those are done or to include them there.)

Git behavior

The prepare_shell_cmd function in Git calls git_shell_path to decide what shell to use to run commands of the form sh -c ...:

char *git_shell_path(void)
{
#ifndef GIT_WINDOWS_NATIVE
	return xstrdup(SHELL_PATH);
#else
	char *p = locate_in_PATH("sh");
	convert_slashes(p);
	return p;
#endif
}

That is:

  • On Windows, the Windows locate_in_PATH function (which is a macro for mingw_locate_in_PATH in another file, and not the Unix-specific locate_in_PATH function in this same file) is called to do a PATH search, then \ separators are changed to /.
  • On all systems, including all Unix-like systems (including Cygwin), the compile-time SHELL_PATH constant is used. This could in principle be set differently when compiling on different systems. But unless configured differently at compile-time, it is /bin/sh.

Steps to reproduce 🕹

This can be double-checked by examining the above references. It could be useful to show results of a simple experiment on a system that continues to be used in practice, in a default or otherwise common realistic configuration, wherein /bin/sh exists but is not a POSIX-compatible shell and a separate POSIX-compatible sh exists and is present in a bin or sbin directory listed ahead of /bin in the PATH. In particular, that could help verify any examples or details being considered for inclusion in docstrings. I currently have no such system, but I may be able to set one up at some point.

@Byron Byron added the help wanted Extra attention is needed label Mar 11, 2025
@Byron
Copy link
Member

Byron commented Mar 11, 2025

Thanks a lot for the detailed analysis!

To me it feels that even though gitoxide could be happy to inherit the shortcomings of Git as well, now that the time was already spent to allow for being more portable, we might as well use it and act.

What I understand is that /bin/sh shouldn't be hard-coded, and instead a search in PATH should be done to find a POSIX-compatible version of sh. As gitoxide isn't using/supporting compile-time configuration like Git does, packagers wouldn't be configuring the shell on systems where /bin/sh isn't POSIX compatible, so we'd want it to be correct there as well.

Generally, I'd prefer a solution which is simple, while documenting it's shortcomings sufficiently well to help improve it later should an issue be reported.

@EliahKagan
Copy link
Member Author

EliahKagan commented Mar 13, 2025

I'm not sure if this would adequately satisfy the preference for simplicity, but it occurs to me that, when git is available, we can find out what shell it was built to use, by examining the shell-path shown in the output of git version --build-options. I think that, whenever present, this is meaningful on all builds for Unix-like systems:

ek@Kip:~$ git version --build-options
git version 2.48.1
cpu: i686
no commit associated with this build
sizeof-long: 4
sizeof-size_t: 4
shell-path: /bin/sh
libcurl: 7.58.0
zlib: 1.2.11

We can't use that on Windows. For example, this is from git not from the Git for Windows SDK, on a system that has no D: drive:

C:\Users\ek> git version --build-options
git version 2.48.1.windows.1
cpu: x86_64
built from commit: 2bd190bcd280bc95f537c2d532880a5e539b5132
sizeof-long: 4
sizeof-size_t: 8
shell-path: D:/git-sdk-64-build-installers/usr/bin/sh
feature: fsmonitor--daemon
libcurl: 8.12.1
OpenSSL: OpenSSL 3.2.4 11 Feb 2025
zlib: 1.3.1

But I think we also would not want to use it on Windows anyway because, on Windows, we already prefer an sh found as part of Git for Windows itself whenever available (where the preference will be stronger once #1862 is done), which differs in location in different setups of the same build.

Running git version --build-options would be another subprocess invocation. But I think it is only on Windows where creating process is slow enough that try hard to avoid doing it unnecessarily. As detailed above, we would not use this on Windows.

One thing I am not sure about is if all versions of git in practical use have git version --build-options and whether shell-path is always part of its output.

To be safe, if this is to be done, I would want to use this only when cfg!(unix), rather than the slightly broader cfg!(not(windows)), unless there are non-Unix-like non-Windows systems where it is known to be helpful. It might also be a good idea to check to make sure something (that can be executed) is there, so that if the local git is a broken build due to the shell path it is built to use, we avoid unnecessarily breaking to match it.

In most builds of git, the shell-path would just be /bin/sh, which we could then use even if it is not POSIX-compatible in order to behave the same as the git build on that system. If it gave a different path, we would use that. If absent, we would either use the simple command sh, or do a PATH search for sh, use the result if found, and fall back to /bin/sh otherwise.

This all assumes that we want to use the same sh that git uses when git is present. If we don't want to do that--for example, if we want to use a POSIX-compatible sh even if git is present locally and built to use a different sh--then preferring the value of shell-path from git version --build-options would not be the way to go. (In that case, it might be worthwhile to use it as a fallback, though the narrow case where they would help might not justify the complexity.)

@Byron
Copy link
Member

Byron commented Mar 15, 2025

To me it seems that doing that would be much more correct than hardcoding /bin/sh, even though that value is the most common one. On Unix, making another git invocation lazily seems OK to do as well.
Something that might speak against it is that it doesn't seem to solve a problem we are currently having (maybe I am mistaken about that though), so adding this knowledge as comment where /bin/sh is hardcoded might help a fix once that turns out not to be good enough anymore.

@EliahKagan
Copy link
Member Author

EliahKagan commented Mar 15, 2025

Something that might speak against it is that it doesn't seem to solve a problem we are currently having

Yes. I have not tested on any systems where /bin/sh is different from a separate POSIX-compatible sh also available on the system.

Before making a change, other than to documentation or comments, I would first make sure that this can actually occur in a readily arising situation on some actual operating system people use (and that supports Rust to the extent that gitoxide, or at least the gix-path crate, is capable of being used on it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants