| Message ID | 20190609224400.41557-4-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Hash-independent tests, part 4 | expand |
On Sun, Jun 9, 2019 at 6:44 PM brian m. carlson <sandals@crustytoothpaste.net> wrote: > Replace several hard-coded full and partial object IDs with variables or > computed values. Create junk data to stuff inside an invalid tree that > can be either 20 or 32 bytes long. Compute a binary all-zeros object ID > instead of hard-coding a 20-byte length. > [...] > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh > @@ -9,6 +9,7 @@ test_description='git fsck random collection of tests > test_expect_success setup ' > + test_oid_init && > git config gc.auto 0 && > git config i18n.commitencoding ISO-8859-1 && > test_commit A fileA one && > @@ -16,7 +17,8 @@ test_expect_success setup ' > git checkout HEAD^0 && > test_commit B fileB two && > git tag -d A B && > - git reflog expire --expire=now --all > + git reflog expire --expire=now --all && > + test_oid_init > ' Why does this function call test_oid_init() twice?
On 2019-06-10 at 07:57:18, Eric Sunshine wrote: > On Sun, Jun 9, 2019 at 6:44 PM brian m. carlson > <sandals@crustytoothpaste.net> wrote: > > Replace several hard-coded full and partial object IDs with variables or > > computed values. Create junk data to stuff inside an invalid tree that > > can be either 20 or 32 bytes long. Compute a binary all-zeros object ID > > instead of hard-coding a 20-byte length. > > [...] > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > > --- > > diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh > > @@ -9,6 +9,7 @@ test_description='git fsck random collection of tests > > test_expect_success setup ' > > + test_oid_init && > > git config gc.auto 0 && > > git config i18n.commitencoding ISO-8859-1 && > > test_commit A fileA one && > > @@ -16,7 +17,8 @@ test_expect_success setup ' > > git checkout HEAD^0 && > > test_commit B fileB two && > > git tag -d A B && > > - git reflog expire --expire=now --all > > + git reflog expire --expire=now --all && > > + test_oid_init > > ' > > Why does this function call test_oid_init() twice? Probably because this was a squash of a couple different patches. I'll reroll with that fixed.
> @@ -84,7 +86,7 @@ test_expect_success 'branch pointing to non-commit' ' > test_expect_success 'HEAD link pointing at a funny object' ' > test_when_finished "mv .git/SAVED_HEAD .git/HEAD" && > mv .git/HEAD .git/SAVED_HEAD && > - echo 0000000000000000000000000000000000000000 >.git/HEAD && > + echo $ZERO_OID >.git/HEAD && > # avoid corrupt/broken HEAD from interfering with repo discovery > test_must_fail env GIT_DIR=.git git fsck 2>out && > cat out && ZERO_OID doesn't seem redefined to the SHA256 variant when being tested under SHA256. Maybe you need a test_oid invocation here. I couldn't verify this, though - do you know if there is a way for me to run the tests with SHA256 instead of SHA1? > @@ -417,13 +426,12 @@ test_expect_success 'force fsck to ignore double author' ' > ' > > _bz='\0' > -_bz5="$_bz$_bz$_bz$_bz$_bz" > -_bz20="$_bz5$_bz5$_bz5$_bz5" > +_bzoid=$(printf $ZERO_OID | sed -e 's/00/\\0/g') Same comment here. > @@ -631,10 +639,12 @@ test_expect_success 'fsck --name-objects' ' > > test_expect_success 'alternate objects are correctly blamed' ' > test_when_finished "rm -rf alt.git .git/objects/info/alternates" && > + path=$(test_oid numeric) && > + path=$(test_oid_to_path "$path") && Double assignment to path?
On Tue, Jun 11, 2019 at 7:03 PM Jonathan Tan <jonathantanmy@google.com> wrote: > > test_expect_success 'alternate objects are correctly blamed' ' > > test_when_finished "rm -rf alt.git .git/objects/info/alternates" && > > + path=$(test_oid numeric) && > > + path=$(test_oid_to_path "$path") && > > Double assignment to path? I tripped over this, as well, when reading the patch, but if you look closely, the second assignment is "refining" a value computed in first assignment. It would have been clearer if written as: name=$(test_oid numeric) && path=$(test_oid_to_path "$name") && or: path=$(test_oid_to_path $(test_oid numeric))
On 2019-06-11 at 23:02:55, Jonathan Tan wrote: > > @@ -84,7 +86,7 @@ test_expect_success 'branch pointing to non-commit' ' > > test_expect_success 'HEAD link pointing at a funny object' ' > > test_when_finished "mv .git/SAVED_HEAD .git/HEAD" && > > mv .git/HEAD .git/SAVED_HEAD && > > - echo 0000000000000000000000000000000000000000 >.git/HEAD && > > + echo $ZERO_OID >.git/HEAD && > > # avoid corrupt/broken HEAD from interfering with repo discovery > > test_must_fail env GIT_DIR=.git git fsck 2>out && > > cat out && > > ZERO_OID doesn't seem redefined to the SHA256 variant when being tested > under SHA256. Maybe you need a test_oid invocation here. That's actually coming in a later series. Eventually, test_oid_init will be called automatically and ZERO_OID will be set by calling test_oid with an appropriate value. > I couldn't verify this, though - do you know if there is a way for me to > run the tests with SHA256 instead of SHA1? There isn't any way in Junio's tree, mostly because there are still a lot of places that need fixing. However, all of those commits are in my transition-stage-4 branch at https://github.com/bk2204/git.git, and you can set the environment variable GIT_TEST_DEFAULT_HASH to "sha256" and it will run the test suite with SHA-256. That branch has a fully functional SHA-256 Git if you'd like to test it out, and the same binary can handle SHA-1 and SHA-256. Interoperability between SHA-1 and SHA-256 repos hasn't been implemented yet, though. > > @@ -631,10 +639,12 @@ test_expect_success 'fsck --name-objects' ' > > > > test_expect_success 'alternate objects are correctly blamed' ' > > test_when_finished "rm -rf alt.git .git/objects/info/alternates" && > > + path=$(test_oid numeric) && > > + path=$(test_oid_to_path "$path") && > > Double assignment to path? Good point. Will fix.
On 2019-06-11 at 23:20:31, Eric Sunshine wrote: > On Tue, Jun 11, 2019 at 7:03 PM Jonathan Tan <jonathantanmy@google.com> wrote: > > > test_expect_success 'alternate objects are correctly blamed' ' > > > test_when_finished "rm -rf alt.git .git/objects/info/alternates" && > > > + path=$(test_oid numeric) && > > > + path=$(test_oid_to_path "$path") && > > > > Double assignment to path? > > I tripped over this, as well, when reading the patch, but if you look > closely, the second assignment is "refining" a value computed in first > assignment. It would have been clearer if written as: > > name=$(test_oid numeric) && > path=$(test_oid_to_path "$name") && > > or: > > path=$(test_oid_to_path $(test_oid numeric)) I think in general people have discouraged the latter form because it doesn't fail predictably if the inner command substitution fails. I'll rename the first variable so that it's less surprising.
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 0f268a3664..f66a39580c 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -9,6 +9,7 @@ test_description='git fsck random collection of tests . ./test-lib.sh test_expect_success setup ' + test_oid_init && git config gc.auto 0 && git config i18n.commitencoding ISO-8859-1 && test_commit A fileA one && @@ -16,7 +17,8 @@ test_expect_success setup ' git checkout HEAD^0 && test_commit B fileB two && git tag -d A B && - git reflog expire --expire=now --all + git reflog expire --expire=now --all && + test_oid_init ' test_expect_success 'loose objects borrowed from alternate are not missing' ' @@ -54,8 +56,8 @@ test_expect_success 'setup: helpers for corruption tests' ' test_expect_success 'object with bad sha1' ' sha=$(echo blob | git hash-object -w --stdin) && - old=$(echo $sha | sed "s+^..+&/+") && - new=$(dirname $old)/ffffffffffffffffffffffffffffffffffffff && + old=$(test_oid_to_path "$sha") && + new=$(dirname $old)/$(test_oid ff_2) && sha="$(dirname $new)$(basename $new)" && mv .git/objects/$old .git/objects/$new && test_when_finished "remove_object $sha" && @@ -84,7 +86,7 @@ test_expect_success 'branch pointing to non-commit' ' test_expect_success 'HEAD link pointing at a funny object' ' test_when_finished "mv .git/SAVED_HEAD .git/HEAD" && mv .git/HEAD .git/SAVED_HEAD && - echo 0000000000000000000000000000000000000000 >.git/HEAD && + echo $ZERO_OID >.git/HEAD && # avoid corrupt/broken HEAD from interfering with repo discovery test_must_fail env GIT_DIR=.git git fsck 2>out && cat out && @@ -244,10 +246,16 @@ test_expect_success 'tree object with duplicate entries' ' ' test_expect_success 'unparseable tree object' ' + test_oid_cache <<-\EOF && + junk sha1:twenty-bytes-of-junk + junk sha256:twenty-bytes-of-junk-twelve-more + EOF + test_when_finished "git update-ref -d refs/heads/wrong" && test_when_finished "remove_object \$tree_sha1" && test_when_finished "remove_object \$commit_sha1" && - tree_sha1=$(printf "100644 \0twenty-bytes-of-junk" | git hash-object -t tree --stdin -w --literally) && + junk=$(test_oid junk) && + tree_sha1=$(printf "100644 \0$junk" | git hash-object -t tree --stdin -w --literally) && commit_sha1=$(git commit-tree $tree_sha1) && git update-ref refs/heads/wrong $commit_sha1 && test_must_fail git fsck 2>out && @@ -275,8 +283,9 @@ test_expect_success 'tree entry with type mismatch' ' ' test_expect_success 'tag pointing to nonexistent' ' - cat >invalid-tag <<-\EOF && - object ffffffffffffffffffffffffffffffffffffffff + badoid=$(test_oid deadbeef) && + cat >invalid-tag <<-EOF && + object $badoid type commit tag invalid tagger T A Gger <tagger@example.com> 1234567890 -0000 @@ -386,8 +395,8 @@ test_expect_success 'rev-list --verify-objects' ' test_expect_success 'rev-list --verify-objects with bad sha1' ' sha=$(echo blob | git hash-object -w --stdin) && - old=$(echo $sha | sed "s+^..+&/+") && - new=$(dirname $old)/ffffffffffffffffffffffffffffffffffffff && + old=$(test_oid_to_path $sha) && + new=$(dirname $old)/$(test_oid ff_2) && sha="$(dirname $new)$(basename $new)" && mv .git/objects/$old .git/objects/$new && test_when_finished "remove_object $sha" && @@ -402,7 +411,7 @@ test_expect_success 'rev-list --verify-objects with bad sha1' ' test_might_fail git rev-list --verify-objects refs/heads/bogus >/dev/null 2>out && cat out && - test_i18ngrep -q "error: hash mismatch 63ffffffffffffffffffffffffffffffffffffff" out + test_i18ngrep -q "error: hash mismatch $(dirname $new)$(test_oid ff_2)" out ' test_expect_success 'force fsck to ignore double author' ' @@ -417,13 +426,12 @@ test_expect_success 'force fsck to ignore double author' ' ' _bz='\0' -_bz5="$_bz$_bz$_bz$_bz$_bz" -_bz20="$_bz5$_bz5$_bz5$_bz5" +_bzoid=$(printf $ZERO_OID | sed -e 's/00/\\0/g') test_expect_success 'fsck notices blob entry pointing to null sha1' ' (git init null-blob && cd null-blob && - sha=$(printf "100644 file$_bz$_bz20" | + sha=$(printf "100644 file$_bz$_bzoid" | git hash-object -w --stdin -t tree) && git fsck 2>out && cat out && @@ -434,7 +442,7 @@ test_expect_success 'fsck notices blob entry pointing to null sha1' ' test_expect_success 'fsck notices submodule entry pointing to null sha1' ' (git init null-commit && cd null-commit && - sha=$(printf "160000 submodule$_bz$_bz20" | + sha=$(printf "160000 submodule$_bz$_bzoid" | git hash-object -w --stdin -t tree) && git fsck 2>out && cat out && @@ -586,7 +594,7 @@ test_expect_success 'fsck --connectivity-only' ' # 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 && + empty=.git/objects/$(test_oid_to_path $EMPTY_BLOB) && blob=$(echo unrelated | git hash-object -w --stdin) && mv -f $(sha1_file $blob) $empty && @@ -631,10 +639,12 @@ test_expect_success 'fsck --name-objects' ' test_expect_success 'alternate objects are correctly blamed' ' test_when_finished "rm -rf alt.git .git/objects/info/alternates" && + path=$(test_oid numeric) && + path=$(test_oid_to_path "$path") && git init --bare alt.git && echo "../../alt.git/objects" >.git/objects/info/alternates && - mkdir alt.git/objects/12 && - >alt.git/objects/12/34567890123456789012345678901234567890 && + mkdir alt.git/objects/$(dirname $path) && + >alt.git/objects/$(dirname $path)/$(basename $path) && test_must_fail git fsck >out 2>&1 && test_i18ngrep alt.git out '
Replace several hard-coded full and partial object IDs with variables or computed values. Create junk data to stuff inside an invalid tree that can be either 20 or 32 bytes long. Compute a binary all-zeros object ID instead of hard-coding a 20-byte length. Additionally, compute various object IDs by using test_oid and $EMPTY_BLOB so that this test works with multiple hash algorithms. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- t/t1450-fsck.sh | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-)