Git: worktree config option is set when cloning with an absolute path

[attilastrba]

• I have found a similar issue : https://github.com/git-for-windows/git/issues/2273 but the behaviour is different

  Setup

• Which version of Git for Windows are you using? Is it 32-bit or 64-bit?

$ git --version --build-options
git version 2.26.0.windows.1
cpu: x86_64
built from commit: 9c98e1ccdfd839e4eaae1c2747d0088ef89d446b
sizeof-long: 4
sizeof-size_t: 8

• Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?

$ cmd.exe /c ver

Microsoft Windows [Version 10.0.18362.476]

• What options did you set as part of the installation? Or did you choose the
  defaults?

# One of the following:
> type "C:\Tools\Git\etc\install-options.txt"
Editor Option: VIM
Custom Editor Path:
Path Option: CmdTools
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFCommitAsIs
Bash Terminal Option: MinTTY
Performance Tweaks FSCache: Enabled
Use Credential Manager: Disabled
Enable Symlinks: Disabled

• Any other interesting things about your environment that might be related
  to the issue you're seeing?

none

Details

• Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other
  PowerShell / Bash

• What commands did you run to trigger this issue? If you can provide a
  Minimal, Complete, and Verifiable example
  this will help us understand the issue.

 git clone https://github.com/lukesampson/scoop  "C:\tools\test5"
PS C:\tools\test5> cat .\.git\config
[core]
        repositoryformatversion = 0
        filemode = false
        bare = false
        logallrefupdates = true
        worktree = C:/Tools/test5
        symlinks = false
        ignorecase = true
[remote "origin"]
        url = https://github.com/lukesampson/scoop
        fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
        remote = origin
        merge = refs/heads/master

• What did you expect to occur after running these commands?

I except to have .git/config file with no worktree attribute

• What actually happened instead?

I got a worktree parameter in the repository config although I haven't added worktree. If I rename my folder than I got an error
fatal: this operation must be run in a work tree

This happens only if I clone with an absolute path in the parameter. I have tested this on a different machine, older git version 2.19 works ok. On linux git version I can't see this behaviour. I have read the https://git-scm.com/docs/git-worktree documentation but as far as I understand the worktree should only be set once it is added as a parameter or set in the config.

• If the problem was occurring with a specific repository, can you provide the
  URL to that repository to help us with testing?

https://github.com/lukesampson/scoop

[dscho]

I can reproduce, but only if the path has backslashes. With forward-slashes, and when the case matches _precisely_ (i.e. the drive letter C is upper-case), no core.worktree entry is created.

[dscho]

Never mind, the backslash seems to have nothing to do with it. In my tests, it is exclusively the matter of the case-insensitivity: If I spell out the path _precisely_ as it is on disk (with upper-case drive letter), I don't get core.worktree. Is your C:\tools maybe all upper-case, or starting with a capital letter?

[attilastrba]

Thank you for looking into this. Yes my Tools starts with an uppercase, so the clone path is starting with a capital C:\Tools\test5. If there is anything else I can do or if you point to the code I should look around would be happy to help.

[dscho]

@attilastrba I started a patch, but lack the time to accompany it with an added regression test case. Please feel free to give it a try (if you want to see a fix in Git for Windows soon, that is, otherwise you can just wait it out).

[attilastrba]

@dscho thank you! I have cloned your repo, installed git-sdk, compiled it and tested (version 2.10.0.rc2.14679.gecd10bed6a) and was unable to reproduce the issue. If you want me to create a regression test if you would point me where I could give it a try for a pull request.

[dscho]

If you want me to create a regression test if you would point me where I could give it a try for a pull request.

The best idea would probably be to add it to t/t5580-unc-paths.sh, imitating the clone into absolute path lacking a drive prefix test.

[SyntevoAlex]

I have studied a similar problem today. Just like @dscho, I first decided to change needs_work_tree_config().

However, after additional debugging, I found that the root cause is elsewhere: mingw_strbuf_realpath() doesn't satisfy the contract of strbuf_realpath(), where path may contain last component that doesn't exist.

In the beginning of init_db(), there's this line:
char *original_git_dir = real_pathdup(git_dir, 1)

/.git doesn't exist yet, and mingw_strbuf_realpath() fails, and default strbuf_realpath() produces something slightly different from what mingw_strbuf_realpath() would do.

Later, near needs_work_tree_config(), there's this line:
const char *work_tree = get_git_work_tree()

It returns value cached with set_git_work_tree(), where mingw_strbuf_realpath() succeeded, because this path doesn't have trailing /.git.

So, in my understanding, the real fix is to change mingw_strbuf_realpath() to survive last component that doesn't exist.

@dscho shall I provide a patch for that?

[dscho]

So, in my understanding, the real fix is to change mingw_strbuf_realpath() to survive last component that doesn't exist.

@dscho shall I provide a patch for that?

Very good analysis!

And yes, a patch would be much appreciated that teaches mingw_strbuf_realpath() to test whether the item actually exists (and if it does not, finds the longest prefix that _does_ refer to an existing one, then canonicalizes _that_, and after that, re-appends the tail).

Thank you!

[SyntevoAlex]

According to contract, strbuf_realpath() may only have just one trailing non-existent component:
must denote a valid, existing directory, but the last component need not exist

It will be rather easy to implement for just one component, because then, no /../ can be involved. I'll just stash the last component, process what's left, and re-append last component. Since this component doesn't exist, it doesn't need case normalization.

Do we really need to implement longest prefix?

[dscho]

According to contract, strbuf_realpath() may only have just one trailing non-existent component:
must denote a valid, existing directory, but the last component need not exist

Hah, I learned something new today.

Do we really need to implement longest prefix?

No, you were right ;-)

[SyntevoAlex]

I hope to provide the patch tomorrow.

[SyntevoAlex]

Here's a patch and a test. As expected, the test fails without the patch.