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

From: Johan Herland <johan@herland.net>
Date: Apr 3, 2011, 01:01 UTC
Message-ID: <201104030301.29923.johan@herland.net>
In-Reply-To: <7v39m1e110.fsf@alter.siamese.dyndns.org>

On Saturday 02 April 2011, Junio C Hamano wrote:

Show 11 quoted lines

> Johan Herland <johan@herland.net> writes:
> > 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.

I was simply emulating what is currently done for idx and pack files (see below).

> 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().

Yeah, probablby, but AFAICS in the receive-pack case, final_pack_name and final_index_name are both NULL (neither are specified on the index-pack command line passed from receive-pack), so the explicit chmod(..., 0444) is never called. Instead the pack and idx files are both opened from odb_mkstemp() (via open_pack_file() and write_idx_file(), respectively), which uses mode 0444. We then call move_temp_to_file(), which calls adjust_shared_perm().

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net