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

msys2_path_conv: pass PC_NOFULL to path_conv #117

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

lazka
Copy link
Member

@lazka lazka commented Nov 20, 2022

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 #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 the 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.

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#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.
@lazka
Copy link
Member Author

lazka commented Nov 20, 2022

For completeness sake I've also checked that git-for-windows/git#1497 (comment) still works with this.

@dscho
Copy link
Collaborator

dscho commented Nov 20, 2022

With msys2/MSYS2-packages#3306 merged, we should rebase and merge this here PR, too, right @lazka?

With this change all our MSYS2 path_conv tests pass again.

I guess it would be a good idea to merge the test suite into this here repository, and then make it part of the CI. What do you think?

@lazka
Copy link
Member Author

lazka commented Nov 20, 2022

With msys2/MSYS2-packages#3306 merged, we should rebase and merge this here PR, too, right @lazka?

Please ignore the fact that it's merged, I just wanted to unbreak the gcc build and do some testing with staging.

With this change all our MSYS2 path_conv tests pass again.

I guess it would be a good idea to merge the test suite into this here repository, and then make it part of the CI. What do you think?

Yes, I happened to create an issue related to this just now: msys2/msys2-tests#24

@lazka
Copy link
Member Author

lazka commented Dec 2, 2022

There hasn't been any more fallout with this backported, so I think it is good to merge.

@lazka lazka merged commit 68060d2 into msys2:msys2-3_3_6-release Dec 2, 2022
dscho added a commit to dscho/msys2-runtime that referenced this pull request May 12, 2023
The long-delayed upgrade of Git for Windows' fork of the MSYS2 runtime
to v3.4.* is finally here. And it comes with quite a few changes. The
following two patches made it into the MSYS2 runtime proper:

- 589fac6 (Fix incorrect path conversion (trailing slash),
  2018-03-14) as msys2/msys2-runtime#103 and
  msys2/msys2-runtime#117

- 3e9c003 (Pass environment variables with empty values, 2015-02-18)
  as msys2/msys2-runtime#101

The following two patches made it even further upstream, into Cygwin
proper:

- fa95548 (Implicitly support the /dev/fd symlink and friends,
  2022-02-20), in Cygwin as 4ec0889 (Cygwin: Implicitly support the
  /dev/fd symlink and friends, 2022-02-21)

- 4704d29 (dumper: avoid linker problem when `libbfd` depends on
  `libsframe`, 2023-01-31) was upstreamed as 9acb704afe (dumper: avoid
  linker problem when `libbfd` depends on `libsframe`, 2023-01-31)

Finally, we backported the commit 4d62197 (Cygwin: console: Fix
hangup of less on quit after the window is resized., 2022-12-22), so
naturally it is no longer necessary to carry it in Git for Windows'
fork.

This commit starts the rebase of 353f6b9 to 8dd31e7d1a

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
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.

None yet

2 participants