git-prompt: fully enable the PS1 prompt on the SDK

[PhilipOakley]

The SDK installation, at some point, overwrites the /etc/bash.bashrc file
and now detects a $MSYS2_PS1 prompt as an alternative to setting a default
PS1 prompt. Any pre-prepared PS1 is overwritten.

Provide that alternate environment variable.

The Git-for-Windows installation does not use that version of the
/etc/bash.bashrc file, so is unaffected.

Signed-off-by: Philip Oakley philipoakley@iee.org

[PhilipOakley]

This is a partial solution to git-for-windows/git#794 (comment) where the title prefix and prompt isn't set as would be expected.

I don't know enough about pqcman and how to debug it to find out which package overwrites the /etc/bash.bashrc file during the SDK install, nor whether it's part of a base package or a patch to the package (and how to determine those)

[dscho]

Thanks! I had to run an updpkgsums to adjust the checksums, but that's quite alright.

[PhilipOakley]

@dscho, Thanks for the merge. Sorry I wasn't aware about the updpkgsums - the foo is running low.

I'll still keep looking at how to find out which package did the dirty deed of updating the bash.bashrc (for the SDK), as I think it could improve it's logic and at least check if the $PS1 was not empty before overwriting it!

[dscho]

I'll still keep looking at how to find out which package did the dirty deed of updating the bash.bashrc (for the SDK), as I think it could improve it's logic and at least check if the $PS1 was not empty before overwriting it!

Sorry, I meant to clarify that earlier... It is a bit inelegant, as filesystem is a pretty core package and kind of wants to dictate that PS1 is set always. The only way I managed to fix that up was by editing the /etc/bash.bashrc file in git-extra's post-.install script: https://github.com/git-for-windows/build-extra/blob/fd41982e25/git-extra/git-extra.install#L61-L62

[PhilipOakley]

I'll still keep looking at how to find out which package did the dirty
deed of updating the bash.bashrc (for the SDK), as I think it could
improve it's logic and at least check if the $PS1 was not empty before
overwriting it!

Sorry, I meant to clarify that earlier... It is a bit inelegant, as
filesystem is a pretty core package and kind of wants to dictate that PS1
is set always. The only way I managed to fix that up was by editing the
/etc/bash.bashrc file in git-extra's post-.install script:
https://github.com/git-for-windows/build-extra/blob/fd41982e25/git-extra/git-extra.install#L61-L62

If I read that correctly, you are simply changing the $PS1=.... to
#$PS1=.... in the default filesystem's /etc/bash.bashrc, which is the
right thing for it's original version.

What I'm seeing (but not understanding) is that by the time the full SDK has
been installed, the /etc/bash.bashrc file has been further
patched/modified/overwritten by some other package (or whatever), so that it
now has a totally new conditional (for the SDK) to try and set the PS1,
based on expecting that "MSYS2_PS1" is set.

I'm thinking that maybe I need to add a trigger/hook to the pacman stuff
such that before / after every package installs it checks to see if that
file has been changed, just to nail down where it happens. It's a bit
slowly, slowly catchee monkey at the moment.

Philip

[dscho]

Heh. I should clarify.

A previous filesystem package had an /etc/bash.bashrc that did set PS1=... outside any conditional. That is the reason why I try to match it at the beginning of the line (that is what the ^ means in regular expressions: the beginning of the line).

Now, I missed the fact that newer versions of the filesystem package changed that, because git-extra modifies the /etc/bash.bashrc file and therefore my local system kept working even if new SDK installations would be broken.

That means that I am very, very grateful for your report on this: I would have missed this breakage otherwise.

And finally, I think your MSYS_PS1="$PS1" at the end of git-prompt.sh is the absolute correct thing to do.

Well, maybe somebody wants to override the PS1 of MSYS2 by setting their own MSYS_PS1? But it is more likely to set PS1 in $HOME/.profile (by that time, setting MSYS_PS1 would be too late).

So I think your PR already fixes things appropriately.

A follow-up PR might want to remove the ^PS1 handling because it won't trigger anymore now.

[PhilipOakley]

@dscho, Thanks for that clarification. However we may be talking at cross purposes.

First can you confirm what you see in your bash.bashrc for the SDK. Note that I select the 'only use the bash for git' (the most safe ;-)

What I see in mine is

if test -n "${MSYS2_PS1}"
    then PS1="${MSYS2_PS1}"
    else PS1='\[\e]0;\w\a\]\n\[\e[32m\]\u@\h \[\e[35m\]Hi-$MSYSTEM\[\e[0m\] \[\e[33m\]\w\[\e[0m\]\n\$ '

Which is nothing like what's in the base filesystem package (the self extracting exe), which you modify for the normal G4W.

I was trying to find out how my bas.bashrc got that way. (and then patch the else case to elif testing for a missing PS1 before setting it ;-)

[dscho]

First can you confirm what you see in your bash.bashrc for the SDK.

Yes, I can see the same now.

Which is nothing like what's in the base filesystem package (the self extracting exe), which you modify for the normal G4W.

Actually, it is... see the output of

tar Oxvf /var/cache/packman/pkg/filesystem-2016.05-3-x86_64.pkg.tar.xz etc/bash.bashrc

I was trying to find out how my bas.bashrc got that way. (and then patch the else case to elif testing for a missing PS1 before setting it ;-)

I think the most logical update to git-extra.install would be to simply comment out the else PS1=... wholesale... ;-)

[PhilipOakley]

Odd that. I thought I'd looked at that file (the 32bit version) from the interweb and did not see it (the msys2_ps1 check) in the one I got.

I'll double check what I'm seeing just to make sure I'm looking at the right thing, and then consider an upstream report.

Goes and checks....
in my "C:\git-sdk-32\var\cache\pacman\pkg\filesystem-2016.05-2-i686.pkg.tar.xz" (thanks for the pointer I hadn't realised about the chache- doh!) I do see those line.

Not got time to check the web source just now (weekend away - yay)

[PhilipOakley]

Found it. I'd taken a wrong turn in drilling down the many layers and had ended up in the BASE side of the packages.
The offending lines are here https://github.com/Alexpux/MSYS2-packages/blob/master/filesystem/bash.bashrc#L46 and was introduced in 2nd May 2016 by msys2/MSYS2-packages@6e6310d

[dscho]

Thanks for digging!

[PhilipOakley]

I have posted an upstream patch as well msys2/MSYS2-packages#651

[PhilipOakley]

@dscho The upstream fix PhilipOakley/MSYS2-packages@4ce39dd is now in place!

[PhilipOakley]

The upstream fix PhilipOakley/MSYS2-packages@4ce39dd is now in place!
@dscho - Oops, that ("in place") is not true - it has not been merged upstream yet. I'd misread the github interface. sorry for the noise.

[dscho]

No worries! Thanks for the update!

[PhilipOakley]

@dscho I thought I'd done a fix (msys2/MSYS2-packages#651) to the upstream (worked for me), but apparently not, so a follow up fix msys2/MSYS2-packages#654 is now in place. And it appears to work for me (for the SDK).

Keep an eye out for any issues