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

Regressions with the terminal alternate screen in msys2-runtime 3.3.6-5 #185

Open
Geoffrey-A opened this issue Dec 29, 2023 · 13 comments
Open

Comments

@Geoffrey-A
Copy link

Geoffrey-A commented Dec 29, 2023

There are 2 regressions I noticed after a pacboy update, which are still observable in the latest msys2-runtime 3.4.10-1:

  1. git help X does not show the man page for X anymore. I have to resort to typing man git-X.
  2. LESS=R git show does not start the less pager in an alternate screen anymore. Because less correctly clears the text on exit, it results in blank vertical space in the terminal, corresponding to the number of lines of the screen that less used for printing content. I can still get the original desired behaviour by typing git show --color | less -R.

I have installed Git for Windows inside MSYS2, as per https://github.com/git-for-windows/git/wiki/Install-inside-MSYS2-proper.

I initially thought these regressions were caused by less, mintty, or git, but finally found out that reverting the single msys2-runtime package to 3.3.6-4 fixes both issues. Beyond the fact that they were introduced at the same time, I am now unsure of how related they are.

Indeed, from the git source code I reduced the problem 1 to the following:

#include <unistd.h>
int main(int, char**) { return execlp("man", "man", "man", (char*)NULL); }

which seems not to do anything from version 3.3.6-5 of the runtime, despite exiting with return code 0. But, replacing all occurrences of man with printf in the above produces no output either, wheareas it does print printf in version 3.3.6-4. So it seems unrelated to alternate screens after all. Please let me know if I should create 2 distinct issues.

I thought this problem could have been introduced in the Cygwin upstream. But after installing it, recompiling and running the snippet of code produced an execution of man man as expected with the cygwin package versions from 3.4.10 down to 3.4.7. I had suspected some commits in Dec 2021 or Jan 2022 which affect the cygwin pty or console. Maybe it is that some msys patches did not play well with such changes.

As for problem 2, I did not manage to get a reduction. I would tend to think that the function mingw_spawnvpe, used to spawn the less pager process and implemented in Git’s source compat/mingw.c, is a likely culprit. That would relate the two issues to process functions. But given that mingw_spawnvpe looks to be implemented in terms of the Windows API, I do not see how a msys runtime change could break it, so that seems to contradict my hypothesis.

What I found amusing about it is that running the equivalent command in PowerShell works as expected:

$env:LESS='R'; git show; $env:LESS=$null
@Biswa96
Copy link
Member

Biswa96 commented Dec 29, 2023

  1. git help X does not show the man page for X anymore.
  2. LESS=R git show does not start the less pager in an alternate screen anymore.

I can not reproduce any of those issues in msys2 setup.

I have installed Git for Windows inside MSYS2

That page clearly warns that "Please note that this scenario is not officially supported by Git for Windows"1

Please install git-for-windows and msys2 separately if you want both in your system.

Footnotes

  1. https://github.com/git-for-windows/git/wiki/Install-inside-MSYS2-proper#please-note-that-this-scenario-is-not-officially-supported-by-git-for-windows

@Geoffrey-A
Copy link
Author

Geoffrey-A commented Jan 5, 2024

I could observe the bad behaviour with vanilla MSYS2, without git installed.

I used the latest installer (msys2-base-x86_64-20231026.tar.xz), here are my steps to setup the MSYS2 environment:

  • extract the archive
  • double click on mingw64.exe, which opens a terminal, and initialises MSYS2 as it’s its first run
  • update the system with the command pacman -Syu. We are prompted to close the terminal as base packages are updated.
  • double click on mingw64.exe again, and run pacman -Su to complete the update
  • install mingw64 gcc: pacman -S mingw-w64-x86_64-gcc

Now create the executable:

gcc -o bad_exec -W{all,extra} -include unistd.h -xc -<<<'int main(int,char**a){return*++a?execlp(*a,*a,*a,(char*)0):-1;}'

Running ./bad_exec printf does not print printf, on the first execution. Running it a few more times will sometimes print it, sometimes not. When I execute for i in {1..10}; { ./bad_exec printf; }, I get between 2 and 4 printf strings printed on screen (on the same line, which is expected).

Now the interesting bit is that if I downgrade the runtime to 3.3.6-4, by manually downloading https://repo.msys2.org/msys/x86_64/msys2-runtime-3.3.6-4-x86_64.pkg.tar.zst and pacman -U /path/to/file, then the issue is still not fixed. However, if I instead install the GFW version of that same runtime, which I found at https://wingit.blob.core.windows.net/x86-64/msys2-runtime-3.3.6-4-x86_64.pkg.tar.xz, then bad_exec.exe behaves as expected.
I did need to run sed '/^LocalFileSigLevel\b/{h;s/^/#/;p;g;s/=.*/= Never/}' -i /etc/pacman.conf before installing the GFW runtime since the corresponding signing key was not present on the system.

@Biswa96
Copy link
Member

Biswa96 commented Jan 5, 2024

Running ./bad_exec printf does not print printf, on the first execution

Again, I can not reproduce the issue with msys2. Here is a screenshot showing the first execution.

image

By the way, the latest msys2 runtime package is 3.4.10-2.

$ pacman -Q msys2-runtime
msys2-runtime 3.4.10-2

@Geoffrey-A
Copy link
Author

Running ./bad_exec printf does not print printf, on the first execution

Again, I can not reproduce the issue with msys2. Here is a screenshot showing the first execution.

I used the MINGW64 environment, not UCRT64.

@Biswa96
Copy link
Member

Biswa96 commented Jan 5, 2024

I used the MINGW64 environment, not UCRT64.

I can not reproduce the issue in MINGW64 also. There is no difference in the result. Here is the screenshot using MINGW64.

image

@Geoffrey-A
Copy link
Author

What is your OS/uname?

My main machine is on Windows 11. I just tried to reproduce on 2 VMs, one win 10, one win 11. I only reproduced a single time on win 10, with 9 strings printed out of 10, across many executions of the for loop command. On win 11 however I could reproduce the problem easily.

I followed the steps I provided above, installing a clean and up-to-date MSYS2, and confirmed the installed msys2-runtime package is version 3.4.10-2.

@Biswa96
Copy link
Member

Biswa96 commented Jan 5, 2024

I have tried the commands both in Windows 10 and Windows 11 latest version (dual boot and bare metal installation). The command shows same output in 10 or 11.

@elieux
Copy link
Member

elieux commented Jan 5, 2024

  1. I cannot reproduce the git help thing since it always wants to show the HTML docs. Not sure how to switch to man docs.
  2. I can reproduce the git show thing (with LESS=R always or without it if the listing does not fit the screen).
  3. I cannot reproduce the bad_exec thing.

I had a go at isolating the issue and it seems to be somewhere in the tty translation because:

  1. LESS=R echo hello | less works properly.
  2. LESS=R echo hello | cmd //c less doesn't work properly. This is like Git because it goes msys → non-msys → msys in the process tree.

@Geoffrey-A
Copy link
Author

Geoffrey-A commented Jan 6, 2024

1. I cannot reproduce the git help thing since it always wants to show the HTML docs. Not sure how to switch to man docs.

You are using GFW installed inside MSYS2 proper aren’t you? If so, I described the procedure in the wiki page:

To be able to view the git man pages when invoking help with git help X or git X --help (in addition to man git-X), add the line export MSYS2_ENV_CONV_EXCL=MANPATH to your shell configuration, and set the man pages as default help format with git config --global help.format man (or append -m to the git help invocation).

If you don’t want to affect your config, you can do MSYS2_ENV_CONV_EXCL=MANPATH git help -m X. The setting of the environment variable is to avoid the error “No manual entry for git-X”. When it is the msys git that is installed, then I believe that this error does not occur, and that the default is the man pages.

2. I can reproduce the git show thing (with LESS=R always or without it if the listing does not fit the screen).

I don’t see any issue on my end with git show if the listing is larger than the screen. Is the screen cleared?

3. I cannot reproduce the bad_exec thing.

What is your uname?

I can reproduce on MINGW64_NT-10.0-22631, MINGW64_NT-10.0-22000, but hardly on MINGW64_NT-10.0-19045. It appears random but does happen. It seems more prone to occur on some kind of an initial/reset state; I am unsure what causes that state, but I feel like closing and reopening the terminal helps. A few commands that might bring more luck:

# preliminary: create a cat file
echo ok >cat
# should print `10 ok`
for i in {1..10}; { ./bad_exec cat; } | uniq -c
# should print `10 ls: cannot access 'ls': No such file or directory`, unless you have an ls file
for i in {1..10}; { ./bad_exec ls; } 2>&1 | uniq -c
# should print `10 printf` -- same test as before
for i in {1..10}; { ./bad_exec printf; } | grep -o printf | uniq -c

@Geoffrey-A
Copy link
Author

Geoffrey-A commented Jan 6, 2024

  • LESS=R echo hello | less works properly.

  • LESS=R echo hello | cmd //c less doesn't work properly. This is like Git because it goes msys → non-msys → msys in the process tree.

I confirm these observations on my end too. This seems to be the best way to demonstrate the problem. As an aside, here, the environment variable is only set on the left hand side of the pipe, so the LESS=R part can be omitted and the commands would still produce the same results.

Note that GFW’s msys2-runtime 3.3.6-4 fixes this problem too.

@Geoffrey-A
Copy link
Author

Another issue is MSYS2_ENV_CONV_EXCL=MANPATH ./bad_exec man: either a) it does not do anything (same behaviour that I reported with the earlier ./bad_exec commands) or b) the first screen of the manual for man is printed, but man seems to terminate as the control returns to the terminal without clearing; it is not possible to view the content after the first screen or navigate/operate man.

@Geoffrey-A
Copy link
Author

Geoffrey-A commented Jan 8, 2024

The issue with seq 5 | cmd //c less is reproducible in GFW alone. I observed that PortableGit-2.40.1-64-bit.7z.exe was good, but PortableGit-2.41.0-64-bit.7z.exe was not. That corresponds to when msys-2.0.dll was upgraded from 3.3.6-bec3d608e to 3.4.6-b4fed42af.

The latest version, PortableGit-2.43.0-64-bit.7z.exe, also has the issue. But Git-2.43.0-64-bit.exe, which is an other installer artefact of the same 2.43.0 release, seems to work fine!

Now, going back to MSYS2 proper, I narrowed down where the bug was introduced: between msys2-runtime-3.3.6-1-x86_64.pkg.tar.zst (good) and msys2-runtime-3.3.6-2-x86_64.pkg.tar.zst (bad). Guessing from the build date, I suspect this commit: msys2/MSYS2-packages@3dad035.

In my first post on this issue, I mixed up the msys2-runtime packages for MSYS2 proper (currently distributed as tar.zst) and those obtained from the GFW repo (tar.xz). There appears to be no 3.3.6-5 on GFW side. The last good version of the package on GFW is 3.3.6-4 as previously indicated, but the first bad one is 3.4.6-1. It would seem as though the regression was first introduced in MSYS2, then 9 months later got into GFW. Looking at the range of commits found in the file properties of msys-2.0.dll, I get this huge diff: git-for-windows/msys2-runtime@bec3d60...82f0d82. It contains most of the changes from the above MSYS2-proper commit, though the actual commits are slightly different: git-for-windows/msys2-runtime@de71536, and the dropping of 8e89fff (as opposed to being reverted, as in the above).

As an aside, GFW’s msys-2.0.dll has, in its file properties’ product version, the git revision hash for the commit in the GFW msys2-runtime repo it was built from. That is very helpful, and I would suggest MSYS2 also do the same. I think it was added in this commit: git-for-windows/msys2-runtime@dbd0a16

This disable_pcon change seemed very related. And I found this documentation page: https://cygwin.com/cygwin-ug-net/using-cygwinenv.html. So, I just gave it a try, starting my terminal as MSYS=disable_pcon mingw.exe and it works! The fix appears to be to just set an other environment variable.

I want to mention that I very much appreciate dscho’s detailed commit messages.

@Geoffrey-A
Copy link
Author

I confirmed that setting the disable_pcon option flag fixed all the issues reported in this ticket.

The latest version, PortableGit-2.43.0-64-bit.7z.exe, also has the issue. But Git-2.43.0-64-bit.exe, which is an other installer artefact of the same 2.43.0 release, seems to work fine!

The reason for that is that, although msys-2.0.dll and git-bash.exe are binary identical in both artefacts, the installer one creates a file /etc/git-bash.config while the portable one does not. The content of that file is the single line MSYS=disable_pcon. I confirmed that running echo MSYS=disable_pcon >>/etc/git-bash.config in the portable git installation and restarting git-bash.exe fixes the issue.

As for my setup, in MSYS2, I needed to add that line to mingw64.ini.

Perhaps the most surprising of all issues fixed, because it seems unrelated to the console, was the fact that the C function execlp randomly failed, in the bad_exec sample. One thing I did not mention when describing this problem is that Windows terminals popped up and closed in a split second when executing, for example, for i in {1..10}; { ./bad_exec which; }. At one point I thought there was one terminal popping up per failed process execution, but that is not the case, sometimes they do not popup at all. I don’t think I had any terminal pop up in the cases where the execution succeeded though, so it might be an indication of a failure to start the process, but by itself is not enough to judge whether it failed or not. Although this problem appears not to have been replicated by others at this stage, I would think this is quite bad. So despite knowing the workaround, I am not closing this issue just yet to give it some visibility, and an opportunity for the maintainers to reconsider what the default value for disable_pcon should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants