Does gix-command mean for $0 to be --?

[EliahKagan]

When use_shell is true and no optimization keeps a shell from actually being used, the impl From<Prepare> for Command returns a std::process::Command instance that, roughly speaking, will run something like /bin/sh -c 'command "$@"' -- args....

The argument after -c and its operand is --. This has been the case since functionality for passing arguments was added in d8e8b54 (#496).

Because the arguments before -- run a command with a shell, the arguments after -- are arbitrary arguments that are passed to the command or script rather than being treated specially by the shell, and -- is often used to prevent subsequent arguments from being treated as options or otherwise specially, it may look at first glance like -- does that here. Perhaps that was even its intent.

But this -- argument does not do that. Instead, to simplify from the standard, this way of invoking sh is:

sh -c command_string [command_name [argument...]]

Where, as the standard documents for -c:

Read commands from the command_string operand. Set the value of special parameter 0 (see 2.5.2 Special Parameters) from the value of the command_name operand and the positional parameters ($1, $2, and so on) in sequence from the remaining argument operands. No commands shall be read from the standard input.

This is to say that the effect of our passing -- here is to tell the script whose code is passed as the operand to -c that it should consider its own name, i.e. the value of $0, to be --. One consequence of this is that errors produced in the shell itself are prefixed --:. For example, in #1839, the error message is:

--: /Users/runner/work/gitoxide/gitoxide/target/debug/examples/arrow: No such file or directory

It seems to me that giving the script the name of -- may be confusing to people examining stderr of a process run with gix-command, unless they are familiar with implementation details of gix-command (as well as with the syntax of sh -c).

If the goal of using -- rather than some other string was to prevent subsequent commands from being interpreted as shell options, then nothing special has to be done to avoid that, at least for POSIX-compatible shells. Arguments that come after the command string are never interpreted this way, instead always becoming positional parameters like $1, $2, $3.

However, we do have to pass something where we are currently passing --, if arguments meant to become $1, $2, $3, ..., are passed. Otherwise the argument meant to be $1 (and thus the first word expanded from "$@") would become $0, and the other positional parameters would, in effect, be shifted.

I'm not sure what would be better. In one-liners and causal use, I think _ is common. That is probably not better here. We should not use the value of the command field (which becomes the command string, sometimes with "$@" appended), or at least we should not use it unmodified. This is because the command field is sometimes just a path, and having it be the value of $0 for the script would make it hard to distinguish messages produced by the shell from those produced by an external command the shell runs. The ability to distinguish this has already been useful in #1839.

[Byron]

Thanks for bringing this up!

I also ran into the --: <error> messages and was quite puzzled, but didn't think about it further either.

With that said, what I tried to do but somehow failed is to implement what Git does:

		/*
		 * If we have no extra arguments, we do not even need to
		 * bother with the "$@" magic.
		 */
		if (!argv[1])
			strvec_push(out, argv[0]);
		else
			strvec_pushf(out, "%s \"$@\"", argv[0]);
```

It doesn't look like both implementations are very similar as `gix-command` allows passing additional arguments.

I think it's OK to not look at Git here and instead decide to use a more standard "no $0 available" name, and replacing `--` with `_` seems like the way to go.

I'd definitely appreciate a PR for that 🙏.

[EliahKagan]

It doesn't look like both implementations are very similar as gix-command allows passing additional arguments.

I don't follow (or don't agree). Both git and gix-command support passing arbitrarily many arguments through and making them available as the shell command string's positional parameters, so "$@" expands to them. The fragment of the Git source code you've shown is part of the implementation of that functionality. The corresponding gix-command code is currently:

gitoxide/gix-command/src/lib.rs

Lines 288 to 303 in b310c16

	if !prep.args.is_empty() {
	if prep.command.to_str().map_or(true, |cmd| !cmd.contains("$@")) {
	if prep.quote_command {
	if let Ok(command) = gix_path::os_str_into_bstr(&prep.command) {
	prep.command = gix_path::from_bstring(gix_quote::single(command)).into();
	}
	}
	prep.command.push(" \"$@\"");
	} else {
	gix_trace::debug!(
	"Will not add '\"$@\"' to '{:?}' as it seems to contain '$@' already",
	prep.command
	);
	}
	}
	cmd.arg(prep.command);

This code (intentionally) implements some functionality absent in Git. But both pass arguments through so they are usable as shell script positional parameters. In that prepare_shell_cmd function in Git, argv[0] is the command string and argv[1], argv[2], and so forth, if present, are the positional parameters. In a later step after the code you showed, Git does this to pass the rest of the arguments:

strvec_pushv(out, argv);

That runs on all code paths (except when the git process itself is aborted due to detecting a logic error). This includes the code path where a shell is used, to which the code you showed pertains. When a shell is used, an effect of pushing all values in argv is that argv[0] appears twice: the first time due to the code you showed, which passes argv[0] as the command string operand for -c (possibly with "$@" appended), and the second time as the command name.

This is to say that, where gix-command currently uses -- for the command name, Git uses the (original value of the) command string as the command name. Because the functionality involved here is similar between Git and gix-command, we could exhibit the same behavior as Git by passing the original value of the command field.

However, that's the behavior I argued against above, on the grounds that it causes the origin of messages to be unclear, because someone examining stderr cannot tell if a message came from the shell or the external command the shell ran.

It seems to me that this reasoning applies to Git, so that it may be a bug that Git does this, and even if it is not a bug, a different behavior might be an improvement. But Git might reasonably do it if it is somehow expected for backward compatibility, if some important software somehow relies on it, or if it turns out to be the most informative behavior in practice. Without knowing why this was chosen in Git--if it was chosen, since it's a very intuitive way to write the code, even though the effect is misleading--it would be hard to know if there are reasons for gix-command to do it. But even if it turns out to be right for Git to behave this way, I suspect gix-command maybe should not, or not by default.

That is no argument against your concrete recommendation of using _. However, that idea seems to be based on my mention that people often use _ when they use sh -c without caring about the value of $0. If we do care about the value of $0, then we might want to use something more informative. The advantage of _ is that its meaning would be readily clear in the code of gix-command itself (at least compared to --). But I think it has two disadvantages:

1. While _ would rarely clash with a path of an external command being run, it would always clash with itself in other commands run the same way with a shell using gix-command. This is no worse than always using --. But it is a way both are worse than what Git does, and maybe this is the reason Git does what it does.

   Suppose two external commands, abc and xyz, are run though shells. In Git, messages from the abc parent shell look like they come from abc, and messages from the xyz parent shell look like they come from xyz, but they are distinguished from each other. In gix-command, both currently and if -- is changed to _, messages from the parent shells of abc and xyz don't look like they come from abc or xyz, but their origins are not distinguished from each other (and no hint is given that they are related to abc and xyz).

2. While _ avoids the usual connotations of --, the significance of _ would still not be clear to some users reading stderr, at least users who aren't in the habit of intentionally running shells with -c and passing positional arguments through.

Some possible approaches:

• Leave "--" as it is. This has the disadvantages of "_" without the advantage of avoiding the misleading connotations of "--", so it is unlikely to be the best approach long-term.

• Replace "--" with "_" and keep everything the same, per GitoxideLabs/gitoxide#1840 (comment). This does not address (1). It does not fully address (2), but some users will recognize what _ means.

• Replace "--" with the original value of the command field (without "$@" appended). This has the advantage of matching the behavior of Git and of fully addressing (1). But it carries the more severe variant of (2) where messages that come from a shell appear to come from a specific external command that they do not come from (and that, as in #1839, may not exist).

  A possible argument in favor of doing this is that, because Git does it, people might be using $0 in the command to get the command (that itself would usually have to contain $0 literally, in order to be used in this way). But I think that is not very likely.

• Replace "--" with cmd.get_program(), i.e., with the command used to run the shell itself, which was computed by:

  gitoxide/gix-command/src/lib.rs

  Lines 284 to 285 in b310c16

  	prep.shell_program
  	.unwrap_or(if cfg!(windows) { "sh" } else { "/bin/sh" }.into()),

  What makes this attractive is that it is exactly what Bourne-style shells use for $0 when they are invoked with -c with no command name argument following the command string operand. For example:

  $ sh -c 'printf "%s\n" "$0"'
  sh
  $ /bin/sh -c 'printf "%s\n" "$0"'
  /bin/sh
  $ ln -s /bin/sh foo
  $ ./foo -c 'printf "%s\n" "$0"'
  ./foo

  This happens because, when -c command_string is used with no command_name, the shell sets $0 from its own argv[0]. (The shell's own argv should not be confused with the argv of the Git function prepare_shell_cmd discussed above, which becomes "$0" "$@" for the shell but does not affect the shell's own argv[0]. It also does not go the other way; as detailed below, passing a command_name argument to set $0 does not seem ever to cause shells to change their own argv[0], nor to behave as though it had been different.)

  This does not address (1), since the value would only be different across different shells, and the same shell_program is usually going to be used. It fully addresses (2).

  If most messages will be due to the inability to find or execute a file whose name would already appear in the message body itself, then I think this is the way to go. I'm not sure how to assess this. But an example is the motivating case in #1839, where the error message was:

  --: /Users/runner/work/gitoxide/gitoxide/target/debug/examples/arrow: No such file or directory
  

  With this approach, i.e. using cmd.get_program() as $0, the message would be:

  /bin/sh: /Users/runner/work/gitoxide/gitoxide/target/debug/examples/arrow: No such file or directory
  

  So in that kind of error situation, we get clear, correct information that is exactly what we need to understand what went wrong. Thus, this may be the best approach, at least as the default behavior. But I am not sure, because there may be other errors where it's not so clear.

• Replace "--" with a string based on the original value of the command field (without "$@" appended), but modified in some clear and simple way so it is not confused with it, such as by enclosing it in parentheses. That is, if we are running /bin/sh -c abc due to a command value of "abc", then the expression replacing "--" would evaluate to "(abc)".

  This fully addresses (1). It partly addresses (2), in that it would be immediately clear that messages are from something related to the command but not from the command, but not necessarily clear that this is a shell.

  This might be the best balance, at least as the default behavior, though if it is to be pursued, I may want to look into whether there is any other meaning of enclosing a command in ( ) that would mislead users. (That certainly does have other meanings--in ps it can mean the process itself modified its arguments, and in a shell script this notation causes a command to be run in a subshell--but these do not seem to create an opportunity for confusion, and may even make the meaning here more intuitive.)

  This shares one disadvantage also present when just using the original value of the command field (i.e., with doing what Git does): if the command is complex, it may be difficult to tell where the origin-designating prefix in a message ends, and where the body of the message begins. But even this disadvantage should usually be less severe, due to the enclosing ( and ).

  This approach has the further possible disadvantage that, if a command does itself use and parse $0, then it could malfunction if it contains unusual characters. I think such a command would be considered to have a bug in order for this to happen, but the presence of ( ) might exacerbate the effects of such a bug.

  Really, I think the main downside of this is that it is slightly more complicated, and its effects more verbose, than just using cmd.get_program() as described above, and it's not obvious that its advantages over cmd.get_program() are decisive. Any messages this appears in do come from the shell, and in practice most will be errors that are reported due to error checking done by the shell rather than explicit error reporting in the script operand that follows -c. The example of in #1839 shows what I mean--this might not be considered an improvement:

  (/Users/runner/work/gitoxide/gitoxide/target/debug/examples/arrow): /Users/runner/work/gitoxide/gitoxide/target/debug/examples/arrow: No such file or directory
  

• Replace "--" with a string based on the original value of the command field but with the value of cmd.get_program() followed by -c preceding it. That is, usually have $0 when running abc in a shell be something like sh -c abc. This fully addresses both (1) and (2) but it seems to me that it will be harder to interpret if the command is complex, for the reason given in the previous bullet point. It would also be highly ambiguous unless quotes are used, in which case it would potentially be even more confusing.

  Most of the other disadvantages of the (abc) way from the preceding bullet point, such as verbosity when the path also appears in an error message body, also apply here.

Finally, for each of those approaches, there is also the composite approach of doing it by default, but adding a field to gix_command::Prepare to allow the value of $0 to be specified. When None, the default strategy would be used. When Some(name), the value of name would be passed.

The only disadvantage I am aware of that is added by allowing $0 to be customized is that the issue of what the shell is told to use as $0 could be confused with the related issue of what the shell is told to use as its own name. As noted above, the string that the shell is told is its own name (i.e. the shell's own argv[0]) is what Bourne-style shells use as the default value of $0 when invoked with -c command_string with no subsequent non-option arguments (thus no command_name argument). But it doesn't go the other way. When -c command_string is used and a command_name argument is passed to tell a shell what $0 should be, this does not cause the shell to behave as though it was itself invoked with that name. For example, in shells where it is easy to query if they are running as a login shell:

$ mkdir temp
$ cd temp
$ for shell in bash ksh93 mksh lksh zsh posh yash; do ln -s -- "$(command -v "$shell")" "-$shell"; done
$ PATH=".:$PATH"
$ bash -c 'printf "%s\t" "$0"; shopt login_shell'
bash    login_shell     off
$ -bash -c 'printf "%s\t" "$0"; shopt login_shell'
-bash   login_shell     on
$ bash -c 'printf "%s\t" "$0"; shopt login_shell' -bash
-bash   login_shell     off
$ ksh93 -c 'printf "%s\t%s\n" "$0" "$-"'
ksh93   chsB
$ -ksh93 -c 'printf "%s\t%s\n" "$0" "$-"'
-ksh93  chsBl
$ ksh93 -c 'printf "%s\t%s\n" "$0" "$-"' -ksh93
-ksh93  chB
$ mksh -c 'printf "%s\t%s\n" "$0" "$-"'
mksh    hc
$ -mksh -c 'printf "%s\t%s\n" "$0" "$-"'
-mksh   lhc
$ mksh -c 'printf "%s\t%s\n" "$0" "$-"' -mksh
-mksh   hc
$ lksh -c 'printf "%s\t%s\n" "$0" "$-"'
lksh    hc
$ -lksh -c 'printf "%s\t%s\n" "$0" "$-"'
-lksh   lhc
$ lksh -c 'printf "%s\t%s\n" "$0" "$-"' -lksh
-lksh   hc
$ zsh -c 'printf "%s\t%s\n" "$0" "$-"'
zsh     569X
$ -zsh -c 'printf "%s\t%s\n" "$0" "$-"'
-zsh    569Xl
$ zsh -c 'printf "%s\t%s\n" "$0" "$-"' -zsh
-zsh    569X
$ posh -c 'printf "%s\t%s\n" "$0" "$-"'
posh    c
$ -posh -c 'printf "%s\t%s\n" "$0" "$-"'
-posh   cl
$ posh -c 'printf "%s\t%s\n" "$0" "$-"' -posh
-posh   c
$ yash -c 'printf "%s\t%s\n" "$0" "$-"'
yash    c
$ -yash -c 'printf "%s\t%s\n" "$0" "$-"'
-yash   cl
$ yash -c 'printf "%s\t%s\n" "$0" "$-"' -yash
-yash   c

In that experiment, the way I caused shells' own argv[0] to begin with -, to make them login shells, was by actually invoking them by executing a file (specifically, a symlink) named that way. But that I did it that way was only for convenience. Although argv[0] for a process is usually populated with a relative or absolute path that was used to find the executable, which if absolute is usually the same as the path used to open the executable, this need not be the case. It is specified independently in most of the standard exec-family functions, which is how the initial login shell of a session is started with - prepended to its name (not by executing a file whose name starts with -).

Furthermore, specifying a custom argv[0] as a shell's own name is something an application developer may be interested in doing. A developer may wish to prepend - to use a login shell. (As discussed in #1752 and #1758, running commands with a login shell is not usually the right thing to do, but may occasionally be the right choice.) Or they may want to run a Bourne-style shell not called sh in POSIX compatibility mode by passing sh as its argv[0], or may want to run a combined executable like busybox as a shell by invoking it "as" sh. There are probably other motivations that make sense.

I say that not to argue that gix-command should gain support for this--I am not sure, and I think it's mostly a separate question from what is being considered here. (On Unix-like systems, where it is most useful, it can be done easily via CommandExt::arg0.) Rather, I think that a developer who wants to do that might confuse a feature to set $0 to a custom value with such functionality, and also erroneously believe that they had set the shell's argv[0] by inspecting $0 (which, when not set in any other way, is the shell's argv[0]) and finding that it has their custom value.

Refraining from adding a new field to Prepare that overrides $0 would avoid this risk. But I think this risk can be overcome, by giving the new field a sufficiently clear name, such as shell_display_name. (The distinction can also be explicated in at least one documentation comment--between the field and its related methods, if any--and referenced by others as needed.) This seems like a good idea to me. But I am not certain, so I have laid out the risk as I see it, so the details can be considered.

[Byron]

Thanks for the clarification! strvec_pushv(out, argv); is key here and it's what I didn't see.

Additionally, I agree that gix-command doesn't have to do use the command as script name just to repeat what Git does in this case, and instead stick to something more conventional.

Thanks to your exhaustive analysis I also see and agree that using the shell path itself seems like what shells do, while solving a lot of problems.

This seems like something that could easily be implemented, is it something you would be interested in?

Regarding making shell_display_name() configurable, since it seems to also affect shells as they'd act like login shells if their name is prefixed with -, I feel like waiting for demand for such a feature is sufficient (to have a reason to spend the time to flesh it out).

[EliahKagan]

Additionally, I agree that gix-command doesn't have to do use the command as script name just to repeat what Git does in this case, and instead stick to something more conventional.

I am not sure which approaches are more conventional. The approach of using the name used to invoke the shell matches what the shell does when the only argument after -c is its command operand.

Thanks to your exhaustive analysis I also see and agree that using the shell path itself seems like what shells do, while solving a lot of problems.

It solves the -- problem I encountered while investigating #1839, in the form I encountered it there. One thing I didn't realize when examining that is that the user experience that is relevant here varies across different shells. That was on macOS, and thus using major version 3. In some (versions of) some shells, the line number is included. Most of the time, though perhaps not always, the command operand to -c will be exactly one line, so this will say line 1.

Here's an example of what an error of this form--though with a different cause--currently looks like when it occurs on Windows (where the bash version, via Git Bash, is similar to what one usually has on a GNU/Linux system, though please note that not all GNU/Linux systems use bash as sh, since Debian and its derivatives including Ubuntu use dash except in very old versions):

--: line 1: testsfixturesgenerated-do-not-editscripts2549957747-windowstrivial: command not found

Under the proposed change, that would probably become:

sh: line 1: testsfixturesgenerated-do-not-editscripts2549957747-windowstrivial: command not found

This seems like an improvement to me, still. In particular, no one is going to think sh itself is a text file and that the error is on the first line of sh itself.

This seems like something that could easily be implemented, is it something you would be interested in?

Yes, I will do this.

Although the change itself should be easy, perhaps even requiring only a single line of code to be changed, we do not have tests for the case of $0 that should change, nor for the cases of $0 that should not change:

• When $0 is expanded due to appearing directly in the operand to -c, the command that leads to this happening should pass arguments in such a way that $0 expands--starting when the change discussed here is made--to the path used to run the shell itself (i.e. the executable receiving the -c option, its operand, and further arguments).
• When $0 is expanded due to appearing in a file that is run as a shell script, and -c was not used, $0 should expand to that file's name or path.
• When $0 is expanded due to appearing in a file that is run as a shell script, and -c was used but still to run that separate file by running a one-line command that invokes the file by its name, $0 should expand to that file's name or path, just the same was if -c was not used.

$@, which may be added automatically, should continue to receive/expand the correct positional parameters. This is true even in the third case, since it is forwarded, i.e., "$@" in the script has the same value as it has in the operand to -c that forwarded the positional parameters, even though $0 in the script does not have the same value as it would have in the operand to -c if that operand were to expand it.

This should automatically work, but I think we need to cover at least $0 in the first and third case with tests. (The distinction between the first and second cases may to some extent be considered an implementation detail, and which one occurs due to the same code may vary across operating systems, even in the absence of bugs, because it depends in part on whether shebangs are handled explicitly or by the operating system.) The risk of a regression is low on Unix-like systems, but:

• Without tests of this, it will not be clear from reading the code what the intended behavior is and that the cases are properly distinguished.
• A regression could plausibly occur on Windows, because the situation on Windows is very unintuitive, more is done explicitly rather than automatically, and which things are done explicitly and how they are done may change in the future, including in ways not currently anticipated.

So far, so good. The problem is that even the tests of $0-related behavior that has been in place for some time, and that the forthcoming $0-related change should not affect, do not currently pass on Windows. This is because, on Windows, the way we run shell scripts from arbitrary paths to script files does not work in general, due to the treatment of \ in those paths.

This should be feasible to fix one way or another, even without yet improving the behavior of the code under test:

• Don't we already have something that renormalizes Windows paths to use /? (Contrary to widespread belief, it actually cannot always be done. In particular, not all \\?\ and \??\ paths can be turned into a path with no backslashes, even if the path need only refer to the same file on the current machine and the filesystem can be accessed to figure out how to produce it. In practice, however, it can usually be done.)
• If not, I can either look into why this problem still seems to happen even when the path is quoted for the shell, or implement a not-really-correct way that is correct enough for use in tests, by requiring the path to be valid UTF-8 and using String::replace. (The reason this is not really correct is not related to the possibility of unpaired surrogates, but rather due to how the meaning or usability of some \\ paths changes if \ is replaced with /. In addition to supporting a //./ variant of \\.\, the Rust standard library even supports a //?/ variant of \\?\. But \\?\ and //?/ paths would still not be equivalent in the rare case that \\?\ was used to represent a literal / in a component on a strange filesystem. More significantly, even programs that recognize \\?\ and recognize / in general may not recognize //?/. Furthermore, the Rust standard library does not support a /??/ variant of \??\, probably because \??\ paths are in the form used by the NT object manager, which does not recognize / as a separator. Some \??\ paths refer to files and directories, and the Rust standard library does support \??\, so changing \??\ to /??/ could break a path that worked before..)
• Another approach is to change directory to where the script is and use ./filename to run it. That works because the script names are fully under our control, and the relative paths ./filename and .\filename do not differ in meaning.

EliahKagan@0d1ac8e is what I have so far for testing the behavior that should not change under the forthcoming change. The commit message states the above ideas in a way that may be clearer, I am not sure. (Once the tests pass on all platforms, I'll convert them to be more compact by using BStr facilities to a greater extent and comparing to a string, rather than dealing with lines as it is currently doing, unless a reason arises why that shouldn't be done.) They pass on Unix-like systems. Here's how they fail on Windows. Interestingly, one of them, name_with_args_indirect, already passes on Windows. I do not know why it does.

The trivial tests just verify that a hello world shell script works. They fail in the same way as the others, isolating the problem to running the script itself. The output of trivial_direct is:

──── STDERR:             gix-command::command spawn::script::trivial_direct
Error: Os { code: 3, kind: NotFound, message: "The system cannot find the path specified." }

That's not very informative. It might be failing in the same way as trivial_indirect, which calls the script indirectly through sh -c where the operand to -c is the path to the script, whose output is more informative, showing the effect that one would expect from the backslashes being interpreted as escape characters and then dropped in quote removal:

──── STDERR:             gix-command::command spawn::script::trivial_indirect
--: line 1: testsfixturesgenerated-do-not-editscripts2549957747-windowstrivial: command not found
thread 'spawn::script::trivial_indirect' panicked at gix-command\tests\command.rs:590:13:
assertion failed: out.status.success()

That is identical to the path munging that occurred for fixture scripts when running the test suite in most local Windows environments from PowerShell rather than Git Bash, prior to #1712. But it is not affected by the environment in which the new tests on that feature branch are run (nor would I expect that).

Although it's tempting to interpret the less informative output from trivial_direct and the other tests like it as indicating the same cause of failure as in trivial_indirect and the other tests like it, I am not completely sure that's the case.

Note

Updated information on this, related to the subsequent commit EliahKagan@387da88:

The causes are different. The *_indirect tests failed due to shell interpretation of \, and changing each \ to / worked for the paths used in those tests; I've noted the limitations in a comment on the relevant part of the new test code. The *_direct tests fail because, while gix-command has logic to parse and run interpreters specified in #! lines on Windows, it does not properly handle leading-/ paths like /bin/sh.

On Windows, paths that start with a single \ or / are relative to the current drive, but such paths in shebangs should not be resolved that way. This relates to #1761, though it is not limited to sh, and to the discussion at #1815 (comment), though modifying the environment would not be enough to fix it. A real fix would make running scripts installed as hooks safer and much more likely to succeed, so that would be a good thing. But I don't think I have to fix this shebang handling bug now to improve test coverage in the way I want to do here.

Instead, I can figure out whether it's reasonable to skip the *_direct tests on Windows. If so, that's the workaround for now. If not, then I can build shebangs that work on Windows. This applies only to the tests of behavior that should not be changed; none of yet-to-be written tests, roughly sketched in EliahKagan@8c0b609, will need to be skipped.

If we are agreed on what the change in gix-command should be for the alternative to -- for $0--even if not decided or agreed on what should be tested--then this discussion could be converted into an issue. That may make it less likely to be forgotten if a small amount of further effort toward getting the tests working does not work and I end up putting this off. (I'm working on other things I consider higher priority--including things in gitoxide that I consider higher priority--and while I had hoped to improve this so that I could get nicer error messages when working on other things, I didn't realize the difficulties that would arise when writing tests that distinguish the cases that should and shouldn't be changed.)

I can convert it to an issue (I'm just waiting for clarification that we agree on the fundamentals of what kind of change would fix it), though I don't think I can assign it to myself.

Regarding making shell_display_name() configurable, since it seems to also affect shells as they'd act like login shells if their name is prefixed with -, I feel like waiting for demand for such a feature is sufficient (to have a reason to spend the time to flesh it out).

I certainly have no objection to omitting this additional functionality for now, and I agree it may not ever be needed.

However, while this is not in any way an argument for adding that feature--actually it is something of an argument against it--please note that the experiment I showed above, running various shells with $0 set to a --prefixed name, shows the opposite: that it does not cause them to act like login shells. It does not cause the shell's own argv[0] to be changed, nor to act as though it had been changed.

Edit: Returning to this and rereading what you've said, I think we are actually on the same page about this, and that what you meant by "it seems to" is that it may appear wrongly to have this effect, rather than that it seems to you or me that it does. The rest of this comment is about interpretation of the experiment with various shells in #1840 (reply in thread). It seems potentially valuable, so I've kept it in, even though it may not be needed for purposes of clarification as I had thought when I wrote it.
 
The significance of this topic is that, if we had a feature to set $0, then developers might wrongly think it changes argv[0] for the shell itself. Some experiments developers might do to check this could erroneously make it look like argv[0] had been changed for the shell, though the experiment I did does distinguish them. Looking at the bash part of the test:

• Running bash by the name bash, passing -c and a command, and not passing a command name argument to set $0, causes $0 to be bash, and does not cause the shell to be a login shell:

  $ bash -c 'printf "%s\t" "$0"; shopt login_shell'
  bash    login_shell     off
  

• Running bash by the name -bash, passing -c and a command, and not passing a command name argument to set $0, causes $0 to be -bash, and does cause the shell to be a login shell:

  $ -bash -c 'printf "%s\t" "$0"; shopt login_shell'
  -bash   login_shell     on
  

• Running bash by the name bsah, passing -c and a command, and passing a command name argument to set $0 to -bash, causes $0 to be -bash, but does not cause the shell to be a login shell:

  $ bash -c 'printf "%s\t" "$0"; shopt login_shell' -bash
  -bash   login_shell     off
  

The tests with other shells are analogous, and show the same results, but look different because the other shells tested place l in the expansion of $- to indicate that they are login shells. (shopt is a bash-ism, but POSIX does not require that $- ever contain l, and bash does not include l in it. Other important shells such as dash and busybox ash are not included in the test because I don't know of any convenient way to test them for this without doing something in .profile and verifying it ran.)

[Byron]

[with 'line 1' appended] This seems like an improvement to me, still. In particular, no one is going to think sh itself is a text file and that the error is on the first line of sh itself.

Yes, that's even better, as it additionally avoids getting overly long.

Yes, I will do this.

Great, thanks!

• Don't we already have something that renormalizes Windows paths to use /? (Contrary to widespread belief, it actually cannot always be done. In particular, not all \\?\ and \??\ paths can be turned into a path with no backslashes, even if the path need only refer to the same file on the current machine and the filesystem can be accessed to figure out how to produce it. In practice, however, it can usually be done.)

There is gix_path::…convert_to_slashes_on_windows() but it definitely doesn't handle ? in any way. I remember using a crate from kornelski once that turns such path back into normal paths, but I forgot its name and where I used it 😅. It was very slim and came without dependencies, so is genuinely something that could be used in gitoxide as well (if it isn't already), particularly if it's a dev-dependency.

• Another approach is to change directory to where the script is and use ./_filename_ to run it. That works because the script names are fully under our control, and the relative paths ./_filename_ and .\_filename_ do not differ in meaning.

This seems like a reliable approach, and I am surprised this doesn't already happen - usually I try to avoid absolute paths.

I can convert it to an issue (I'm just waiting for clarification that we agree on the fundamentals of what kind of change would fix it), though I don't think I can assign it to myself.

Please do go ahead and create the issue with whatever solution for -- you find preferable. I did like the <shell-path> line 1 variant, but even just using <shell-path> is an improvement worth going for.

Edit: Returning to this and rereading what you've said, I think we are actually on the same page about this, and that what you meant by "it seems to" is that it may appear wrongly to have this effect, rather than that it seems to you or me that it does.

Actually I understood it wrongly, and it's good to know that shell_display_name() will not be able to affect the login shell behaviour - that's what I originally thought.

I am sorry if I didn't capture all context and/or respond to each question I should have responded to, these days I am extra-short on time and race through everything.

[EliahKagan]

Please do go ahead and create the issue with whatever solution for -- you find preferable.

Sounds good, I've created issue #1842 from this, and also added a summary of what I think is the most important information (across all comments here). Hopefully I will also manage to make a PR that closes it sometime soon. Please feel free to assign me to the issue if desired. You may also want to move the help-wanted label from here to there.

[with 'line 1' appended] This seems like an improvement to me, still. In particular, no one is going to think sh itself is a text file and that the error is on the first line of sh itself.

Yes, that's even better, as it additionally avoids getting overly long.

I did like the <shell-path> line 1 variant, but even just using <shell-path> is an improvement worth going for.

To clarify, these are actually the same approach. Both are with $0 set to the name/path that was used to invoke the shell itself. Whether line 1, other contextual information, or nothing extra appears depends on the implementation of the shell, the cause of the error, and whether the error is produced directly by the shell or by expanding $0 in the operand to -c. I do not propose to add anything like this explicitly, which would be more complicated, would produce things like line 1: line 1 some of the time, and would make expansions of $0 elsewhere than at the beginning of a message more confusing.

Actually I understood it wrongly, and it's good to know that shell_display_name() will not be able to affect the login shell behaviour - that's what I originally thought.

Ah, okay--I'm glad I clarified further then. I will still refrain from adding shell_display_name() as something the user sets via gix_command::Prepare unless and until a concrete use for this arises.

I am sorry if I didn't capture all context and/or respond to each question I should have responded to, these days I am extra-short on time and race through everything.

That's quite all right. Even if you were not able to give full attention to some parts, they remain available to refer back to if interested, or when the topics they relate to come up again. In addition, from #1815 (comment) it looks like you are continuing to notice and give feedback on the most important things. So while I hope everything is going okay, I have no complaints!

In view of your being short on time, I have ordered my replies in this comment so that the parts that may not be of immediate interest are below, and I will not assume that you have specifically considered them unless you say so. (In particular, I will not assume, without checking, that you would agree with any future design decisions that might be based on any of the analysis below.)

---

• Don't we already have something that renormalizes Windows paths to use /?

There is gix_path::…convert_to_slashes_on_windows()

I didn't find a method by that name, but it put me on the right path (pun intended) to:

gitoxide/gix-path/src/convert.rs

Lines 220 to 230 in b4fe425

	/// Replaces windows path separators with slashes, but only do so on windows.
	pub fn to_unix_separators_on_windows<'a>(path: impl Into<Cow<'a, BStr>>) -> Cow<'a, BStr> {
	#[cfg(windows)]
	{
	replace(path, b'\\', b'/')
	}
	#[cfg(not(windows))]
	{
	path.into()
	}
	}

but it definitely doesn't handle ? in any way.

I have this caveat in code on my feature branch, on a function that can possibly be eliminated in favor of using to_unix_separators_on_windows:

gitoxide/gix-command/tests/command.rs

Lines 569 to 575 in 387da88

	/// The path uses `/` as a separator. On Windows, it achieves this by replacing each
	/// occurrence of `\` with `/`. This does not always preserve usability, and sometimes
	/// does not even preserve meaning. In particular, `\\?\` paths would often become less
	/// usable (though `std` recognizes `//?/`) and occasionally incorrect (if a component
	/// contained a literal `/` on a strange filesystem), and NT object manager paths such as
	/// those that begin with `\??\` (which `std` recognizes, when they refer to entries on a
	/// filesystem) are always broken. But they might be good enough to use in the test suite.

While the wording can definitely be improved, maybe I can add something along those lines to the to_unix_separators_on_windows docstring.

I remember using a crate from kornelski once that turns such path back into normal paths, but I forgot its name and where I used it 😅. It was very slim and came without dependencies, so is genuinely something that could be used in gitoxide as well (if it isn't already), particularly if it's a dev-dependency.

Two possibilities come to mind.

I think you may be thinking of dunce, whose simplified function is already used in gix-discover, in discover_opts and in find_ceiling_height.

dunce does not produce paths with / separators, since that is not related to its purpose. Instead, it turns \\?\ paths into the more ordinary kind, when doing so preserves the meaning. But it might still be used to good effect as part of a routine that changes a path to use / separators, to remove \\?\ when that can readily be done, so that the resulting /-using path is more likely to be usable and possibly more likely to be correct.

My impression is that dunce is widely used and tends to work very well in practice. (There is also talk in rust-lang/rust#42869 of bringing some functionality of this kind into std.) I would not hesitate to use it to replace code that is already in production with bugs that dunce could clearly fix. However, besides that, there are some subtleties that I may want to look into before making broader use of it in tricky situations, such as with paths obtained from unknown sources or that are used in complex or unpredictable ways:

• It does not modify any \\.\ paths, which might be the right design decision, though the associated test case looks like it may based on the false belief that \\.\C:\notdisk wouldn't refer to a file or directory named notdisk in the root directory of the C: drive. (\\.\ paths are called local device paths, but the only difference between \\.\ and \\?\ paths is that \\?\ paths are treated more literally, i.e., \\?\ suppresses the special meaning of everything other than \. \\.\C:\notdisk is equivalent to \\?\C:\notdisk and equally safe to simplify to C:\notdisk.)

  C:\Users\ek> evcxr
  Welcome to evcxr. For help, type :help
  >> :dep dunce
     Compiling dunce v1.0.5
  >> use std::path::Path;
  >> fn p(s: &str) { println!("{}", dunce::simplified(Path::new(s)).to_str().unwrap()); }
  >> p(r"\\?\C:\Windows");
  C:\Windows
  >> p(r"\\.\C:\Windows");
  \\.\C:\Windows
  

• It does not modify any \??\ paths, which I think is a deliberate design decision and which I suspect is the right choice, even if some could be simplified. So I don't think this is a bug, but it is relevant to some potential uses.

  >> p(r"\??\C:\Windows");
  \??\C:\Windows
  

• Paths with unpaired surrogates appearing anywhere in them are never modified. This is documented.

• It refrains from removing a \\?\ prefix from paths containing most of the names Microsoft documents as reserved, but it does not treat the names with superscript numbers as reserved, even though they are documented and behave as such. (This comment claims that all reserved names have an "ASCII-compatible stem," which suggests that the names with superscripts, which I think are representable in most ANSI code pages used on Windows but which are not ASCII, were not considered.)

  >> p(r"\\?\C:\Windows\COM1");
  \\?\C:\Windows\COM1
  >> p(r"\\?\C:\Windows\COM¹");
  C:\Windows\COM¹
  

  This is a bug, but I think a minor one. In my testing, the names with superscript numbers are separate from the similar names with ordinary numbers. For example, COM¹ is reserved, but it is not equivalent to COM1 and it does not refer to a serial port. Opening an actual device from a name like COM¹ would therefore only happen if a driver is installed that gives a device that name.

  I have not been deeply investigating anything specifically to do with dunce. My testing of how Windows treats filenames like COM¹ was done for gitoxide, which also does not currently treat them as reserved when they appear in a repository's paths or ref names. I think we should probably treat them as reserved, too, on Windows, but it is not at all urgent to do so.

• At least in Windows 10 and earlier (I have not tested this on Windows 11), Windows treats the names CONIN$ and CONOUT$ as reserved device names, behaving like CON but only open for read or write, respectively. This is even though Microsoft does not list them as such. dunce does not refrain from removing the \\?\ prefix from paths with CONOUT$ and CONIN$ components. (gitoxide does already rightly treat them as reserved in paths and ref names in a repository, as well as covering the documented reserved names other than the minor case of those with superscripts.)

  >> p(r"\\?\C:\Windows\CON");
  \\?\C:\Windows\CON
  >> p(r"\\?\C:\Windows\CONIN$");
  C:\Windows\CONIN$
  >> p(r"\\?\C:\Windows\CONOUT$");
  C:\Windows\CONOUT$
  

  Unless this is intended, I think it is a significant bug. dunce does document that it implements the rules described in that article, and that article does not mention CONOUT$ or CONIN$. But if treating them as non-reserved is intended, I think it should be documented explicitly. Opening a dunce issue or merge request is on my list of things to do, if no one else gets to it first.

The other crate that comes to mind (though not by kornelski) is path-slash, which converts Windows paths between / and \ form or, in some cases, partially does so.

to_unix_separators and to_windows_separators have TODO comments about using this crate "to handle escapes." As detailed in #1841, I think it is not feasible to use it that way. One of the reasons is that it does not seem to offer such functionality. When its documentation says \ should not be changed to / when it was used for escaping, I think it is offering usage advice, rather than describing its own functionality.

This does not mean path-slash can't potentially be used, in those gix-path functions or in the test suite. That its scope does not seem to include treating \ as an escape character seems like a good thing to me, and may even argue for its use, in those gix-path functions or in the test suite. However, it has a few limitations that make me reluctant to reach for it immediately:

• It uses String rather than OsString. This limitation related to UTF-8 is documented and not a bug, but it may make it not always suitable for processing paths that come from a repository. (Note that, unlike dunce, it does not leave such paths as-is. Instead, depending on which function is used, it either fails or replaces invalid UTF-8 such as unpaired surrogates with a substitution character.)

• While it preserves a trailing \, it removes a trailing /. This is a problem if it is ever used to operate on a path that designates a device followed by just a /. This includes some fairly common absolute paths like C:/, which could be present in input and seems especially likely to arise when operating on a Git repository, and which would be turned C:, which deisgnates the current directory on the C: drive. This also makes path-slash non-idempotent (C:\ → C:/ → C:).

  C:\Users\ek> evcxr
  Welcome to evcxr. For help, type :help
  >> :dep path-slash
  >> use std::path::Path;
  >> use path_slash::PathExt;
  >> fn p(s: &str) { println!("{}", Path::new(s).to_slash().unwrap()); }
  >> p(r"C:\");
  C:/
  >> p(r"C:/");
  C:
  >> Path::new(r"C:\").canonicalize()
  Ok("\\\\?\\C:\\")
  >> Path::new(r"C:/").canonicalize()
  Ok("\\\\?\\C:\\")
  >> Path::new(r"C:").canonicalize()
  Ok("\\\\?\\C:\\Users\\ek")
  

  This also affects some paths that do not contain drive letters:

  >> p(r"\\.\pipe\");
  \\.\pipe/
  >> p(r"\\.\pipe/");
  \\.\pipe
  >> Path::new(r"\\.\pipe\").exists()
  true
  >> Path::new(r"\\.\pipe/").exists()
  true
  >> Path::new(r"\\.\pipe").exists()
  false
  

• It breaks \\?\ paths by keeping the prefix \\?\ while changing the directory separator from \ to / otherwise. As mentioned above regarding the difference between \\.\ and \\?\, the / character is never a directory separator in a \\?\ path.

  >> p(r"C:\Windows");
  C:/Windows
  >> Path::new(r"C:\Windows").exists()
  true
  >> Path::new(r"C:/Windows").exists()
  true
  >> p(r"\\?\C:\Windows");
  \\?\C:/Windows
  >> Path::new(r"\\?\C:\Windows").exists()
  true
  >> Path::new(r"\\?\C:/Windows").exists()
  false
  

• It breaks \??\ paths by converting all occurrences of \ to / in them. The NT object manager does not recognize / as a directory separator, and there is no /??/ form related to \??\ in Windows or std. (If such paths are to be converted to change any occurrence of \ to /, they also have to be converted to a non-\??\ form, which is nontrivial, would not always be possible, and when possible would sometimes requiring accessing the filesystem to figure out how.)

  >> p(r"\??\C:\Windows");
  /??/C:/Windows
  >> Path::new(r"\??\C:\Windows").exists()
  true
  >> Path::new(r"/??/C:/Windows").exists()
  false
  

[Byron]

I do not propose to add anything like this explicitly, which would be more complicated, would produce things like line 1: line 1 some of the time, and would make expansions of $0 elsewhere than at the beginning of a message more confusing.

I see, thanks for the clarification.

That's quite all right. Even if you were not able to give full attention to some parts, they remain available to refer back to if interested, or when the topics they relate to come up again. In addition, from #1815 (comment) it looks like you are continuing to notice and give feedback on the most important things. So while I hope everything is going okay, I have no complaints!

In view of your being short on time, I have ordered my replies in this comment so that the parts that may not be of immediate interest are below, and I will not assume that you have specifically considered them unless you say so. (In particular, I will not assume, without checking, that you would agree with any future design decisions that might be based on any of the analysis below.)

That's very considerate and does make a difference! Everything is indeed alright, all my time goes to GitButler for now.

I think you may be thinking of dunce, whose simplified function is already used in gix-discover, in discover_opts and in find_ceiling_height.

Yes, that was it! Thanks for jogging my memory 😅.

However, besides that, there are some subtleties that I may want to look into before making broader use of it in tricky situations, such as with paths obtained from unknown sources or that are used in complex or unpredictable ways:

I have a feeling that this research, as it's done by you, would be useful for both std and dunce. I am glad you already got involved in the std thread, so I am optimistic your work will lead to improvements there as well 🙏.

[EliahKagan]

Everything is indeed alright

That's good to hear!

[EliahKagan]

A correction: The relationship between the value of $0 and the path the shell is run with, when the shell runs a command with -c but no command name argument is passed, is as stated above for Unix-like systems, at least for all shells tested including bash. But it does not work this way when the shell is a in Cygwin/MSYS/MSYS2 environment on Windows, including for sh.exe and bash.exe shipped with Git for Windows. For example:

C:\Users\ek> C:\Users\ek\scoop\apps\git\2.48.1\usr\bin\sh.exe -c 'echo "$0"'
/usr/bin/sh

This is in (git installation)\usr\bin; it is not a shim, so this behavior is not specific to when a shim is used. I believe it is actually receiving a different value of argv[0], after the MSYS2 translation layer: programs that link to msys-2.0.dll (or analogous on MSYS1 or Cygwin) can receive MSYS2 paths that would have a different interpretation in native Windows applications.

This doesn't necessarily mean we can't pass the actual Windows path used to run the shell as the command name argument. I think there are some other path transformations that could potentially cause a problem, but that's not the main thing I'm thinking about right now; it even more significantly affects positional parameters, and I hope to look into it separately, ideally sometime soon. Rather, this motivates a simpler design idea.

The full Windows path may be unexpected because it would not be shown in shell scripts under other circumstances. Using only the last component of the path may be sufficient, may avoid confusion as well as other path-transformation problems, and is shorter. It reflects a choice of shell set via Prepare::shell_program() but otherwise will usually be sh.exe on Windows and will currently always be sh on a Unix-like system.