Jeff King <peff@xxxxxxxx> writes:
> +static void mark_object_for_connectivity(const unsigned char *sha1)
> +{
> + struct object *obj = lookup_object(sha1);
> +
> + /*
> + * Setting the object type here isn't strictly necessary here for a
> + * connectivity check.
Drop one of these "here"s?
> The cmd_fsck() part of the diff is pretty nasty without
> "-b".
True. I also wonder if swapping the if/else arms make the end
result and the patch easier to read. i.e.
+ if (connectivity_only) {
+ mark loose for connectivity;
+ mark packed for connectivity;
+ } else {
... existing code comes here reindented ...
}
But the patch makes sense.
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index e88ec7747..c1b2dda33 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -523,9 +523,21 @@ test_expect_success 'fsck --connectivity-only' '
> touch empty &&
> git add empty &&
> test_commit empty &&
> +
> + # Drop the index now; we want to be sure that we
> + # recursively notice that we notice the broken objects
> + # because they are reachable from refs, not because
> + # they are in the index.
> + rm -f .git/index &&
> +
> + # corrupt the blob, but in a way that we can still identify
> + # its type. That lets us see that --connectivity-only is
> + # not actually looking at the contents, but leaves it
> + # free to examine the type if it chooses.
> empty=.git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 &&
> - rm -f $empty &&
> - echo invalid >$empty &&
> + blob=$(echo unrelated | git hash-object -w --stdin) &&
> + mv $(sha1_file $blob) $empty &&
> +
> test_must_fail git fsck --strict &&
> git fsck --strict --connectivity-only &&
> tree=$(git rev-parse HEAD:) &&
> @@ -537,6 +549,13 @@ test_expect_success 'fsck --connectivity-only' '
> )
> '
>
> +test_expect_success 'fsck --connectivity-only with explicit head' '
> + (
> + cd connectivity-only &&
> + test_must_fail git fsck --connectivity-only $tree
> + )
> +'
Most of the earlier "tree=..." assignments are done in subshells,
and it is not clear which tree this refers to. Is this the one that
was written in 'rev-list --verify-objects with bad sha1' that has
been removed in its when-finished handler?
> remove_loose_object () {
> sha1="$(git rev-parse "$1")" &&
> remainder=${sha1#??} &&