Re: [PATCH 1/3] read-cache: use shared perms when writing shared index

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 22, 2017, 19:51 UTC
Message-ID: <xmqq8tkjkdgl.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170622190158.13265-1-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

Show 15 quoted lines

> Since f6ecc62dbf (write_shared_index(): use tempfile module, 2015-08-10)
> write_shared_index() has been using mks_tempfile() to create the
> temporary file that will become the shared index.
>
> But even before that, it looks like the functions used to create this
> file didn't call adjust_shared_perm(), which means that the shared
> index file has always been created with 600 permissions regardless
> of the shared permission settings.
>
> This means that on repositories created with `git init --shared=all`
> and using the split index feature one gets an error like:
>
> fatal: .git/sharedindex.a52f910b489bc462f187ab572ba0086f7b5157de: index file open failed: Permission denied
>
> when another user performs any operation that reads the shared index.

Assuming that a "shared" repository setting should allow uses by different users, the above analysis makes sense to me.

But the conclusion does not.

Show 6 quoted lines

> Let's fix that by using create_tempfile() instead of mks_tempfile()
> to create the shared index file.
>
> ...
> -	fd = mks_tempfile(&temporary_sharedindex, git_path("sharedindex_XXXXXX"));
> +	fd = create_tempfile(&temporary_sharedindex, git_path("sharedindex_XXXXXX"));

So we used to create a temporary file that made sure its name is unique but now we create sharedindex_XXXXXX with 6 X's literally at the end?

Doesn't mks_tempfile() family include a variant where you can give custom mode? Better yet, perhaps you can call adjust_shared_perm() on the path _after_ seeing that mks_tempfile() succeeds (you can ask get_tempfile_path() which path to adjust, I presume)?

>  	if (fd < 0) {
>  		hashclr(si->base_sha1);
>  		return do_write_locked_index(istate, lock, flags);