Fix Windows permissions problems

[carlosmn]

packed-refs should not be treated like a packfile. It looks like windows doesn't like it when we're not allowed to write to that file. It looks like Windows won't let us overwrite a file if we can't write to it (or something like that, I don't particularly care why).

This fixes the tests for both MSVC and MSYS. The real fix is the last commit. The rest is stuff I found whilst hunting this down.

[vmg]

👍

[nulltoken]

👍

[aiiie]

When you tested this with Git itself, did you first change your umask to 0?

$ cd /tmp
$ umask 0
$ git init foobar
Initialized empty Git repository in /private/tmp/foobar/.git/
$ cd foobar
$ git pack-refs
$ stat -f '%p' .git/packed-refs
100666

You should be using 0666, not 0644. Otherwise you're not fully respecting the user's umask.

[aiiie]

First question: Is this just refactoring, or is it required to fix Windows support?

Second question: Aren't things in posix.h supposed to use the real underlying POSIX implementations if they're available? Why isn't this just a #define for rename(2)? And why does this call link() and unlink() instead of just doing rename()? If anything, wouldn't that open up the possibility of this operation not being atomic?

[carlosmn]

On Mon, Nov 07, 2011 at 12:58:24PM -0800, Brodie Rao wrote:

@@ -39,6 +39,20 @@ int p_getcwd(char *buffer_out, size_t size)
return GIT_SUCCESS;
}

+int p_rename(const char *from, const char *to)
+{

• if (!link(from, to)) {

•   p_unlink(from);
  

•   return GIT_SUCCESS;
  

• }

• if (!rename(from, to))

•   return GIT_SUCCESS;
  
• return GIT_ERROR;

+}
+

First question: Is this just refactoring, or is it required to fix Windows support?

Yes and no. Our current test suite doesn't test the Unicode support so
it doesn't actually fix any problems we're seeing, but p_rename
needs to transform the path to UTF-16 on Windows, so it does fix a
problem (just not this particular one).

Second question: Aren't things in posix.h supposed to use the real
underlying POSIX implementations if they're available? Why isn't

Not exactly. We stick to POSIX except where we find it to be lacking
or annoying for our purposes.

this just a #define for rename(2)? And why does this call
link() and unlink() instead of just doing rename()? If
anything, wouldn't that open up the possibility of this operation
not being atomic?

I believe @tanoku wrote that originally, I'm just moving the Windows
part so it uses UTF-16.

cmn

[carlosmn]

Does umask even exist on Windows?@xpaulbettsx could you share your wisdom?

[anaisbetts]

Ummm, in a way - ACLs inherit their parent's ACLs if the directory is set to do so, it's not set via any global mask like umask is.

Paul Betts

SENT FROM MY COMMODORE 64: RESPONSES MAY BE IN ALL CAPS

On Nov 8, 2011, at 8:18, Carlos Martín Nietoreply@reply.github.com wrote:

@@ -39,6 +39,20 @@ int p_getcwd(char *buffer_out, size_t size)
return GIT_SUCCESS;
}

+int p_rename(const char *from, const char *to)
+{

• if (!link(from, to)) {

•    p_unlink(from);
  

•    return GIT_SUCCESS;
  

• }

• if (!rename(from, to))

•    return GIT_SUCCESS;
  
• return GIT_ERROR;

+}
+

Does umask even exist on Windows?@xpaulbettsx could you share your wisdom?

---

Reply to this email directly or view it on GitHub:
https://github.com/libgit2/libgit2/pull/475/files#r215829

[carlosmn]

We might want to set ACLs to do something equivalent to git init --shared. MSDN tells me that the _umask does exist (but with quite limited capabilities, as the umask doesn't really exist on Windows). I was thinking that maybe we should apply the umask in p_chmod ourselves under Windows, but since it would only affect the user's permissions and all the default umask does there is remove the execute bit, which doesn't exist under Windows (or rather, it can't be unset), so we might just as well ignore it.

[aiiie]

You shouldn't need to use umask() in libgit2. That's just for changing the current umask. open(), chmod(), etc. will respect the umask.

I was just mentioning it as a way to confirm exactly what permissions Git uses.

[carlosmn]

On Mon, Nov 07, 2011 at 12:52:39PM -0800, Brodie Rao wrote:

When you tested this with Git itself, did you first change your umask to 0?

I didn't check with git.git. I was trying to solve the problem libgit2
had, which turned out to be that we removed ourselves the permission
to rename that file (under Windows).

$ cd /tmp
$ umask 0
$ git init foobar
Initialized empty Git repository in /private/tmp/foobar/.git/
$ cd foobar
$ git pack-refs
$ stat -f '%p' .git/packed-refs
100666

You should be using 0666, not 0644. Otherwise you're not fully
respecting the user's umask.

You're the one who wrote the permissions code; As I said, I was just
trying the particular error we were having. If the correct mode is
0666, then I'll change it so it is.

cmn