Re: [PATCH v3 1/3] repack: point out a bug handling stale shallow info

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Wed, 24 Oct 2018, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > +...
> > +	d="$(git -C shallow-server rev-parse --verify D)" &&
> > +	git -C shallow-server checkout master &&
> > +
> > +...
> > +	! grep $d shallow-client/.git/shallow &&
> 
> We know D (and $d) is not a tag,


Actually, it is... the `test_commit D` creates that tag, and that is what
I use here.

> but perhaps the place that assigns to $d (way above) can do the
> rev-parse of D^0, not D, to make it more clear what is going on,
> especially given that...

... that the `grep` really wants to test for the absence of the *commit*,
not the *tag* in .git/shallow?

Yes, you are right ;-)

So why did my test do the right thing, if it looked at a tag, but did not
dereference it via ^0? The answer is: the `test_commit` function creates
light-weight tags, i.e. no tag objects. And therefore, the $d^0 you found
below, that's just confusing.

I will change it so that the `rev-parse` call uses ^0 (even if it is
technically not necessary), to avoid said confusion.

Thanks,
Dscho

> 
> > +	git -C shallow-server branch branch-orig D^0 &&
> 
> ... this does that D^0 thing here to makes us wonder if D needs
> unwrapping before using it as a commit (not commit-ish). 
> 
> If we did so, we could use $d here instead of D^0.
> 
> > +	git -C shallow-client fetch --prune --depth=2 \
> > +		origin "+refs/heads/*:refs/remotes/origin/*"
> > +'
> > +
> >  . "$TEST_DIRECTORY"/lib-httpd.sh
> >  start_httpd
> 
>