git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 2/2] commit-graph.c: no lazy fetch in lookup_commit_in_graph()

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jul 1, 2022, 19:31 UTC
Message-ID
<n3p471no-671q-2701-1r72-s0q02ns09053@tzk.qr>
In-Reply-To
<xmqq5ykignwb.fsf@gitster.g>
Hi Junio,
On Thu, 30 Jun 2022, Junio C Hamano wrote:
Show 36 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
> > On Thu, Jun 30 2022, Johannes Schindelin wrote:
> >
> >> On Tue, 28 Jun 2022, Junio C Hamano wrote:
> >>
> >>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> >>>
> >>> >> +test_expect_success 'setup: prepare a repository with commit-graph contains the commit' '
> >>> >> +	git init with-commit-graph &&
> >>> >> +	echo "$(pwd)/with-commit/.git/objects" \
> >>> >> +		>with-commit-graph/.git/objects/info/alternates &&
> >>> >
> >>> > nit: you can use $PWD instead of $(pwd).
> >>>
> >>> We can, and it would not make any difference on non-Windows.
> >>>
> >>> But which one should we use to cater to Windows?  $(pwd) is a full
> >>> path in Windows notation "C:\Program Files\Git\..." while $PWD is
> >>> MSYS style "/C/Program Files/Git/..." or something like that, IIRC?
> >>
> >> Indeed, and since the `alternates` file is supposed to be read by
> >> `git.exe`, a non-MSYS program, the original was good, and the nit
> >> suggested the incorrect form.
> >
> > I looked at t5615-alternate-env.sh which does the equivalent of:
> >
> > 	GIT_ALTERNATE_OBJECT_DIRECTORIES="$PWD/one.git/objects:$PWD/two.git/objects" \
> >         	git cat-file [...]
> >
> > We run that test on all our platforms, does the $PWD form work in the
> > environment variable, but not when we write it to the "alternates" file?
> > Or is there some other subtlety there that I'm missing?
>
> I am also curious to see a clear and concise explanation so that we
> do not have to repeat this discussion later.

Unfortunately, I do not see any way to explain this concisely: MSYS2 does hard-to-explain things here, in the hopes to Do The Right Thing (most of the time, anyways).

Whenever you call a non-MSYS program from an MSYS program (and remember, an MSYS program is a program that uses the MSYS2 runtime that acts as a POSIX emulation layer), "magic" things are done. In our context, `bash.exe` is an MSYS program, and the non-MSYS program that is called is `git.exe`.

So what are those "magic" things? The command-line arguments and the environment variables are auto-converted: everything that looks like a Unix-style path (or path list, like the `PATH` environment variable) is converted to a Windows-style path or path list (on Windows, the colon cannot be the separator in `PATH`, therefore the semicolon is used).

And this is where it gets _really_ tricky to explain what is going on: what _does_ look like a Unix-style path? The exact rules are convoluted and hard to explain, but they work _most of the time_. For example, `/usr/bin:/hello` is converted to `C:\Program Files\Git\usr\bin;C:\Program Files\Git\hello` or something like that. But `kernel.org:/home/gitster` is not, because it looks more like an SSH path. Similarly, `C:/Program Files` is interpreted as a Windows-style path, even if it could technically be a Unix-style path list.

Now, if you call `git.exe -C /blabla <command>`, it works, because `git.exe` is a non-MSYS program, therefore that `/blabla` is converted to a Windows-style path before executing `git.exe`. However, when you write a file via `echo /blabla >file`, that `echo` is either the Bash built-in, or it is an MSYS program, and no argument conversion takes place. If you _then_ ask `git.exe` to read and interpret the file as a path, it won't know what to do with that Unix-style path.

You can substitute `$PWD` for `/blabla` in all of this, and it will hold true just the same.

So what makes `pwd` special?

Well, `pwd.exe` itself is an MSYS program, so it would still report a path that `git.exe` cannot understand. But in Git's test suite, we specifically override `pwd` to be a shell function that calls `pwd.exe -W`, which does output Windows-style paths.

The thing that makes that `GIT_*=$PWD git ...` call work is that the environment is automagically converted because `git` is a non-MSYS program. The thing that makes `echo $PWD >.git/objects/info/alternates` not work is that `echo` _is_ an MSYS program (or Bash built-in, which is the same thing here, for all practical purposes), so it writes the path verbatim into that file, but then we expect `git.exe` to read this file and interpret it as a list of paths.

Hopefully that clarifies the scenario a bit, even if it is far from a concise explanation (I did edit this mail multiple times for clarity and brevity, though, as I do with pretty much all of my mails).

Ciao, Dscho

Show 55 quoted lines
> We have
>
>  - When a test checks for an absolute path that a git command generated,
>    construct the expected value using $(pwd) rather than $PWD,
>    $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference on
>    Windows, where the shell (MSYS bash) mangles absolute path names.
>    For details, see the commit message of 4114156ae9.
>
> in t/README, but even with the log mesasge of 4114156a (Tests on
> Windows: $(pwd) must return Windows-style paths, 2009-03-13) [*1*],
> I have no idea what makes the thing you found in t5615 work and your
> suggestion to use $PWD in the new one not work.
>
> GIT_ALTERNATE_OBJECT_DIRECTORIES is a PATH_SEP (not necessarily a
> colon) separated list, and I think the way t5615 uses it is broken
> on Windows where PATH_SEP is defined as semicolon without the $PWD
> vs $(pwd) issue.  Is the test checking the right thing?
>
>
> [Footnote]
>
> *1*
>
>     Tests on Windows: $(pwd) must return Windows-style paths
>
>     Many tests pass $(pwd) in some form to git and later test that the output
>     of git contains the correct value of $(pwd). For example, the test of
>     'git remote show' sets up a remote that contains $(pwd) and then the
>     expected result must contain $(pwd).
>
>     Again, MSYS-bash's path mangling kicks in: Plain $(pwd) uses the MSYS style
>     absolute path /c/path/to/git. The test case would write this name into
>     the 'expect' file. But when git is invoked, MSYS-bash converts this name to
>     the Windows style path c:/path/to/git, and git would produce this form in
>     the result; the test would fail.
>
>     We fix this by passing -W to bash's pwd that produces the Windows-style
>     path.
>
>     There are a two cases that need an accompanying change:
>
>     - In t1504 the value of $(pwd) becomes part of a path list. In this case,
>       the lone 'c' in something like /foo:c:/path/to/git:/bar inhibits
>       MSYS-bashes path mangling; IOW in this case we want the /c/path/to/git
>       form to allow path mangling. We use $PWD instead of $(pwd), which always
>       has the latter form.
>
>     - In t6200, $(pwd) - the Windows style path - must be used to construct the
>       expected result because that is the path form that git sees. (The change
>       in the test itself is just for consistency: 'git fetch' always sees the
>       Windows-style path, with or without the change.)
>
>     Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>
>
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 31 of 50 in “Re: An endless loop fetching issue with partial clone, alternates and commit graph”
  1. Haiyng TanJun 14, 2022
  2. Taylor BlauJun 15, 2022
  3. 0/2 Re: An endless loop fetching issue with partial clone, alternates and commit graphHan Xin, Jun 16, 2022
  4. 1/2 commit-graph.c: add "flags" to lookup_commit_in_graph()Han Xin, Jun 16, 2022
  5. 2/2 fetch-pack.c: pass "oi_flags" to lookup_commit_in_graph()Han Xin, Jun 16, 2022
  6. Jonathan TanJun 17, 2022
  7. commit-graph.c: no lazy fetch in lookup_commit_in_graph()Han Xin, Jun 18, 2022
  8. Patrick SteinhardtJun 20, 2022
  9. 欣韩Jun 20, 2022
  10. Patrick SteinhardtJun 20, 2022
  11. Jonathan TanJun 21, 2022
  12. Han XinJun 22, 2022
  13. 0/2 commit-graph.c: no lazy fetch in lookup_commit_in_graph()Han Xin, Jun 24, 2022
  14. 1/2 test-lib.sh: add limited processes to test-libHan Xin, Jun 24, 2022
  15. Junio C HamanoJun 24, 2022
  16. Han XinJun 25, 2022
  17. Junio C HamanoJun 27, 2022
  18. 2/2 commit-graph.c: no lazy fetch in lookup_commit_in_graph()Han Xin, Jun 24, 2022
  19. Junio C HamanoJun 24, 2022
  20. Han XinJun 25, 2022
  21. Han XinJun 25, 2022
  22. 0/2 no lazy fetch in lookup_commit_in_graph()Han Xin, Jun 28, 2022
  23. 1/2 test-lib.sh: add limited processes to test-libHan Xin, Jun 28, 2022
  24. 2/2 commit-graph.c: no lazy fetch in lookup_commit_in_graph()Han Xin, Jun 28, 2022
  25. Ævar Arnfjörð BjarmasonJun 28, 2022
  26. Junio C HamanoJun 28, 2022
  27. Johannes SchindelinJun 30, 2022
  28. Ævar Arnfjörð BjarmasonJun 30, 2022
  29. Junio C HamanoJun 30, 2022
  30. Ævar Arnfjörð BjarmasonJun 30, 2022
  31. Johannes SchindelinJul 1, 2022
  32. Junio C HamanoJul 1, 2022
  33. Han XinJun 29, 2022
  34. test name conflict + js/ci-github-workflow-markup regression (was: [PATCH v3 0/2] no lazy fetch in lookup_commit_in_graph())Ævar Arnfjörð Bjarmason, Jun 30, 2022
  35. 0/1 no lazy fetch in lookup_commit_in_graph()Han Xin, Jul 1, 2022
  36. 1/1 commit-graph.c: no lazy fetch in lookup_commit_in_graph()Han Xin, Jul 1, 2022
  37. Michael J GruberJul 9, 2022
  38. Jeff KingJul 11, 2022
  39. Junio C HamanoJul 11, 2022
  40. Han XinJul 12, 2022
  41. Junio C HamanoJul 12, 2022
  42. Han XinJul 12, 2022
  43. Jeff KingJul 12, 2022
  44. Junio C HamanoJul 12, 2022
  45. 0/1 no lazy fetch in lookup_commit_in_graph()Han Xin, Jul 12, 2022
  46. 1/1 commit-graph.c: no lazy fetch in lookup_commit_in_graph()Han Xin, Jul 12, 2022
  47. Ævar Arnfjörð BjarmasonJul 12, 2022
  48. Han XinJul 13, 2022
  49. Jeff KingJul 12, 2022
  50. t5330: remove run_with_limited_processses()Han Xin, Jul 12, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.