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

[New msys2-runtime version] 19 new items #4009

Closed
git-for-windows-ci opened this issue Sep 6, 2022 · 1 comment
Closed

[New msys2-runtime version] 19 new items #4009

git-for-windows-ci opened this issue Sep 6, 2022 · 1 comment

Comments

@git-for-windows-ci
Copy link

[New msys2-runtime version]

    Handle ORIGINAL_PATH just like PATH

Handle ORIGINAL_PATH just like PATH

MSYS2 recently introduced that hack where the ORIGINAL_PATH variable is
set to the original PATH value in /etc/profile, unless previously set.
In Git for Windows' default mode, that ORIGINAL_PATH value is the used
to define the PATH variable explicitly.

So far so good.

The problem: when calling from inside an MSYS2 process (such as Bash) a
MINGW executable (such as git.exe) that then calls another MSYS2
executable (such as bash.exe), that latter call will try to re-convert
ORIGINAL_PATH after the previous call converted ORIGINAL_PATH from POSIX
to Windows paths. And this conversion may very well fail, e.g. when the
path list contains mixed semicolons and colons.

So let's just *force* the MSYS2 runtime to handle ORIGINAL_PATH in the
same way as the PATH variable (which conversion works, as we know).

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@7af6781

[New msys2-runtime version]

    POSIX-ify the SHELL variable

POSIX-ify the SHELL variable

When calling a non-MSys2 binary, all of the environment is converted from
POSIX to Win32, including the SHELL environment variable. In Git for
Windows, for example, `SHELL=/usr/bin/bash` is converted to
`SHELL=C:\Program Files\Git\usr\bin\bash.exe` when calling the `git.exe`
binary. This is appropriate because non-MSys2 binaries would not handle
POSIX paths correctly.

Under certain circumstances, however, `git.exe` calls an *MSys2* binary in
turn, such as `git config --edit` calling `vim.exe` unless Git is
configured to use another editor specifically.

Now, when this "improved vi" calls shell commands, it uses that $SHELL
variable *without quoting*, resulting in a nasty error:

C:\\Program: No such file or directory

Many other programs behave in the same manner, assuming that $SHELL does
not contain spaces and hence needs no quoting, unfortunately including
some of Git's own scripts.

Therefore let's make sure that $SHELL gets "posified" again when entering
MSys2 programs.

Earlier attempts by Git for Windows contributors claimed that adding
`SHELL` to the `conv_envvars` array does not have the intended effect.
These reports just missed that the `conv_start_chars` array (which makes
the code more performant) needs to be adjusted, too.

Note that we set the `immediate` flag to `true` so that the environment
variable is set immediately by the MSys2 runtime, i.e. not only spawned
processes will see the POSIX-ified `SHELL` variable, but the MSys2 runtime
*itself*, too.

This fixes #542,
#498, and
#468.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@ae6fcef

[New msys2-runtime version]

    install-libs: depend on the "toollibs"

install-libs: depend on the "toollibs"

Before symlinking libg.a, we need the symlink source `libmsys-2.0.a`: in
MSYS2, we copy by default (if we were creating Unix-style symlinks, the
target would not have to exist before symlinking, but when copying we do
need the source _right away_).

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@31b5e84

[New msys2-runtime version]

    docs: skip building texinfo and PDF files

docs: skip building texinfo and PDF files

The MSYS2 packages lack the infrastructure to build those.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@20f59b4

[New msys2-runtime version]

    Cygwin: make option for native inner link handling.

Cygwin: make option for native inner link handling.

This code has been causing issues with SUBST and mapped network drives,
so add an option (defaulted to on) which can be used to disable it where
needed. MSYS=nonativeinnerlinks

msys2/msys2-runtime@aa9dbc9

[New msys2-runtime version]

    kill: kill Win32 processes more gently

kill: kill Win32 processes more gently

This change is the equivalent to the change to the Ctrl+C handling we
just made.

Co-authored-by: Naveen M K naveen@syrusdark.website
Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@486c760

[New msys2-runtime version]

    Emulate GenerateConsoleCtrlEvent() upon Ctrl+C

Emulate GenerateConsoleCtrlEvent() upon Ctrl+C

This patch is heavily inspired by the Git for Windows' strategy in
handling Ctrl+C.

When a process is terminated via TerminateProcess(), it has no chance to
do anything in the way of cleaning up. This is particularly noticeable
when a lengthy Git for Windows process tries to update Git's index file
and leaves behind an index.lock file. Git's idea is to remove the stale
index.lock file in that case, using the signal and atexit handlers
available in Linux. But those signal handlers never run.

Note: this is not an issue for MSYS2 processes because MSYS2 emulates
Unix' signal system accurately, both for the process sending the kill
signal and the process receiving it. Win32 processes do not have such a
signal handler, though, instead MSYS2 shuts them down via
`TerminateProcess()`.

For a while, Git for Windows tried to use a gentler method, described in
the Dr Dobb's article "A Safer Alternative to TerminateProcess()" by
Andrew Tucker (July 1, 1999),
http://www.drdobbs.com/a-safer-alternative-to-terminateprocess/184416547

Essentially, we injected a new thread into the running process that does
nothing else than running the ExitProcess() function.

However, this was still not in line with the way CMD handles Ctrl+C: it
gives processes a chance to do something upon Ctrl+C by calling
SetConsoleCtrlHandler(), and ExitProcess() simply never calls that
handler.

So for a while we tried to handle SIGINT/SIGTERM by attaching to the
console of the command to interrupt, and generating the very same event
as CMD does via GenerateConsoleCtrlEvent().

This method *still* was not correct, though, as it would interrupt
*every* process attached to that Console, not just the process (and its
children) that we wanted to signal. A symptom was that hitting Ctrl+C
while `git log` was shown in the pager would interrupt *the pager*.

The method we settled on is to emulate what GenerateConsoleCtrlEvent()
does, but on a process by process basis: inject a remote thread and call
the (private) function kernel32!CtrlRoutine.

To obtain said function's address, we use the dbghelp API to generate a
stack trace from a handler configured via SetConsoleCtrlHandler() and
triggered via GenerateConsoleCtrlEvent(). To avoid killing each and all
processes attached to the same Console as the MSYS2 runtime, we modify
the cygwin-console-helper to optionally print the address of
kernel32!CtrlRoutine to stdout, and then spawn it with a new Console.

Note that this also opens the door to handling 32-bit process from a
64-bit MSYS2 runtime and vice versa, by letting the MSYS2 runtime look
for the cygwin-console-helper.exe of the "other architecture" in a
specific place (we choose /usr/libexec/, as it seems to be the
convention for helper .exe files that are not intended for public
consumption).

The 32-bit helper implicitly links to libgcc_s_dw2.dll and
libwinpthread-1.dll, so to avoid cluttering /usr/libexec/, we look for
the helped of the "other" architecture in the corresponding mingw32/ or
mingw64/ subdirectory.

Among other bugs, this strategy to handle Ctrl+C fixes the MSYS2 side of
the bug where interrupting `git clone https://...` would send the
spawned-off `git remote-https` process into the background instead of
interrupting it, i.e. the clone would continue and its progress would be
reported mercilessly to the console window without the user being able
to do anything about it (short of firing up the task manager and killing
the appropriate task manually).

Note that this special-handling is only necessary when *MSYS2* handles
the Ctrl+C event, e.g. when interrupting a process started from within
MinTTY or any other non-cmd-based terminal emulator. If the process was
started from within `cmd.exe`'s terminal window, child processes are
already killed appropriately upon Ctrl+C, by `cmd.exe` itself.

Also, we can't trust the processes to end it's subprocesses upon receiving
Ctrl+C. For example, `pip.exe` from `python-pip` doesn't kill the python
it lauches (it tries to but fails), and I noticed that in cmd it kills python
also correctly, which mean we should kill all the process using
`exit_process_tree`.

Co-authored-by: Naveen M K naveen@syrusdark.website
Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@be43d13

[New msys2-runtime version]

    Add a helper to obtain a function's address in kernel32.dll

Add a helper to obtain a function's address in kernel32.dll

In particular, we are interested in the address of the CtrlRoutine
and the ExitProcess functions. Since kernel32.dll is loaded first thing,
the addresses will be the same for all processes (matching the
CPU architecture, of course).

This will help us with emulating SIGINT properly (by not sending signals
to *all* processes attached to the same Console, as
GenerateConsoleCtrlEvent() would do).

Co-authored-by: Naveen M K naveen@syrusdark.website
Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@22f7709

[New msys2-runtime version]

    Do not show Error dialogs by default

Do not show Error dialogs by default

In msys2/msys2-runtime#18, we discussed a change
that would allow default Windows error handling of spawned processes to
kick in (such as registered JIT debuggers). We even agreed that it would
make sense to hide this functionality behind a flag, `winjitdebug`.

However, when this got upstreamed as 21ec498d7f (cygwin: use
CREATE_DEFAULT_ERROR_MODE in spawn, 2020-12-09), that flag was deemed
unnecessary.

But it would appear that it _is_ necessary: As reported in
msys2/MSYS2-packages#2414 (comment)
this new behavior is pretty disruptive e.g. in CI scenarios.

So let's introduce that `winjitdebug` flag (settable via the environment
variable `MSYS`) at long last.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@c20ec3a

[New msys2-runtime version]

    Disable the 'cygwin' GitHub workflow

Disable the 'cygwin' GitHub workflow

It does not work at all. For example, `rpm -E %fedora` says that there
should be version 33 of rpmsphere at
https://github.com/rpmsphere/noarch/tree/master/r, but there is only
version 32.

Another thing that is broken: Cygwin now assumes that a recent
mingw-w64-headers version is available, but Fedora apparently only
offers v7.0.0, which is definitely too old to accommodate for the
expectation of cygwin/cygwin@c1f7c4d1b6d7.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@991789a

[New msys2-runtime version]

    Expose full command-lines to other Win32 processes by default

Expose full command-lines to other Win32 processes by default

In the Cygwin project, it was decided that the command-line of Cygwin
processes, as shown in the output of `wmic process list`, would suffer
from being truncated to 32k (and is transmitted to the child process via
a different mechanism, anyway), and therefore only the absolute path of
the executable is shown by default.

Users who would like to see the full command-line (even if it is
truncated) are expected to set `CYGWIN=wincmdln` (or, in MSYS2's case,
`MSYS=wincmdln`).

Seeing as MSYS2 tries to integrate much better with the surrounding
Win32 ecosystem than Cygwin, it makes sense to turn this on by default.

Users who wish to suppress it can still set `MSYS=nowincmdln`.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@b37c67a

[New msys2-runtime version]

    Set up a GitHub Action to keep in sync with Cygwin

Set up a GitHub Action to keep in sync with Cygwin

This will help us by automating an otherwise tedious task.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@51766ab

[New msys2-runtime version]

    CI: add a GHA for doing a basic build test

CI: add a GHA for doing a basic build test

msys2/msys2-runtime@69029b1

[New msys2-runtime version]

    popen: call /usr/bin/sh instead of /bin/sh

popen: call /usr/bin/sh instead of /bin/sh

We mount /usr/bin to /bin, but in a chroot this is broken and we
have no /bin, so try to use the real path.

chroot is used by pacman to run install scripts when called with --root
and this broke programs in install scripts calling popen()
(install-info from texinfo for example)

There are more paths hardcoded to /bin in cygwin which might also be broken
in this scenario, so this maybe should be extended to all of them.

msys2/msys2-runtime@f9b94aa

[New msys2-runtime version]

    Introduce the `enable_pcon` value for `MSYS`

Introduce the `enable_pcon` value for `MSYS`

It is simply the negation of `disable_pcon`, i.e. `MSYS=enable_pcon` is
equivalent to `MSYS=nodisable_pcon` (the former is slightly more
intuitive than the latter) and likewise `MSYS=noenable_pcon` is
equivalent to `MSYS=disable_pcon` (here, the latter is definitely more
intuitive than the former).

This is needed because we just demoted the pseudo console feature to be
opt-in instead of opt-out, and it would be awkward to recommend to users
to use "nodisable_pcon"... "nodisable" is not even a verb.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@a4a2aeb

[New msys2-runtime version]

    Default to `disable_pcon`

Default to `disable_pcon`

The pseudo console support is just a bit too buggy still:

  • When typing anything in an interactive Bash session before the prompt
    is shown, frequently the keystrokes are then replayed _twice_ when the
    prompt is active.

    Even worse: it seems that under certain circumstances (e.g. when
    spawning `less.exe` from a MINGW process), keystrokes are recorded
    while a process is active that wants to consume them but those
    recorded keystrokes are then replayed once the process finished (e.g.
    the `q` keystroke to exit `less.exe` will then be misinterpreted for
    interactive input in the Bash session).

  • When `vim` is called from a MINGW process, it seems that the terminal
    loses the `onlcr` property after the `vim` process finished, i.e.
    subsequently printed lines do not start at the beginning of the line
    anymore, but precisely where the previous line ended.

  • In `vim`, when selecting text visually (via the `v` keystroke), it
    seems that the selection is frequently reset while navigating with the
    arrow keys.

There are probably quite a few more rough edges in the pseudo console
feature, unfortunately.

In light of these issues, it makes most sense to disable the pseudo
console support and make it opt-in rather than opt-out.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@8e89fff

[New msys2-runtime version]

    strace --quiet: be *really* quiet

strace --quiet: be *really* quiet

The biggest problem with strace spitting out `create_child: ...` despite
being asked to be real quiet is that its output can very well interfere
with scripts' operations.

For example, when running any of Git for Windows' shell scripts with
`GIT_STRACE_COMMANDS=/path/to/logfile` (which is sadly an often needed
debugging technique while trying to address the many MSYS2 issues Git for
Windows faces), any time the output of any command is redirected into a
variable, it will include that `create_child: ...` line, wreaking havoc
with Git's expectations.

So let's just really be quiet when we're asked to be quiet.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

msys2/msys2-runtime@09db143

[New msys2-runtime version]

    QueryUnbiasedInterruptTime must be load from kernel32.dll

QueryUnbiasedInterruptTime must be load from kernel32.dll

msys2/msys2-runtime@b0485d4

[New msys2-runtime version]

    Fix native symbolic link spawn passing wrong arg0

Fix native symbolic link spawn passing wrong arg0

msys2/msys2-runtime@3578452

@dscho
Copy link
Member

dscho commented Sep 6, 2022

Already done, as part of #4008.

@dscho dscho closed this as completed Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants