Re: [PATCH v3 22/22] t7700: stop losing return codes of git commands
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 23, 2019, 01:49 UTC
- Message-ID
- <xmqqftif2wg7.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <e9835b85427a3486e2dba136bbf34506e521d355.1574449072.git.liu.denton@gmail.com>
Denton Liu <liu.denton@gmail.com> writes:
> - objsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |
> - sed -e "s/^\([0-9a-f]\{40\}\).*/\1/") &&
> + git verify-pack -v pack-$packsha1.idx >packlist &&
> + objsha1=$(head -n 1 packlist | sed -e "s/^\([0-9a-f]\{40\}\).*/\1/") &&We probably should lose reference to SHA-1 and use $OID_REGEX; this is obviously a #leftoverbits material that is outside the scope of this series.
Show 10 quoted lines
> @@ -91,7 +93,8 @@ test_expect_success 'loose objects in alternate ODB are not repacked' ' > git prune-packed && > for p in .git/objects/pack/*.idx > do > - if git verify-pack -v $p | egrep "^$objsha1" > + git verify-pack -v $p >packlist || return $? > + if egrep "^$objsha1" packlist > then > found_duplicate_object=1 > echo "DUPLICATE OBJECT FOUND"
These egrep that try to match lines that begin with an object name can be a simple grep instead (again, outside the scope of this series).
Show 9 quoted lines
> @@ -109,15 +112,18 @@ test_expect_success 'packed obs in alt ODB are repacked even when local repo is
> test_path_is_file "$myidx" &&
> for p in alt_objects/pack/*.idx
> do
> - git verify-pack -v $p | sed -n -e "/^[0-9a-f]\{40\}/p"
> - done | while read sha1 rest
> + git verify-pack -v $p >packlist || return $?
> + sed -n -e "/^[0-9a-f]\{40\}/p"
> + done >packs &&A misleading filename? The lines in this file are not pack files; rather the file has a list of objects in various packs.
Show 12 quoted lines
> + git verify-pack -v $myidx >mypacklist && > + while read sha1 rest > do > - if ! ( git verify-pack -v $myidx | grep "^$sha1" ) > + if ! grep "^$sha1" mypacklist > then > echo "Missing object in local pack: $sha1" > return 1 > fi > - done > + done <packs > '
Again outside the scope of this series, but this looks O(n^2) to me.
If I were writing this today, I would prepare a sorted list of all object names (and nothing else on each line) in alt_objects/pack/ in one file (call it 'orig'), and prepare another file with a sorted list of all object names described in $myidx (call it 'dest'), and then run "comm -23 orig dest" and see if there is anything that is unique in the 'orig' file (i.e. something in 'orig' is missing from 'dest').
Show 21 quoted lines
> @@ -132,15 +138,18 @@ test_expect_success 'packed obs in alt ODB are repacked when local repo has pack
> test_path_is_file "$myidx" &&
> for p in alt_objects/pack/*.idx
> do
> - git verify-pack -v $p | sed -n -e "/^[0-9a-f]\{40\}/p"
> - done | while read sha1 rest
> + git verify-pack -v $p >packlist || return $?
> + sed -n -e "/^[0-9a-f]\{40\}/p" packlist
> + done >packs &&
> + git verify-pack -v $myidx >mypacklist &&
> + while read sha1 rest
> do
> - if ! ( git verify-pack -v $myidx | grep "^$sha1" )
> + if ! grep "^$sha1" mypacklist
> then
> echo "Missing object in local pack: $sha1"
> return 1
> fi
> - done
> + done <packs
> 'Likewise.
Show 21 quoted lines
> @@ -160,15 +169,18 @@ test_expect_success 'packed obs in alternate ODB kept pack are repacked' '
> test_path_is_file "$myidx" &&
> for p in alt_objects/pack/*.idx
> do
> - git verify-pack -v $p | sed -n -e "/^[0-9a-f]\{40\}/p"
> - done | while read sha1 rest
> + git verify-pack -v $p >packlist || return $?
> + sed -n -e "/^[0-9a-f]\{40\}/p" packlist
> + done >packs &&
> + git verify-pack -v $myidx >mypacklist &&
> + while read sha1 rest
> do
> - if ! ( git verify-pack -v $myidx | grep "^$sha1" )
> + if ! grep "^$sha1" mypacklist
> then
> echo "Missing object in local pack: $sha1"
> return 1
> fi
> - done
> + done <packs
> 'Likewise.