Re: [PATCH 1/2] index-pack: Create .keep files with same permissions and .pack/.idx

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 2, 2011, 04:21 UTC
Message-ID: <7v39m1e110.fsf@alter.siamese.dyndns.org>
In-Reply-To: <201104020127.41774.johan@herland.net>

Johan Herland <johan@herland.net> writes:

> Hmpf. I didn't know that clone --local --no-hardlinks was unsafe. If it's 
> not safe, should it still be the default behavior for a cross-filesystem 
> clone?

Unsafe is not quite the right word to use here in the sense that it wouldn't lead to any repository _corruption_ per-se, but if you ended up copying such a transient .keep file, the pack will stay forever in your clone target unless you notice and remove it yourself.

Having said that, I expect that the majority of use of a filesystem level local clone these days is to clone your own repository, likely on your own machine, and you have absolute control on both ends (e.g. you wouldn't be running a repack on the source while running a clone---you would more likely to see the repack finish and then clone). So in that sense I would still think that file level clone being the default on a local machine is a reasonable default.

Show 6 quoted lines

> While pushing to a remote repo, Git transiently adds a .keep file for the
> pack being pushed, to protect it from a concurrent "git gc". Sometimes, when 
> the push fails or is aborted, the .keep file is left stale in the repo. This 
> causes problems for other users of the same repo, since the permissions on 
> the .keep file (0600) make it inaccessible even though the rest of the repo 
> is accessible (0444 modulo shared_repository setting).

I was also wondering why you initialized with 0444 in your patch and then even adjusted for shared repository settings.

This is a tangent, but wouldn't it be wrong for index-pack to always leave the idx and pack files in 0444 with an explicit chmod() in the first place? I suspect that we simply forgot to fix it when we introduced adjust_shared_perm().