Re: [PATCH 2/6] fsck: report trees as dangling

2017-01-17 Thread Junio C Hamano
Jeff King  writes:

> After checking connectivity, fsck looks through the list of
> any objects we've seen mentioned, and reports unreachable
> and un-"used" ones as dangling. However, it skips any object
> which is not marked as "parsed", as that is an object that
> we _don't_ have (but that somebody mentioned).
>
> Since 6e454b9a3 (clear parsed flag when we free tree
> buffers, 2013-06-05), that flag can't be relied on, and the
> correct method is to check the HAS_OBJ flag. The cleanup in
> that commit missed this callsite, though. As a result, we
> would generally fail to report dangling trees.
>
> We never noticed because there were no tests in this area
> (for trees or otherwise). Let's add some.
>
> Signed-off-by: Jeff King 
> ---

Makes sense, and the new test is very easy to read, too.

Queued; thanks.



[PATCH 2/6] fsck: report trees as dangling

2017-01-16 Thread Jeff King
After checking connectivity, fsck looks through the list of
any objects we've seen mentioned, and reports unreachable
and un-"used" ones as dangling. However, it skips any object
which is not marked as "parsed", as that is an object that
we _don't_ have (but that somebody mentioned).

Since 6e454b9a3 (clear parsed flag when we free tree
buffers, 2013-06-05), that flag can't be relied on, and the
correct method is to check the HAS_OBJ flag. The cleanup in
that commit missed this callsite, though. As a result, we
would generally fail to report dangling trees.

We never noticed because there were no tests in this area
(for trees or otherwise). Let's add some.

Signed-off-by: Jeff King 
---
 builtin/fsck.c  |  2 +-
 t/t1450-fsck.sh | 27 +++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index f01b81eeb..3e67203f9 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -225,7 +225,7 @@ static void check_unreachable_object(struct object *obj)
 * to complain about it being unreachable (since it does
 * not exist).
 */
-   if (!obj->parsed)
+   if (!(obj->flags & HAS_OBJ))
return;
 
/*
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 6eef8b28e..e88ec7747 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -559,4 +559,31 @@ test_expect_success 'fsck --name-objects' '
)
 '
 
+# for each of type, we have one version which is referenced by another object
+# (and so while unreachable, not dangling), and another variant which really is
+# dangling.
+test_expect_success 'fsck notices dangling objects' '
+   git init dangling &&
+   (
+   cd dangling &&
+   blob=$(echo not-dangling | git hash-object -w --stdin) &&
+   dblob=$(echo dangling | git hash-object -w --stdin) &&
+   tree=$(printf "100644 blob %s\t%s\n" $blob one | git mktree) &&
+   dtree=$(printf "100644 blob %s\t%s\n" $blob two | git mktree) &&
+   commit=$(git commit-tree $tree) &&
+   dcommit=$(git commit-tree -p $commit $tree) &&
+
+   cat >expect <<-EOF &&
+   dangling blob $dblob
+   dangling commit $dcommit
+   dangling tree $dtree
+   EOF
+
+   git fsck >actual &&
+   # the output order is non-deterministic, as it comes from a hash
+   sort actual.sorted &&
+   test_cmp expect actual.sorted
+   )
+'
+
 test_done
-- 
2.11.0.642.gd6f8cda6c