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

Rebase to Cygwin v3.5.7 #85

Merged
merged 74 commits into from
Jan 30, 2025
Merged

Rebase to Cygwin v3.5.7 #85

merged 74 commits into from
Jan 30, 2025

Conversation

dscho
Copy link
Member

@dscho dscho commented Jan 30, 2025

git range-diff origin/main^{/^Merge.branch.*msys}..origin/main HEAD^{/^Merge.branch.*msys}..
$ diff -u <(git diff cygwin-3.5.6 cygwin-3.5.7) <(git diff origin/main rebase-to-v3.5.7 --)
--- /dev/fd/63  2025-01-30 08:58:41.051202788 +0100
+++ /dev/fd/62  2025-01-30 08:58:41.051202788 +0100
@@ -1,5 +1,5 @@
 diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
-index 0d1b17b2e9..78fbce0d7f 100644
+index 97878699ca..245f04ff7f 100644
 --- a/winsup/cygwin/include/cygwin/version.h
 +++ b/winsup/cygwin/include/cygwin/version.h
 @@ -11,7 +11,7 @@ details. */
@@ -40,10 +40,10 @@
    /* Like is_fs_special but excluding local sockets. */
    int is_lnk_special () const {return is_fs_special () && !issocket ();}
 diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
-index e6f4c3e3de..599809f941 100644
+index 66a4f485a0..38549eaaee 100644
 --- a/winsup/cygwin/path.cc
 +++ b/winsup/cygwin/path.cc
-@@ -1233,10 +1233,7 @@ path_conv::check (const char *src, unsigned opt,
+@@ -1241,10 +1241,7 @@ path_conv::check (const char *src, unsigned opt,

          /* FIXME: bad hack alert!!!  We need a better solution */
          if (!strncmp (path_copy, MQ_PATH, MQ_LEN) && path_copy[MQ_LEN])

This fixes git-for-windows/git#5387

github-cygwin and others added 30 commits January 26, 2025 20:12
Signed-off-by: Corinna Vinschen <[email protected]>
Message queues are basically just files and in most cases can be handled
like normal files.  So it was a mistake to set the "on-disk-device" flag
for them to fix the mq_unlink() problem reported in
https://cygwin.com/pipermail/cygwin/2025-January/257119.html

Rather, given that unlink() just checks if the object to be deleted
has an on-disk representation, make sure message queues are added to
the path_conv::isondisk() predicate.

This also reverts commit d870655.

Fixes: d870655 ("Cygwin: path_conv: set on-disk-device flag for message queue files")
Reported-by: Christian Franke <[email protected]>
Signed-off-by: Corinna Vinschen <[email protected]>
(cherry picked from commit b940faa)
Deadlocks have been observed if the message queue functions are called
from different threads in the same process.

Remove incorrectly locking the descriptor table while accessing the
message queue fhandler, potentially calling blocking functions.

Fixes: 46f3b0c ("Cygwin: POSIX msg queues: move all mq_* functionality into fhandler_mqueue")
Reported-by: Christian Franke <[email protected]>
Signed-off-by: Corinna Vinschen <[email protected]>
(cherry picked from commit fe6ddc1)
Cygwin's speclib doesn't handle dashes or dots. However, we are about to
rename the output file name from `cygwin1.dll` to `msys-2.0.dll`.

Let's preemptively fix up all the import libraries that would link
against `msys_2_0.dll` to correctly link against `msys-2.0.dll` instead.
…ent variables to Windows form for native Win32 applications.
…t without ACLs. - Can read /etc/fstab with short mount point format.
The new `winsymlinks` mode `deepcopy` (which is made the default) lets
calls to `symlink()` create (deep) copies of the source file/directory.

This is necessary because unlike Cygwin, MSYS2 does not try to be its
own little ecosystem that lives its life separate from regular Win32
programs: the latter have _no idea_ about Cygwin-emulated symbolic links
(i.e. system files whose contents start with `!<symlink>\xff\xfe` and
the remainder consists of the NUL-terminated, UTF-16LE-encoded symlink
target).

To support Cygwin-style symlinks, the new mode `sysfile` is introduced.

Co-authored-by: Johannes Schindelin <[email protected]>
With MSys1, it was necessary to set the TERM variable to "msys". To
allow for a smooth transition from MSys1 to MSys2, let's simply handle
TERM=msys as if the user had not specified TERM at all and wanted us to
use our preferred TERM value.
Strace is a Windows program so MSYS2 will convert all arguments and environment vars and that makes debugging msys2 software with strace very tricky.
Commit message for this code was:

* strace.cc (create_child): Set CYGWIN=noglob when starting new process so that

  Cygwin will leave already-parsed the command line alonw."

I can see no reason for it and it badly breaks the ability to use
strace.exe to investigate calling a Cygwin program from a Windows
program, for example:
strace mingw32-make.exe
.. where mingw32-make.exe finds sh.exe and uses it as the shell.
The reason it badly breaks this use-case is because dcrt0.cc depends
on globbing to happen to parse commandlines from Windows programs;
irrespective of whether they contain any glob patterns or not.

See quoted () comment:
"This must have been run from a Windows shell, so preserve
 quotes for globify to play with later."
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 <[email protected]>
When converting `/c/` to `C:\`, the trailing slash is actually really
necessary, as `C:` is not an absolute path.

We must be very careful to do this only for root directories, though. If
we kept the trailing slash also for, say, `/y/directory/`, we would run
into the following issue: On FAT file systems, the normalized path is
used to fake inode numbers. As a result, `Y:\directory\` and
`Y:\directory` have different inode numbers!!!

This would result in very non-obvious symptoms. Back when we were too
careless about keeping the trailing slash, it was reported to the Git
for Windows project that the `find` and `rm` commands can error out on
FAT file systems with very confusing "No such file or directory" errors,
for no good reason.

During the original investigation, Vasil Minkov pointed out in
git-for-windows/git#1497 (comment),
that this bug had been fixed in Cygwin as early as 1997... and the bug
was unfortunately reintroduced into early MSYS2 versions.

Signed-off-by: Johannes Schindelin <[email protected]>
When calling `cygpath -u C:/msys64/` in an MSYS2 setup that was
installed into `C:/msys64/`, the result should be `/`, not `//`.

Let's ensure that we do not append another trailing slash if the
converted path already ends in a slash.

This fixes msys2/msys2-runtime#112

Signed-off-by: Johannes Schindelin <[email protected]>
In theory this doesn't make a difference because posix_to_win32_path()
is only called with rooted/absolute paths, but as pointed out in
msys2/msys2-runtime#103 PC_NOFULL will preserve
the trailing slash of unix paths (for some reason).

See "cygpath -m /bin/" (preserved) vs "cygpath -am /bin/" (dropped)

One use case where we need to trailing slashes to be preserved is the GCC build
system:
https://github.com/gcc-mirror/gcc/blob/6d82e0fea5f988e829912a/gcc/Makefile.in#L2314

The Makefile appends a slash to the prefixes and the C code doing relocation will
treat the path as a directory if there is a trailing slash. See
msys2/MINGW-packages#14173 for details.

With this change all our MSYS2 path_conv tests pass again.
When calling windows native apps from MSYS2, the runtime tries to
convert commandline arguments by a specific set of rules. This idea was
inherited from the MSys/MinGW project (which is now seemingly stale, yet
must be credited with championing this useful feature, see MinGW wiki
https://web.archive.org/web/20201112005258/http://www.mingw.org/wiki/Posix_path_conversion).

If the user does not want that behavior on a big scale, e.g. inside a
Bash script, with the changes introduced in this commit, the user can
now set the the environment variable `MSYS_NO_PATHCONV` when calling
native windows commands.

This is a feature that has been introduced in Git for Windows via
git-for-windows#11 and it predates
support for the `MSYS2_ENV_CONV_EXCL` and `MSYS2_ARG_CONV_EXCL`
environment variables in the MSYS2 runtime; Many users find the
simplicity of `MSYS_NO_PATHCONV` appealing.

So let's teach MSYS2 proper this simple trick that still allows using
the sophisticated `MSYS2_*_CONV_EXCL` facilities but also offers a
convenient catch-all "just don't convert anything" knob.

Signed-off-by: 마누엘 <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
Otherwise if globbing is allowed and we get called from a
Windows program, build_argv thinks we've been called from
a Cygwin program.
…spec

Reverts 25ba8f3. I can't figure out what
the intention was. I'm sure I'll find out soon enough when everything breaks.

This change means that input of:
  '"C:/test.exe SOME_VAR=\"literal quotes\""'

becomes:
  'C:/test.exe SOME_VAR="literal quotes"'

instead of:
  'C:/test.exe SOME_VAR=\literal quotes\'

.. which is at least consistent with the result for:
  '"no_drive_or_colon SOME_VAR=\"literal quotes\""'

The old result of course resulted in the quoted string being split into
two arguments at the space which is clearly not intended.

I *guess* backslashes in dos paths may have been the issue here?
If so I don't care since we should not use them, ever, esp. not at
the expense of sensible forward-slash-containing input.
Works very much like MSYS2_ARG_CONV_EXCL. In fact it uses the same
function, arg_heuristic_with_exclusions (). Also refactors parsing
the env. variables to use new function, string_split_delimited ().

The env. that is searched through is the merged (POSIX + Windows)
one. It remains to be seen if this should be made an option or not.

This feature was prompted because the R language (Windows exe) calls
bash to run configure.win, which then calls back into R to read its
config variables (LOCAL_SOFT) and when this happens, msys2-runtime
converts R_ARCH from "/x64" to an absolute Windows path and appends
it to another absolute path, R_HOME, forming an invalid path.
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 <[email protected]>
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.
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 <[email protected]>
Build with --disable-dependency-tracking because we only build once
and this saves 3-4 minutes in CI.
This will help us by automating an otherwise tedious task.

Signed-off-by: Johannes Schindelin <[email protected]>
dependabot bot and others added 19 commits January 30, 2025 08:51
Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
It came in real handy while debugging an issue that strace 'fixed'.

Signed-off-by: Johannes Schindelin <[email protected]>
When a non ascii char is at the beginning of a path the current conversion
destroys the path. This fix will prevent this with an extra check for
non-ascii UTF-8 characters.

Helped-by: Johannes Schindelin <[email protected]>
Signed-off-by: 마누엘 <[email protected]>
	* dcrt0.cc (dll_crt0_1), dtable.cc (handle_to_fn), environ.cc
	(environ_init, getwinenveq, build_env), external.cc
	(fillout_pinfo), fhandler_disk_file.cc (__DIR_mounts::eval_ino,
	fhandler_disk_file::readdir_helper), fhandler_netdrive.cc
	(fhandler_netdrive::readdir), fhandler_process.cc
	(format_process_winexename, format_process_maps,
	format_process_stat, format_process_status), fhandler_procsys.cc
	(fill_filebuf, fhandler_procsys::readdir), mount.cc
	(fs_info::update, mount_info::create_root_entry,
	mount_info::conv_to_posix_path, mount_info::from_fstab_line),
	nlsfuncs.cc (internal_setlocale), path.cc (path_conv::check,
	sysmlink_info::check_shortcut, symlink_info::check_sysfile,
	symlink_info::check_reparse_point,
	symlink_info::check_nfs_symlink, cygwin_conv_path,
	cygwin_conv_path_list, cwdstuff::get_error_desc, cwdstuff::get),
	strfuncs.cc (sys_wcstombs_no_path, sys_wcstombs_alloc_no_path),
	uinfo.cc (ontherange, fetch_from_path, cygheap_pwdgrp::get_home,
	cygheap_pwdgrp::get_shell, cygheap_pwdgrp::get_gecos), wchar.h
	(sys_wcstombs_no_path, sys_wcstombs_alloc_no_path): Convert call
	sites of the sys_wcstombs*() family to specify explicitly when the
	parameter refers to a path or file name, to avoid future
	misconversions.

Detailed explanation:

The sys_wcstombs() function contains special handling for paths/file
names, to work around file name restriction on Windows that are
unexpected in the POSIX context of Cygwin.

We actually do not want that special handling for WCS strings that
do *not* refer to paths or file names.

Neither do we want to convert those special file names unless they come
from inside Cygwin: if the source of the string value is the Windows API,
we *know* it cannot be such a special file name because Windows itself
would not be able to handle it in the way Cygwin does.

So let's switch the previous sys_wcstombs()/sys_wcstombs_no_path() (and
the *_alloc* variant) around to sys_wcstombs_path()/sys_wcstombs(). We do
this for several reasons:

- whenever a call site wants to convert a WCS representation of a path or
  file name to an MBS one, it should be made very clear that we *want* the
  special file name conversion to happen.

- it is shorter to read and write.

- future calls to sys_wcstombs() will not incur unwanted conversion by
  accident (it is easy for unsuspecting programmers to assume that the
  function name "sys_wcstombs()" refers to a regular text conversion that
  has nothing to do with paths or filenames).

By keeping the name sys_wcstombs() (and not switching to
sys_wcstombs_path()), the following call sites are implicitly changed to
*exclude* the special path/file name conversion:

cygheap.h (get_drive):
	Cannot contain special characters

external.cc (cygwin_internal):
	Refers to user/domain names, not paths

fhandler_clipboard.cc (fhandler_dev_clipboard::read):
	Is not a path or file name but characters from the Windows
	clipboard

fhandler_console.cc: (dev_console::con_to_str):
	Is not a path or file name but characters from the console

fhandler_registry.cc (encode_regname):
	Is a registry key, not a path or filename

fhandler_registry.cc (multi_wcstombs):
	All call sites pass registry values, not paths or filenames

fhandler_registry.cc (fstat):
	Is a registry value, not a path or filename

fhandler_registry.cc (fill_filebuf):
	Is a registry value, not a path or filename

net.cc (get_ipv4fromreg):
	Is a registry value, not a path or filename

net.cc (get_friendlyname):
	Is a device name, not a path or filename

netdb.cc (open_system_file):
	Is from outside Cygwin

smallprint.cc (__small_vsprintf):
	Is a free text, not a path or filename

strfuncs.cc (strlwr):
	Should preserve the characters from the private page if there
	are any

strfuncs.cc (strupr):
	Should preserve the characters from the private page if there
	are any

uinfo.cc (cygheap_user::init):
	Refers to a user name, not a path or filename

uinfo.cc (pwdgrp::fetch_account_from_windows):
	Refers to value from outside Cygwin

By keeping the function name sys_wcstombs_alloc() (and not changing it to
sys_wcstombs_alloc_path()), the following call sites are implicitly
changed to *exclude* the special path/file name conversion:

ldap.cc (cyg_ldap::remap_uid):
	Refers to a user name, not a path or filename

ldap.cc (cyg_ldap::remap_gid):
	Refers to a group name, not a path or filename

pinfo.cc (_pinfo::cmdline):
	Refers to a command line from Windows, outside Cygwin

uinfo.cc (cygheap_user::env_logsrv):
	Is a server name, not a path or filename

uinfo.cc (cygheap_user::env_domain):
	Refers to the user/domain name, not a path or filename

uinfo.cc (cygheap_user::env_userprofile):
	Refers to Windows' idea of a path, outside Cygwin

uinfo.cc (cygheap_user::env_systemroot):
	Refers to Windows' idea of a path, outside Cygwin

uinfo.cc (fetch_from_description):
	Refers to values from outside of Cygwin

uinfo.cc (cygheap_pwdgrp::get_gecos):
	Refers to user/domain name and email address, not path nor filename

Signed-off-by: Johannes Schindelin <[email protected]>
Windows native symlinks must match the type of their target (file or
directory), otherwise native Windows tools will fail. Creating symlinks in
'nativestrict' mode currently requires the target to exist in order to
check its type.

However, the target of a symlink can change at any time after the symlink
has been created. Thus users of native symlinks must be prepared to deal
with type mismatches anyway. Checking the target type at symlink creation
time is not a good reason to violate the symlink() API specification.

In 'nativestrict' mode, always create native symlinks. Choose the symlink
type according to the target if it exists. Otherwise check the target path
for a trailing '/' as hint to create a directory symlink.

This allows callers to explicitly specify the expected target type, e.g.:

  $ ln -s test/ link-to-test
  $ mkdir test

Signed-off-by: Karsten Blees <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
This is a forked repository...

Signed-off-by: Johannes Schindelin <[email protected]>
Internally, Cygwin already uses __utf8_mbtowc(), even if it still claims
to use the "ASCII" charset.

But the `MB_CUR_MAX` value (which is not actually a constant, but
dependent on the current locale) was still 1, which broke the initial
`globify()` call while parsing the the command-line in `build_argv()`
for non-ASCII arguments.

This fixes git-for-windows/git#2189

Signed-off-by: Johannes Schindelin <[email protected]>
Allow native symlinks to non-existing targets in 'nativestrict' mode
This might break things, but it turns out several Windows libraries like
to be loaded at 0x180000000.

This causes a problem, because `msys-2.0.dll` loads at `0x180040000` and
expects `0x180000000-0x180040000` to be available. A problem arises when
Antiviruses (or other DLL hooking mechanisms) load a DLL whose preferred
load address is `0x180000000` and fits in size before `0x180010000`:

1. `msys-2.0.dll` loads and fills `0x180010000-0x180040000` assuming no
   shared console structure is going to be needed.

2. Another DLL loads and fills `0x180000000-0x18000xxxx`

3. `msys-2.0.dll` tries to load `0x180000000-0x180010000` but it's not
   available. It falls back to another address, but down the line
   something else fails.

This bug triggers when using subshells (e.g.: `git clone --recursive`).

The MSYS2 runtime should be able to work around the address conflict,
but the code is failing in some way or other...

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: Mikael Larsson <[email protected]>
This topic branch fixes the problem where a UTF-16 command-line was
converted to UTF-8 in an incorrect way (because Cygwin treated it as if
it was a file name and applied some magic that is intended to allow for
otherwise invalid file names on Windows).

Signed-off-by: Johannes Schindelin <[email protected]>
Commit a5bcfe6 removed an optimization that fetches the
default group from the current user token, as it is sometimes
not accurate such as when groups like the builtin
Administrators group is the primary group.

However, removing this optimization causes extremely poor
performance when connected to some Active Directory
environments.

Restored this optimization as the default behaviour, and
added a `group: db-accurate` option to `nsswitch.conf` that
can be used to disable the optimization in cases where
accurate group information is required.

This fixes git-for-windows/git#4459

Signed-off-by: Richard Glidden <[email protected]>
Workaround certain anti-malware programs

Signed-off-by: Johannes Schindelin <[email protected]>
It was reported in git-for-windows/git#5199
that as of v3.5.4, cloning or fetching via SSH is hanging indefinitely.

Bisecting the problem points to 555afcb (Cygwin: select: set pipe
writable only if PIPE_BUF bytes left, 2024-08-18). That commit's
intention seems to look at the write buffer, and only report the pipe as
writable if there are more than one page (4kB) available.

However, the number that is looked up is the number of bytes that are
already in the buffer, ready to be read, and further analysis
shows that in the scenario described in the report, the number of
available bytes is substantially below `PIPE_BUF`, but as long as they
are not handled, there is apparently a dead-lock.

Since the old logic worked, and the new logic causes a dead-lock, let's
essentially revert 555afcb (Cygwin: select: set pipe writable only if
PIPE_BUF bytes left, 2024-08-18).

Note: This is not a straight revert, as the code in question has been
modified subsequently, and trying to revert the original commit would
cause merge conflicts. Therefore, the diff looks very different from the
reverse diff of the commit whose logic is reverted.

Signed-off-by: Johannes Schindelin <[email protected]>
msys2-runtime: restore fast path for current user primary group
One particularly important part of Git for Windows' MSYS2 runtime is
that it is used to run Git's tests, and regressions happened there: For
example, the first iteration of MSYS2 runtime v3.5.5 caused plenty of
hangs. This was realized unfortunately only after deploying the
msys2-runtime Pacman package, and some painful vacation-time scrambling
was required to revert to v3.5.4.This was realized unfortunately only
after deploying the msys2-runtime Pacman package, and some painful
vacation-time scrambling was required to revert to v3.5.4.

To verify that this does not happen anymore, let's reuse what
`setup-git-for-windows-sdk` uses in Git's very own CI:

- determine the latest successful `ci-artifacts` workflow run in
  git-for-windows/git-sdk-64

- download its Git files and build artifacts

- download its minimal-sdk

- overwrite the MSYS2 runtime in the minimal-sdk

- run the test suite and the assorted validations just like the
  `ci-artifacts` workflow (from which these jobs are copied)

This obviously adds a hefty time penalty (around 7 minutes!) to every
MSYS2 runtime PR in the git-for-windows org. Happily, these days we
don't need many of those, and the balance between things like the v3.5.5
scramble and waiting a little longer for the CI to finish is clearly in
favor of the latter.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented Jan 30, 2025

This addresses git-for-windows/git#5387

@rimrul that does not seem to link this here PR to that there issue. Something's thoroughly broken.

@rimrul
Copy link
Member

rimrul commented Jan 30, 2025

This addresses git-for-windows/git#5387

@rimrul that does not seem to link this here PR to that there issue. Something's thoroughly broken.

The keyword is fixes

@dscho
Copy link
Member Author

dscho commented Jan 30, 2025

The keyword is fixes

Huh. I could have sworn that "addresses" was part of this list of closing keywords, but apparently it isn't (and maybe never was?). Huh.

@dscho
Copy link
Member Author

dscho commented Jan 30, 2025

/open pr

The workflow run was started

@dscho dscho merged commit 882031d into git-for-windows:main Jan 30, 2025
21 checks passed
@dscho dscho deleted the rebase-to-v3.5.7 branch January 30, 2025 12:10
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

Successfully merging this pull request may close these issues.

[New cygwin version] cygwin-3.5.7