Panic due to unkown shell #16
Comments
|
Noticed this myself. Some additional information: it seems only certain terminal emulators return |
zackw
added a commit
to zackw/silver
that referenced
this issue
Nov 24, 2020
All silver subcommands look up the parent process’s name, and use that, verbatim, to identify the shell in use. This has several problems: - On Windows, the parent process’s name may or may not end with “.exe”; there was special case logic to handle “powershell.exe” versus “powershell” but not “bash.exe” versus “bash”. Relatedly, the parent process’s name ought to be interpreted case- insensitively on Windows; BASH.EXE is the same as bash.exe. - On Unix, there is a notion of a _login_ shell (invoked directly by login(1) for console logins, or sshd(1) for remote logins); these are identified by a leading dash in their process name, *not* by a special option. There was no code to handle this at all (bug reujab#67). - sysinfo is slow, because it reads a whole bunch of files in /proc whether it’s going to need the data within or not. Some but not all of this overhead can be eliminated by using `System::new_with_specifics(RefreshKind::new())` instead of `System::new_all()`, but even then the wall-clock time for `silver init` drops from 0.021s to 0.002s (release build) if sysinfo is not invoked at all. This patch addresses all these problems, as follows: - There is a new command line option `--shell` which may be used to explicitly set the shell in use. Also, support for the `SILVER_SHELL` environment variable (used for this purpose in 1.1 and previous) is restored. sysinfo is only consulted if neither the command line option, nor the environment variable, is available. - No matter where we get the process name from, it is lowercased, a trailing “.exe” is removed, a leading dash is removed, and any leading directory names are also removed (bug reujab#16). - The initialization scripts set `SILVER_SHELL`, so sysinfo will be consulted at most once per session, not once per command. I also reorganized the code in the Command::Init branch of main for readability’s sake, and made the “unknown shell” error message not be a panic. (Panics are for _bugs_, not for errors with a cause outside the program.)
zackw
added a commit
to zackw/silver
that referenced
this issue
Nov 24, 2020
All silver subcommands look up the parent process’s name, and use that, verbatim, to identify the shell in use. This has several problems: - On Windows, the parent process’s name may or may not end with “.exe”; there was special case logic to handle “powershell.exe” versus “powershell” but not “bash.exe” versus “bash”. Relatedly, the parent process’s name ought to be interpreted case- insensitively on Windows; BASH.EXE is the same as bash.exe. - On Unix, there is a notion of a _login_ shell (invoked directly by login(1) for console logins, or sshd(1) for remote logins); these are identified by a leading dash in their process name, *not* by a special option. There was no code to handle this at all (bug reujab#67). - sysinfo is slow, because it reads a whole bunch of files in /proc whether it’s going to need the data within or not. Some but not all of this overhead can be eliminated by using `System::new_with_specifics(RefreshKind::new())` instead of `System::new_all()`, but even then the wall-clock time for `silver init` drops from 0.021s to 0.002s (release build) if sysinfo is not invoked at all. This patch addresses all these problems, as follows: - There is a new command line option `--shell` which may be used to explicitly set the shell in use. Also, support for the `SILVER_SHELL` environment variable (used for this purpose in 1.1 and previous) is restored. sysinfo is only consulted if neither the command line option, nor the environment variable, is available. - No matter where we get the process name from, it is lowercased, a trailing “.exe” is removed, a leading dash is removed, and any leading directory names are also removed (bug reujab#16). - The initialization scripts set `SILVER_SHELL`, so sysinfo will be consulted at most once per session, not once per command. I also reorganized the code in the Command::Init branch of main for readability’s sake, and made the “unknown shell” error message not be a panic. (Panics are for _bugs_, not for errors with a cause outside the program.)
zackw
added a commit
to zackw/silver
that referenced
this issue
Nov 24, 2020
All silver subcommands look up the parent process’s name, and use that, verbatim, to identify the shell in use. This has several problems: - On Windows, the parent process’s name may or may not end with “.exe”; there was special case logic to handle “powershell.exe” versus “powershell” but not “bash.exe” versus “bash”. Relatedly, the parent process’s name ought to be interpreted case- insensitively on Windows; BASH.EXE is the same as bash.exe. - On Unix, there is a notion of a _login_ shell (invoked directly by login(1) for console logins, or sshd(1) for remote logins); these are identified by a leading dash in their process name, *not* by a special option. There was no code to handle this at all (bug reujab#67). - sysinfo is slow, because it reads a whole bunch of files in /proc whether it’s going to need the data within or not. Some but not all of this overhead can be eliminated by using `System::new_with_specifics(RefreshKind::new())` instead of `System::new_all()`, but even then the wall-clock time for `silver init` drops from 0.021s to 0.002s (release build) if sysinfo is not invoked at all. This patch addresses all these problems, as follows: - There is a new command line option `--shell` which may be used to explicitly set the shell in use. Also, support for the `SILVER_SHELL` environment variable (used for this purpose in 1.1 and previous) is restored. sysinfo is only consulted if neither the command line option, nor the environment variable, is available. - No matter where we get the process name from, it is lowercased, a trailing “.exe” is removed, a leading dash is removed, and any leading directory names are also removed (bug reujab#16). - The initialization scripts set `SILVER_SHELL`, so sysinfo will be consulted at most once per session, not once per command. I also reorganized the code in the Command::Init branch of main for readability’s sake, and made the “unknown shell” error message not be a panic. (Panics are for _bugs_, not for errors with a cause outside the program.)
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Problem under fedora:
results in
A possible solution for bash would be to modify in
.bashrc:Another option would be to correctly parse the path of
$0in main.By the way, the error message would be more helpful (i.e. the use does not need to look at the src), if it includes the current value of $SILVER_SHELL and the supported values.
The text was updated successfully, but these errors were encountered: