Re: [PATCH 0/3] commit-graph: write non-split graphs as read-only

From: Taylor Blau <me@ttaylorr.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Taylor Blau <me@ttaylorr.com>,
	git@vger.kernel.org, dstolee@microsoft.com, mhagger@alum.mit.edu,
	peff@peff.net
Subject: Re: [PATCH 0/3] commit-graph: write non-split graphs as read-only
Date: Mon, 20 Apr 2020 17:39:07 -0600	[thread overview]
Message-ID: <20200420233907.GA97996@syl.local> (raw)
In-Reply-To: <xmqq5zdtrbby.fsf@gitster.c.googlers.com>

On Mon, Apr 20, 2020 at 04:23:13PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > The first two patches set this up, and the third patch uses it in
> > commit-graph.c, and corrects some test fallout. Eventually, we may want
> > to take another look at all of this and always create lockfiles with
> > permission 0444, but that change is much larger than this one and could
> > instead be done over time.
>
> So,... is the problem that this did not follow the common pattern of
> creating (while honoring umask) and then call adjust_shared_perm(),
> which I thought was the common pattern for files in $GIT_DIR/?

We do call adjust_shared_perm() from (based on what's currently in
master) 'write_commit_graph_file' -> 'hold_lockfile_for_update' ->
'lock_file_timeout' -> 'lock_file' -> 'create_tempfile'.

But do we want commit-graphs to be respecting 'core.sharedRepository'
here? Either we:

  * do want to respect core.sharedRepository, in which case the
    current behavior of always setting split-graph permissions to '0444'
    is wrong, or

  * we do not want to respect core.sharedRepository, in which case these
    patches are doing what we want by setting all commit-graph files to
    have read-only permissions.

My hunch is that we do not want to abide by core.sharedRepository here
for the same reason that, say, loose objects are read-only. What do you
think?

Thanks,
Taylor