Issue with busybox ash inside bash #428
Comments
|
No way to fix it other than setting a PS1 value which is suitable for ash. In fact, it won't work in any other shell either, and sometimes not even with bash, and also on linux. Because the string Even more so, the better way to use the git-prompt thing in bash is to use PROMPT_COMMAND, which, in turn, sets PS1 every time a new prompt is printed, so that PS1 value is sometimes not even set by bash or the user, but rather by the git-prompt thing. And that git-prompt doesn't support ash. It uses arrays and some other featured iirc which just don't work in ash. So the only way to "fix" it to to setup a PS1 value of your own which ash supports. |
This was fixed recently. By default, ash incorrectly converted back slashes to forward slashes, but it was fixed about a month ago iirc. Update your busybox.exe . |
Actually, I'm not sure. There was an issue that when using But this is a different issue. These are backslashes which should remain unmodified, but the default behavior is to convert them to slashes, so it's probably still an issue in master. As a workaround, you can use a recent busybox.exe (month or two) and sh -X to preserve backslashes in env vars (including paths). |
|
I currently have the busybox of 2024-06-16. |
No need. it's recent enough. Just use sh -X to preserve the backslashes in PS1, so this should fix the issue with incorrect forward slashes. As for the __git_ps1 issue, there's no fix because it's incompatible with ash. |
|
I understand that the issue with The |
Maybe @rmyorston wants to add it as another exception to the list (which currently includes at least COMSPEC).
Speak for yourself. For me, on win10, there's no reason for busybox to modify env vars values at all (changing the name of some vars to uppercase is OK). But it does modify variables by default, exactly like it did for you with PS1, which is rightly a bug. So to workaround the bug which modifies env values by default, I use -X, and I encourage others to do that as well. Just be careful that some paths include backslashes, so any script which deals with paths with NIH code should take that into account. But if one uses the standard tools to handle paths (dirname, basename, etc), then these would handle backslashes withot any issues. |
It is obviously an user choice but I usually prefer to use slashes in pathes everywhere to be more coherent. The only issues are like in these cases where backslashes aren't part of pathes. |
|
PS: An alternative way would be instead of a blacklist use a whitelist so that only variables that are know to contain pathes are changed. |
-X only affects (prevents) converting env vars when the shell is entered. The relevant ones are typically the standard windows path vars ($PATH, $UERSPROFILE, $LOCALAPPDATA, etc). But it doesn't prevent you from using forward shashes in everything you do in bb-w32, or adding dirs with forward shashes to $PATH, etc.
Out of curiosity, in the years you used bysybox-w32, did you ever have a use case which would break if those vars were not converted to forward slashes? Because I use the bb-w32 shell quite a lot, and I don't think I've ever had a use case where I needed to use one of these windows path vars which would break if the paths contains backslashes... And if you really have some vars which you want converted to forward shashes, then you can still use -X and then add something like this to your for name in PATH UERSPROFILE LOCALAPPDATA; do
eval "v=\$$name"
v=$(printf %s "$v" | sed 's/\\/\//g')
eval "$name=\$v"
done(and add as many variables names as you want) |
|
It isn't breaking, it is just annoying. For example, without conversion, with this: I know I could also fix them with realpath but it is nice to have them correct as is. eval is usually something that I avoid unless there isn't any other choice (potentially slow and risky). |
IMO, the fact that you're annoyed by something which is not broken and has zero impact on functionality, is not a good enough reason for bb-w32 to modify env vars by default. IMO clearly the goal in converting to forward slashes was to avoid breaking some things, but I just can't think of anything which would break in such case under reasonably normal usage.
It's never slow. It's typically undistinguishable from non-eval code in terms of speed. Even if it was slow, doesn't matter when it only executes once you enter an interactive shell. This specific use case is also 100% safe, and 100% correct (up to
It's not the regexp which is slow, but rather the additional processes and command substitution, but again, it has no impact when it only executes once on shell init, and it would only add very few ms in practice. There's not much to mess up here anyway. It's a trivial replacement, which you can also do with If you just refuse to use eval and sed, then don't: PATH=$(printf %s "$PATH" | tr \\ /)
FOO=$(printf %s "$FOO" | tr \\ /)
...The bottom line of all this is that I don't see a need to touch vars on shell init, and if you really want forward slashes only in specific vars, then currently the only option is to use -X and modify the vars you want as described above - which would fix your PS1 issue, and the issue with any other var which was incorrectly converted to forward slashes. |
Shell variables imported from the environment are marked with a special flag and backslashes in their values are (by default) replaced with forward slashes. Sometimes this may not be what we want. Modify the 'export' shell built-in so unexporting a variable of this type restores its original value from the environment and removes its special flag. It can then be re-exported. Adds 32 bytes. (GitHub issue #428)
|
I've tweaked the shell's This allows
There are new prereleases (PRE-5405 or above). |
Hmm... 100% undetectable applies to many bb shell features, like How would one know that But more on the topic of the discussion above, what is the default conversion to forward slashes supposed to help with? As noted above, I haven't encountered an issue where anything would break due to keeping backslashes (and I do use -X, so I'd know if something got broken), and the only "issue" ale5000-git can report is that it's "annoying" that the result of e.g. |
|
Yup,
As I said before:
|
It can in some cases, and it's a personal preference, but for me the cons outeright the pros, because any shell code in busybox-w32 need to handle backslashes anyway, e.g. it it arrives unmodified via argv from cmd.exe or elsewhere, and *nix script also expect different list-separator (colon vs semicolon), so it's much better to be on the safe side and ensure that both backslashes and semicolon work in such scripts, and not try to use the code unmodified. And obviously converting to forward shashes need to be curated, because some things like $COMSPEC must be excluded, as should $PS1, and other things we can't think of currently. And trying to teach users to do Also, users on windows do expect paths to contain backslashes. it wouldn't surprise anyone IMO that $PATH has backslashes and semicolon separator - people expect that. So overall I think for most people it would be better to not convert back to forward slashes, but indeed ultimately the decision about the default is yours. |
I just express my opinion, obviously this thing is opinable but even though I don't always agree I respect the opinion of everyone and I really appreciate your contribution and the one of rmyorston. |
|
@rmyorston Note: The thing inside ` is executed but also with backslashes should work correctly. PS3 does not expand Bash escape sequences, so it doesn't really matter. |
FYI, I fixed that. upstream It's not blazing fast on Windows because it can invoke See the docs at the top of the file, but e.g. this works in bb(-w32) ash in . path/to/git-prompt.sh
GIT_PS1_SHOWCOLORHINTS=1
GIT_PS1_SHOWDIRTYSTATE=1
GIT_PS1_SHOWUPSTREAM=verbose
GIT_PS1_DESCRIBE_STYLE=tag
GIT_PS1_COLOR_PRE='\['
GIT_PS1_COLOR_POST='\]'
PS1='\n\w$(__git_ps1) \u@\h\n\$ '(feel free to manually add colors to PS1 or otherwise customize it to your liking outside the __git_ps1 part) |
|
@avih |
I'm not using bash included in git, and don't intend to try fixing issues there, but a good first step would be that you report the issue to them. |
|
I know but I think the developers of git for windows are mostly focused to git and not much to bash; in fact the bugs (including a crash) that I have reported in the past are still open. |
|
Reporting it there is still better than doing nothing or reporting it here. |
By running busybox ash inside bash (bash is from https://gitforwindows.org/) I get this:
Is there any way to fix it without manually altering PS1 or not?
From a different version of bash there isn't any error but it is still messed up:
/[/e]0;/w/a/]/n/[/e[32m/]/u@/h /[/e[35m/]/[/e[0m/] /[/e[33m/]/w/[/e[0m/]/n/$The text was updated successfully, but these errors were encountered: