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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 24, 2018, 03:39 UTC
Message-ID: <xmqqr2ggovxk.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <ed8559b91b7a3b51a5950d62e78fc726ed5b44c2.1540245934.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 9 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> A `git fetch --prune` can turn previously-reachable objects unreachable,
> even commits that are in the `shallow` list. A subsequent `git repack
> -ad` will then unceremoniously drop those unreachable commits, and the
> `shallow` list will become stale. This means that when we try to fetch
> with a larger `--depth` the next time, we may end up with:
>
> 	fatal: error in object: unshallow <commit-hash>

Nicely analysed and described. One minor thing nagged me in the implementation but it is not a big issue.

Show 6 quoted lines

> +...
> +	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, 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...

> +	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.

Show 6 quoted lines

> +	git -C shallow-client fetch --prune --depth=2 \
> +		origin "+refs/heads/*:refs/remotes/origin/*"
> +'
> +
>  . "$TEST_DIRECTORY"/lib-httpd.sh
>  start_httpd