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

Backport Git for Windows' pathconv adjustments #182

Merged
merged 9 commits into from
Dec 19, 2023

Conversation

dscho
Copy link
Collaborator

@dscho dscho commented Dec 17, 2023

This PR aligns the MSYS2 runtime with the variant that Git for Windows uses. In every commit message, the test cases of Git's test suite are listed whose expectations would not be met by the MSYS2 runtime without the respective commit.

This PR is part of my effort to address msys2/MINGW-packages#16383.

@dscho dscho self-assigned this Dec 17, 2023
@dscho dscho force-pushed the git-for-windows-pathconv-adjustments branch 2 times, most recently from 1b0ab77 to 693c449 Compare December 18, 2023 08:20
@dscho dscho marked this pull request as ready for review December 18, 2023 08:48
@lazka
Copy link
Member

lazka commented Dec 18, 2023

lgtm

I'll try to extend our external tests with the examples provided in the commit messages once this lands.

edit: I've also confirmed that our existing tests still pass with this. (or let's say the changes behave as expected)

…nvironment variables to Windows form for native Win32 applications.

This teaches MSYS2's path conversion to leave arguments starting with a
tilde or quote alone: It is not a good idea to expand, say, ~/.gitconfig
partially: replacing it by ~C:\msys64\.gitconfig is most likely the
wrong thing to do!

This addresses the expectations of the following test cases in the test
suite of https://github.com/git/git/tree/v2.43.0:

-t0001.19 init with init.templatedir using ~ expansion
-t0003.12 core.attributesfile
-t0003.13 attribute test: read paths from stdin
-t0003.15 attribute test: --all option
-t0003.16 attribute test: --cached option
-t0003.17 root subdir attribute test
-t0003.18 negative patterns
-t0003.19 patterns starting with exclamation
-t0003.20 "**" test
-t0003.21 "**" with no slashes test
-t0003.23 using --source
-t0003.32 bare repository: check that --cached honors index
-t0003.34 binary macro expanded by -a
-t0003.35 query binary macro directly
-t0003.40 large attributes line ignored in tree
-t0003.41 large attributes line ignores trailing content in tree
-t0003.43 large attributes line ignored in index
-t0003.44 large attributes line ignores trailing content in index
-t0068.1 run based on configured value

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
…nvironment variables to Windows form for native Win32 applications.

Let's teach MSYS2's path conversion to leave Git's :name and :/message
arguments alone, please. These arguments start with colons and are hence
unlikely to contain a path list (a path list starting with a colon would
start with an empty item?!?).

Without this patch, you will see this:

$ GIT_TRACE=1 /c/Program\ Files/Git/cmd/git version :/message
13:48:44.258390 exec-cmd.c:244          trace: resolved executable dir: C:/Program Files/Git/mingw64/bin
13:48:44.269314 git.c:463               trace: built-in: git version ';C:\msys64\message'
git version 2.43.0.windows.1

In other words, the argument `:/message` is mangled in an undesired way.

This addresses the expectations of the following test cases in the test
suite of https://github.com/git/git/tree/v2.43.0:

-t0060.81 <drive-letter>:\\abc is an absolute path
-t1501.35 Auto discovery
-t1501.36 $GIT_DIR/common overrides core.worktree
-t1501.37 $GIT_WORK_TREE overrides $GIT_DIR/common
-t1506.3 correct relative file objects (0)
-t1506.8 correct relative file objects (5)
-t1506.9 correct relative file objects (6)
-t2070.7 restore --staged uses HEAD as source
-t3013.16 git ls-files --format with relative path
-t3400.6 rebase, with <onto> and <upstream> specified as :/quuxery
-t3404.84 rebase -i, with <onto> and <upstream> specified as :/quuxery
-t3703.2 add :/
-t3703.3 add :/anothersub
-t4202.8 oneline
-t4202.22 git log --no-walk <commits> sorts by commit time
-t4202.23 git log --no-walk=sorted <commits> sorts by commit time
-t4202.24 git log --line-prefix="=== " --no-walk <commits> sorts by commit time
-t4202.25 git log --no-walk=unsorted <commits> leaves list of commits as given
-t4202.26 git show <commits> leaves list of commits as given
-t4202.138 log --source paints branch names
-t4202.139 log --source paints tag names
-t4202.140 log --source paints symmetric ranges
-t4208.2 "git log :/" should not be ambiguous
-t4208.3 "git log :/a" should be ambiguous (applied both rev and worktree)
-t4208.4 "git log :/a -- " should not be ambiguous
-t4208.5 "git log :/detached -- " should find a commit only in HEAD
-t4208.7 "git log :/detached -- " should find HEAD only of own worktree
-t4208.10 "git log :/in" should not be ambiguous
-t4208.13 git log HEAD -- :/
-t5616.43 lazy-fetch in submodule succeeds
-t6132.7 t_e_i() exclude sub2 from sub
-t6132.14 m_p_d() exclude sub2 from sub
-t6132.19 grep --untracked PATTERN
-t6132.21 grep --untracked PATTERN :(exclude)*FILE
-t6133.6 :/*.t from a subdir dwims to a pathspec
-t7201.14 checkout to detach HEAD with :/message
-t9903.37 prompt - untracked files status indicator - untracked files
-t9903.39 prompt - untracked files status indicator - non-empty untracked dir
-t9903.40 prompt - untracked files status indicator - untracked files outside cwd
-t9903.44 prompt - untracked files status indicator - shell variable set with config enabled
-t9903.56 prompt - bash color pc mode - untracked files status indicator

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
…nvironment variables to Windows form for native Win32 applications.

With this change, MSYS2's path conversion leaves paths containing any
special characters alone.

This addresses the expectations of the following test cases in the test
suite of https://github.com/git/git/tree/v2.43.0:

-t1091.13 set sparse-checkout using builtin
-t1092.82 grep sparse directory within submodules
-t1402.56 ref name '*/foo' is valid with options --refspec-pattern
-t1402.57 ref name '*/foo' is valid with options --refspec-pattern --allow-onelevel
-t1402.59 ref name '*/foo' is valid with options --refspec-pattern --normalize
-t3001.25 ls-files with "**" patterns
-t3001.26 ls-files with "**" patterns and --directory
-t3070.423 wildmatch: match 'foo' '**/foo'
-t3070.425 iwildmatch: match 'foo' '**/foo'
-t3070.433 wildmatch: match 'XXX/foo' '**/foo'
-t3070.435 iwildmatch: match 'XXX/foo' '**/foo'
-t3070.437 pathmatch: match 'XXX/foo' '**/foo'
-t3070.439 ipathmatch: match 'XXX/foo' '**/foo'
-t3070.443 wildmatch: match 'bar/baz/foo' '**/foo'
-t3070.445 iwildmatch: match 'bar/baz/foo' '**/foo'
-t3070.447 pathmatch: match 'bar/baz/foo' '**/foo'
-t3070.449 ipathmatch: match 'bar/baz/foo' '**/foo'
-t3070.457 pathmatch: match 'bar/baz/foo' '*/foo'
-t3070.459 ipathmatch: match 'bar/baz/foo' '*/foo'
-t3070.467 pathmatch: match 'foo/bar/baz' '**/bar*'
-t3070.469 ipathmatch: match 'foo/bar/baz' '**/bar*'
-t3070.473 wildmatch: match 'deep/foo/bar/baz' '**/bar/*'
-t3070.475 iwildmatch: match 'deep/foo/bar/baz' '**/bar/*'
-t3070.477 pathmatch: match 'deep/foo/bar/baz' '**/bar/*'
-t3070.479 ipathmatch: match 'deep/foo/bar/baz' '**/bar/*'
-t3070.487 pathmatch: match 'deep/foo/bar/baz/' '**/bar/*'
-t3070.489 ipathmatch: match 'deep/foo/bar/baz/' '**/bar/*'
-t3070.493 wildmatch: match 'deep/foo/bar/baz/' '**/bar/**'
-t3070.495 iwildmatch: match 'deep/foo/bar/baz/' '**/bar/**'
-t3070.497 pathmatch: match 'deep/foo/bar/baz/' '**/bar/**'
-t3070.499 ipathmatch: match 'deep/foo/bar/baz/' '**/bar/**'
-t3070.513 wildmatch: match 'deep/foo/bar/' '**/bar/**'
-t3070.515 iwildmatch: match 'deep/foo/bar/' '**/bar/**'
-t3070.517 pathmatch: match 'deep/foo/bar/' '**/bar/**'
-t3070.519 ipathmatch: match 'deep/foo/bar/' '**/bar/**'
-t3070.527 pathmatch: match 'foo/bar/baz' '**/bar**'
-t3070.529 ipathmatch: match 'foo/bar/baz' '**/bar**'
-t3070.533 wildmatch: match 'foo/bar/baz/x' '*/bar/**'
-t3070.535 iwildmatch: match 'foo/bar/baz/x' '*/bar/**'
-t3070.537 pathmatch: match 'foo/bar/baz/x' '*/bar/**'
-t3070.539 ipathmatch: match 'foo/bar/baz/x' '*/bar/**'
-t3070.547 pathmatch: match 'deep/foo/bar/baz/x' '*/bar/**'
-t3070.549 ipathmatch: match 'deep/foo/bar/baz/x' '*/bar/**'
-t3070.553 wildmatch: match 'deep/foo/bar/baz/x' '**/bar/*/*'
-t3070.555 iwildmatch: match 'deep/foo/bar/baz/x' '**/bar/*/*'
-t3070.557 pathmatch: match 'deep/foo/bar/baz/x' '**/bar/*/*'
-t3070.559 ipathmatch: match 'deep/foo/bar/baz/x' '**/bar/*/*'
-t3070.633 wildmatch: match 'XXX/\' '*/\\'
-t3070.635 iwildmatch: match 'XXX/\' '*/\\'
-t3070.637 pathmatch: match 'XXX/\' '*/\\'
-t3070.639 ipathmatch: match 'XXX/\' '*/\\'
-t3070.763 wildmatch: match 'foo/bar/baz/to' '**/t[o]'
-t3070.765 iwildmatch: match 'foo/bar/baz/to' '**/t[o]'
-t3070.767 pathmatch: match 'foo/bar/baz/to' '**/t[o]'
-t3070.769 ipathmatch: match 'foo/bar/baz/to' '**/t[o]'
-t3070.1493 wildmatch: match 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
-t3070.1495 iwildmatch: match 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
-t3070.1497 pathmatch: match 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
-t3070.1499 ipathmatch: match 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
-t3070.1533 wildmatch: match 'foo/bba/arr' '*/*/*'
-t3070.1535 iwildmatch: match 'foo/bba/arr' '*/*/*'
-t3070.1537 pathmatch: match 'foo/bba/arr' '*/*/*'
-t3070.1539 ipathmatch: match 'foo/bba/arr' '*/*/*'
-t3070.1547 pathmatch: match 'foo/bb/aa/rr' '*/*/*'
-t3070.1549 ipathmatch: match 'foo/bb/aa/rr' '*/*/*'
-t3070.1553 wildmatch: match 'foo/bb/aa/rr' '**/**/**'
-t3070.1555 iwildmatch: match 'foo/bb/aa/rr' '**/**/**'
-t3070.1557 pathmatch: match 'foo/bb/aa/rr' '**/**/**'
-t3070.1559 ipathmatch: match 'foo/bb/aa/rr' '**/**/**'
-t3070.1583 wildmatch: match 'ab/cXd/efXg/hi' '*/*X*/*/*i'
-t3070.1585 iwildmatch: match 'ab/cXd/efXg/hi' '*/*X*/*/*i'
-t3070.1587 pathmatch: match 'ab/cXd/efXg/hi' '*/*X*/*/*i'
-t3070.1589 ipathmatch: match 'ab/cXd/efXg/hi' '*/*X*/*/*i'
-t3070.1593 wildmatch: match 'ab/cXd/efXg/hi' '**/*X*/**/*i'
-t3070.1595 iwildmatch: match 'ab/cXd/efXg/hi' '**/*X*/**/*i'
-t3070.1597 pathmatch: match 'ab/cXd/efXg/hi' '**/*X*/**/*i'
-t3070.1599 ipathmatch: match 'ab/cXd/efXg/hi' '**/*X*/**/*i'
-t5500.154 fetch-pack --diag-url ./[::1]:repo
-t5500.156 fetch-pack --diag-url ./[::1]:23:repo
-t5500.165 fetch-pack --diag-url [::1]:/~repo
-t5500.251 fetch-pack --diag-url ./[::1]:re:po
-t5500.253 fetch-pack --diag-url ./[::1]:23:re:po
-t5500.262 fetch-pack --diag-url [::1]:/~re:po
-t5500.348 fetch-pack --diag-url ./[::1]:re/po
-t5500.350 fetch-pack --diag-url ./[::1]:23:re/po
-t5500.358 fetch-pack --diag-url [::1]:re/po
-t5500.359 fetch-pack --diag-url [::1]:/~re/po
-t5601.63 clone [::1]:rep/home/project
-t5601.66 clone [::1]:/~repo
-t6018.17 rev-parse --exclude with --branches
-t6018.85 rev-list --exclude with --branches
-t6130.20 **/ works with --glob-pathspecs
-t7817.1 setup
-t7817.2 working tree grep honors sparse checkout
-t7817.3 grep searches unmerged file despite not matching sparsity patterns
-t7817.5 grep --recurse-submodules honors sparse checkout in submodule
-t7817.7 working tree grep does not search the index with CE_VALID and SKIP_WORKTREE
-t9902.55 __git_refs - full refs
-t9902.58 __git_refs - remote on local file system - full refs
-t9902.75 __git refs - excluding full refs

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
…nvironment variables to Windows form for native Win32 applications.

We do not perform tilde expansion in the MSys2 runtime; let's leave
paths containing '/~' intact for programs that want to expand such paths
themselves.

This addresses the expectations of the following test cases in the test
suite of https://github.com/git/git/tree/v2.43.0:

-t5500.163 fetch-pack --diag-url host:/~repo
-t5500.260 fetch-pack --diag-url host:/~re:po
-t5500.357 fetch-pack --diag-url host:/~re/po
-t5601.65 clone host:/~repo

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
…nvironment variables to Windows form for native Win32 applications.

This skips posix-to-windows conversion when '::' is seen: The substring
'::' most often found in an IPv6 address, never in a path (and only in
bogus path lists that contain empty elements).

This addresses the expectations of the following test cases in the test
suite of https://github.com/git/git/tree/v2.43.0:

-t0060.197 test_submodule_relative_url: (null) helper:://hostname/repo ../subrepo => helper:://hostname/subrepo
-t0060.198 test_submodule_relative_url: (null) helper:://hostname/repo ../../subrepo => helper:://subrepo
-t0060.199 test_submodule_relative_url: (null) helper:://hostname/repo ../../../subrepo => helper::/subrepo
-t0060.200 test_submodule_relative_url: (null) helper:://hostname/repo ../../../../subrepo => helper::subrepo
-t0060.201 test_submodule_relative_url: (null) helper:://hostname/repo ../../../../../subrepo => helper:subrepo
-t0060.202 test_submodule_relative_url: (null) helper:://hostname/repo ../../../../../../subrepo => .:subrepo
-t5801.2 cloning from local repo
-t5801.4 pulling from local repo
-t5801.5 pushing to local repo
-t5801.6 fetch new branch
-t5801.7 fetch multiple branches
-t5801.8 push when remote has extra refs
-t5801.9 push new branch by name
-t5801.10 push new branch with old:new refspec
-t5801.11 push new branch with HEAD:new refspec
-t5801.12 push delete branch
-t5801.13 forced push
-t5801.14 cloning without refspec
-t5801.15 pulling without refspecs
-t5801.16 pushing without refspecs
-t5801.17 pulling without marks
-t5801.19 push all with existing object
-t5801.20 push ref with existing object
-t5801.23 push update refs
-t5801.24 push update refs disabled by no-private-update
-t5801.25 push update refs failure
-t5801.26 proper failure checks for fetching
-t5801.27 proper failure checks for pushing
-t5801.28 push messages
-t5801.29 fetch HEAD
-t5801.30 fetch url
-t5801.31 fetch tag
-t7400.80 ../subrepo works with helper URL- helper:://hostname/repo

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
…nvironment variables to Windows form for native Win32 applications.

With this commit, the POSIX-to-Windows conversion also leaves Git's
`<rev>:./<name>` syntax alone. Such a string would otherwise be mistaken
for indicating a path list, but path lists are expected to contain only
absolute paths, which would not be the case here.

This addresses the expectations of the following test cases in the test
suite of https://github.com/git/git/tree/v2.43.0:

-t1506.4 correct relative file objects (1)
-t1506.5 correct relative file objects (2)
-t1506.6 correct relative file objects (3)
-t1506.7 correct relative file objects (4)
-t1506.14 relative path not found
-t1506.15 relative path outside worktree
-t1506.16 relative path when cwd is outside worktree
-t1513.5 empty prefix HEAD:./path
-t1513.6 valid prefix HEAD:./path
-t1513.7 valid prefix HEAD:../path
-t2070.4 restore a file on worktree from another ref
-t2070.5 restore a file in the index from another ref
-t2070.6 restore a file in both the index and worktree from another ref
-t2070.8 restore --worktree --staged uses HEAD as source
-t7900.33 start and stop macOS maintenance
-t7900.34 use launchctl list to prevent extra work
-t7900.35 start and stop Windows maintenance
-t7900.36 start and stop Linux/systemd maintenance
-t7900.37 start and stop when several schedulers are available
-t9300.195 Y: rewrite submodules
-t9304.6 import with submodule mapping
-t9304.7 paths adjusted for relative subdir

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
…nvironment variables to Windows form for native Win32 applications.

Do not let MSYS2's path conversion mistake arguments starting with '@@'
for being paths.

This addresses the expectations of the following test cases in the test
suite of https://github.com/git/git/tree/v2.43.0:

-t1508.1 setup
-t1508.2 HEAD = refs/heads/new-branch
-t1508.3 @{1} = new-one
-t1508.4 HEAD@{1} = new-one
-t1508.5 @{now} = new-two
-t1508.6 HEAD@{now} = new-two
-t1508.7 @{-1} = refs/heads/old-branch
-t1508.8 @{-1}@{0} = old-two
-t1508.9 @{-1}@{1} = old-one
-t1508.10 @{u} = refs/heads/upstream-branch
-t1508.11 HEAD@{u} = refs/heads/upstream-branch
-t1508.12 @{u}@{1} = upstream-one
-t1508.13 @{-1}@{u} = refs/heads/main
-t1508.14 @{-1}@{u}@{1} = main-one
-t1508.15 @ = new-two
-t1508.16 @@{u} = refs/heads/upstream-branch
-t1508.17 @@/at-test = refs/heads/@@/at-test
-t1508.18 @at-test = refs/heads/@at-test
-t1508.24 HEAD@{3} = old-two
-t1508.26 switch to old-branch
-t1508.27 HEAD = refs/heads/old-branch
-t1508.28 HEAD@{1} = new-two
-t1508.29 @{1} = old-one

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
…nvironment variables to Windows form for native Win32 applications.

Let's prevent scp-style arguments from being mangled by MSYS2's path
conversion.

An argument like `me@example.com:/tmp/` is not something we should convert
into a Windows path; Use the absence of a slash before the colon as a
tell-tale that it is *not* a POSIX path list (exception: if the part
left of the colon is `.` or `..`).

This addresses the expectations of the following test cases in the test
suite of https://github.com/git/git/tree/v2.43.0:

-t5516.8 fetch with insteadOf
-t5516.16 push with insteadOf
-t5516.17 push with pushInsteadOf
-t5602.2 clone calls git upload-pack unqualified with no -u option
-t5602.3 clone calls specified git upload-pack with -u option
-t5603.31 clone of host:/ goes to host (non-bare)
-t5603.35 clone of user@host:/ goes to host (non-bare)
-t5813.81 full paths still work

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When the character set is specified as ASCII (by setting the locale to
`C`), we should handle data outside the 7-bit range gracefully by simply
copying it, even if it is technically no longer ASCII.

Cygwin, however, wants to be a lot stricter than that.

Let's be more lenient in MSYS2 by making the strict 7-bit only handling
contingent on the `STRICTLY_7BIT_ASCII` macro (which we never set
because we don't want it).

This addresses the expectations of two of Git's test cases:
t7400.108(submodule with UTF-8 name) and t9300.193(X: handling
encoding).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the git-for-windows-pathconv-adjustments branch from 693c449 to 663d774 Compare December 18, 2023 23:33
@dscho dscho requested a review from lazka December 18, 2023 23:33
@dscho
Copy link
Collaborator Author

dscho commented Dec 18, 2023

I rebased this to resolve the merge conflicts.

@lazka lazka merged commit 6ab2da6 into msys2:msys2-3.4.10 Dec 19, 2023
1 check passed
@dscho dscho deleted the git-for-windows-pathconv-adjustments branch December 19, 2023 09:05
dscho added a commit to dscho/MSYS2-packages that referenced this pull request Dec 19, 2023
This combines the patches introduced via
msys2/msys2-runtime#181 and
msys2/msys2-runtime#182.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@lazka
Copy link
Member

lazka commented Dec 22, 2023

The test part is here: https://github.com/msys2/msys2-tests/pull/58/files

If this turns out to create any problems and there is no easy fix we could think about a MSYS2_ARG_CONV_INCL to opt in certain arguments to not be skipped, or something like that.

@dscho
Copy link
Collaborator Author

dscho commented Dec 23, 2023

Thank you, @lazka !

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