Switching branches after performing git-new-workdir fails

[rburgstaler]

Error fatal: Cannot lock ref 'refs/heads/Branch1': unable to create directory for .git/refs/heads/Branch1 when checking out a branch into a repostiory that was created using GetNewWorkDir

Steps to reproduce:
1.) Clone a repository to C:\Dev\Projects_TRUNK via https from some source code server
2.) Use GitNewWorkDir to create a new working directory C:\Dev\Projects_Branch1 that uses C:\Dev\Projects_TRUNK as it's base directory
3.) cd to C:\Dev\Projects_Branch1 and execute: git checkout -B Branch1 "remotes/origin/Branch1"
4.) This will give error: fatal: Cannot lock ref 'refs/heads/Branch1': unable to create directory for .git/refs/heads/Branch1

As far as I can tell this appears to be a regression bug from Git-2.4.1.1-dev-preview-64-bit.exe

I found that calling (git checkout -B Branch1 "remotes/origin/Branch1") works as expected in:
Git-2.4.1.1-dev-preview-64-bit.exe

When I went to upgrade to a newer version of Git-For-Windows (Git-2.4.4.2-3rd-release-candidate-64-bit.exe), I found that calling (git checkout -B Branch1 "remotes/origin/Branch1") gives error (fatal: Cannot lock ref 'refs/heads/Branch1': unable to create directory for .git/refs/heads/Branch1)
I found that it is also broken in: Git-2.4.3.1-2nd-release-candidate-64-bit.exe

The exe's I am referring to are the ones that dscho released in:
https://github.com/git-for-windows/git/releases

[dscho]

What is GetNewWorkDir? I do not think that Git for Windows ships with it.

[rburgstaler]

Sorry, I mistyped. I meant git-new-workdir:
https://github.com/git-for-windows/git/blob/master/contrib/workdir/git-new-workdir

I am guessing that the new version of Git does not like the symbolic directory links that git-new-workdir creates.

[dscho]

You did see #170, didn't you?

[rburgstaler]

Yeah, I saw #170 and at first I thought it was the same issue. I worked around that issue by creating empty files for the files that the symbolic links were supposed to be pointing to.

In my case, the directory C:\Dev\Projects_TRUNK.git\refs\heads does actually exist. The symbolic link (C:\Dev\Projects\Branch1.git\refs) that points to (C:\Dev\Projects_TRUNK.git\refs) actually works, git just fails to create the ref file: (C:\Dev\Projects_Branch1.git\refs\heads\Branch1) in the newer versions of git for windows when calling git checkout -B Branch1 "remotes/origin/Branch1".

And as I mentioned before, all I need to do to work around this is uninstall Git For Windows and install Git-2.4.1.1-dev-preview-64-bit.exe. After installing the older version and running git checkout -B Branch1 "remotes/origin/Branch1" it works just fine.

The fact that I can roll back to an older version and simply execute the (git checkout -B Branch1 "remotes/origin/Branch1") command with success makes me think that it is not related to #170

[rburgstaler]

FYI - I am debugging this one in my spare time to see if I can come up with the root source of the problem.

[rburgstaler]

So I tracked the issue I was having down. It appears to have broken in SHA: a275af5

In the function mingw_stat() where it was checking the status of the .git/refs directory (which is a symbolic link) it was first checking whether has_symlinks is true or false. In my case (and a bunch of other people's cases I am guessing) it would be false because "git clone" and "git init" make a best guess how to set that property and is setting it to false:
See https://www.kernel.org/pub/software/scm/git/docs/git-config.html
under core.symlinks: "The default is true, except git-clone(1) or git-init(1) will probe and set core.symlinks false if appropriate when the repository is created."

I am able to get things working by simply calling:
git config --local core.symlinks true

However, my bigger picture question is, should mingw_stat() be checking the has_symlinks variable when the caller is checking mingw_stat() for the sake of git plumbing operations working on internal git directories. I would think that the sole intention of has_symlinks is when checking the mingw_stat() of files being checked out into the repository.

[nalla]

Thanks for tanking both the time and the interest in tracking down the issue at hand. @kblees maybe we need to differenciate between git dir and working dir when calling the override?

[dscho]

should mingw_stat() be checking the has_symlinks variable

Please note that we already have a PR open for this: #220

I agree, and we just have to convince @kblees that this is the way to go (and that it should be safe to use FILE_FLAG_BACKUP_SEMANTICS because it should fall back to "whatever file security checks would ordinarily be in effect" when the current user lacks the appropriate privileges, rather than erroring out.

It would be really grand if you could verify that it actually does that: falling back rather than erroring out if using with an unprivileged account. @rburgstaler could you do that, please?

[rburgstaler]

@dscho Yeah, I can verify, however, I am a little unclear to what exactly you want me to do?
This is what I gather you are wanting me to do:
1.) Apply PR #220
2.) Login with non Administrator user
3.) run: git checkout -B Branch1 "remotes/origin/Branch1"
Are we looking to see if it resolves my issue or is there additional checking that I would need to do such as verify that mingw_stat() still returns the same result as well as populating *buf with the same values as with admin rights?

[dscho]

I want to know whether the code in #220 works without symlinks and without privileges to run with the backup semantics. The concern is that it might fail to work altogether, in which case we could not go with that code, of course.

[rburgstaler]

ok... wanted to give a status update on this one just in case I am going down the wrong path. First of all, I assumed that "without privileges" means that neither the SE_BACKUP_NAME nor the SE_RESTORE_NAME privilege is enabled. To do this, I wanted to test whether these privileges existed at all in the first place when running the mingw_stat() method. I found that they are not enabled even when running git-bash as an administrator and with elevated privileges.

I used this chunk of code for testing this:
SHA: d5ecef2

The very first output from this debug code is:
Process IS running with admin privileges
SeBackupPrivilege NOT ENABLED!
SeBackupPrivilege ENABLED!
SeRestorePrivilege NOT ENABLED!
SeRestorePrivilege ENABLED!

In this commit, I found that we could set the SE_BACKUP_NAME and SE_RESTORE_NAME privileges to enabled if we desired to ensure that my check as to whether they were actually disabled in the first place was accurate. (FYI - you can only set the SE_BACKUP_NAME and SE_RESTORE_NAME if you have elevated privileges to the process which means that you right clicked on the git-bash and chose to "Run as Administratory")

My next steps are to perform this test with a more limited user that is not an Administrator, however, I kind of feel that this is redundant as even an administrator running with elevated privileges does not have SE_BACKUP_NAME nor SE_RESTORE_NAME enabled by default.

[dscho]

@rburgstaler any news on that test?

[rburgstaler]

@dscho Side tracked... sorry... I will try to get that done tonight or tomorrow night.

Hey, does my observations from my prior post make sense to you? Basically, I found that neither the SeBackupPrivilege privilege nor the SeRestorePrivilege will be enabled for any user of Git (even if run as an Administrator). In order to have those privileges set in the first place, you would need to explicitly write code that enables the privilege through use of the AdjustTokenPrivileges() call. I did not find any code in git-for-windows/git that calls AdjustTokenPrivileges().

Either way... To be complete, I will perform the tests with a user that is not a member of the Windows Administrators group.

[dscho]

@rburgstaler I am mainly interested in knowing whether the code to read symlinks would work for non-admins, even if asking with those backup semantics.

[rburgstaler]

So, I created a Windows User (testgit) that only was a member of "Home Users" and "Users", logged in as that user "testgit" and I was able to able to operate correctly after the #220 patch.

I used the following code to prove the case:
https://github.com/rburgstaler/git/tree/rb/debug-symlink-independent-of-stat-check-for-errors

Let me know if you need me to give more justification as to how it succeeded.

[dscho]

Fixed by #220 (I think).