On Wed, Jan 30, 2019 at 6:21 PM Max Kirillov <max@xxxxxxxxxx> wrote:
> If packed-refs is marked as sorted but not really sorted it causes
> very hard to comprehend misbehavior of reference resolving - a reference
> is reported as not found.
>
> As the scope of the issue is not clear, make it visible by failing
> pack-refs command - the one which would not suffer performance penalty
> to verify the sortedness - when it encounters not really sorted existing
> data.
>
> Signed-off-by: Max Kirillov <max@xxxxxxxxxx>
> ---
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> @@ -1088,6 +1088,7 @@ static int write_with_updates(struct packed_ref_store *refs,
> + struct strbuf prev_ref = STRBUF_INIT;
> @@ -1137,6 +1138,20 @@ static int write_with_updates(struct packed_ref_store *refs,
> + if (iter)
> + {
> + if (prev_ref.len && strcmp(prev_ref.buf, iter->refname) > 0)
> + {
> + strbuf_addf(err, "broken sorting in packed-refs: '%s' > '%s'",
> + prev_ref.buf,
> + iter->refname);
strbuf_release(&prev_ref) either here or after the "error" label.
> + goto error;
> + }
> +
> + strbuf_init(&prev_ref, 0);
> + strbuf_addstr(&prev_ref, iter->refname);
> + }
> diff --git a/t/t3212-pack-refs-broken-sorting.sh b/t/t3212-pack-refs-broken-sorting.sh
> @@ -0,0 +1,26 @@
> +test_expect_success 'setup' '
> + git commit --allow-empty -m commit &&
> + for num in $(test_seq 10)
> + do
> + git branch b$(printf "%02d" $num) || break
This should probably be "|| return 1" rather than "|| break" in order
to fail the test immediately.
> + done &&
> + git pack-refs --all &&
> + head_object=$(git rev-parse HEAD) &&
> + printf "$head_object refs/heads/b00\\n" >>.git/packed-refs &&
> + git branch b11
> +'
> +
> +test_expect_success 'off-order branch not found' '
> + ! git show-ref --verify --quiet refs/heads/b00
> +'
Use test_must_fail() rather than '!' when expecting a Git command to fail.
> +test_expect_success 'subsequent pack-refs fails' '
> + ! git pack-refs --all
> +'
Ditto.
