Re: [PATCH 2/2] repack -ad: prune the list of shallow commits

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 19, 2018, 16:42 UTC
Message-ID: <nycvar.QRO.7.76.6.1807191840490.71@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqh8kxrfrf.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Tue, 17 Jul 2018, Junio C Hamano wrote:

Show 13 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > I'm OK with having a partial fix, or one that fixes immediate pain
> > without doing a big cleanup, as long as it doesn't make anything _worse_
> > in a user-visible way. And that's really my question: is pruning here
> > going to bite people unexpectedly (not rhetorical -- I really don't
> > know)?
> 
> Yeah, that matches the general guideline I follow when reviewing a
> patch that claims to make existing things better.  And I do not
> think I can explain to a third person why pruning here is a good
> idea and won't cause problems, after seeing these patches and
> the discussion from the sideline.

It is very easy to explain: `git repack` can drop unreachable commits without further warning, making the corresponding entries in `.git/shallow` invalid, which causes serious problems when deepening the branches.

The solution is easy: drop also the now-invalid entries in `.git/shallow` after dropping unreachable commits unceremoniously.

While I am sympathetic to Peff's attempt to make this a bit more general, I think he overthinks it, and we do not even need to complexify the code for now.

Ciao, Dscho