Re: [PATCH 1/6] t1450: clean up sub-objects in duplicate-entry test

2017-01-17 Thread Jeff King
On Tue, Jan 17, 2017 at 12:52:43PM -0800, Junio C Hamano wrote:

> > Since the setup code happens inside a subshell, we can't
> > just set a variable for each object. However, we can stuff
> > all of the sha1s into the $T output variable, which is not
> > used for anything except cleanup.
> >
> > Signed-off-by: Jeff King 
> > ---
> >  t/t1450-fsck.sh | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Thanks.  
> 
> It is tempting to move this loop to remove_object, but that is not
> necessary while the user is only this one.

I agree it would be less gross. I avoided it because I knew that I
hacked up remove_object() in the other topic.

-Peff


Re: [PATCH 1/6] t1450: clean up sub-objects in duplicate-entry test

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

> This test creates a multi-level set of trees, but its
> cleanup routine only removes the top-level tree. After the
> test finishes, the inner tree and the blob it points to
> remain, making the inner tree dangling.
>
> A later test ("cleaned up") verifies that we've removed any
> cruft and "git fsck" output is clean. This passes only
> because of a bug in git-fsck which fails to notice dangling
> trees.
>
> In preparation for fixing the bug, let's teach this earlier
> test to clean up after itself correctly. We have to remove
> the inner tree (and therefore the blob, too, which becomes
> dangling after removing that tree).
>
> Since the setup code happens inside a subshell, we can't
> just set a variable for each object. However, we can stuff
> all of the sha1s into the $T output variable, which is not
> used for anything except cleanup.
>
> Signed-off-by: Jeff King 
> ---
>  t/t1450-fsck.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks.  

It is tempting to move this loop to remove_object, but that is not
necessary while the user is only this one.

>
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index ee7d4736d..6eef8b28e 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -189,14 +189,16 @@ test_expect_success 'commit with NUL in header' '
>  '
>  
>  test_expect_success 'tree object with duplicate entries' '
> - test_when_finished "remove_object \$T" &&
> + test_when_finished "for i in \$T; do remove_object \$i; done" &&
>   T=$(
>   GIT_INDEX_FILE=test-index &&
>   export GIT_INDEX_FILE &&
>   rm -f test-index &&
>   >x &&
>   git add x &&
> + git rev-parse :x &&
>   T=$(git write-tree) &&
> + echo $T &&
>   (
>   git cat-file tree $T &&
>   git cat-file tree $T