{"thread":{"id":"49164","subject":"[PATCH v2 00/11] Hash-independent tests (part 3)","startedAt":"2018-08-19T17:54:12Z","lastAt":"2018-08-20T14:30:26Z","messageCount":24,"participants":["brian m. carlson","Eric Sunshine","Torsten Bögershausen"],"isPatch":true,"patchVersion":2,"patchTotal":11},"messages":[{"id":"356038","messageId":"20180819175351.449973-1-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":null,"subject":"[PATCH v2 00/11] Hash-independent tests (part 3)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:40Z","receivedAt":"2018-08-19T17:54:12Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This is next in the series of improvements to make tests\nhash-independent.\n\nThis round modifies the helpers to more closely follow the\nlighter-weight design that Eric Sunshine suggested in v1, at the cost of\nrequiring lookup keys to be shell identifiers.  If that's judged to be\nundesirable, I can always hash the key before use.\n\nSince we've decided on SHA-256, I put in an extra commit to update t0000\nand remove the use of the SHA1 prerequisite.  As described in the commit\nmessage, I wrote a Ruby script to synthesize the SHA-1 and SHA-256 blobs\nand then used a second one to generate the trees, using the same\nmethodology for both algorithms. I did this specifically to ensure that\nthe object IDs are exactly the ones we think they are.\n\nChanges from v1:\n* Adopt pure shell approach for helper.\n* Add tests for the helpers.\n* Explicitly refer to SHA-256 now that we know it will be NewHash.\n* Updated t0000 to remove SHA1 prerequisite.\n* Change name of helper from test_translate to test_oid.\n* Add helper to cache information in the shell.\n* Simplified lookup of HEAD in t0002.\n* Switched to using existing helper function in t0027.\n* Simplified handling of IDs in t0064.\n\nbrian m. carlson (11):\n  t: add tool to translate hash-related values\n  t0000: use hash translation table\n  t0000: update tests for SHA-256\n  t0002: abstract away SHA-1 specific constants\n  t0027: make hash size independent\n  t0064: make hash size independent\n  t1006: make hash size independent\n  t1400: switch hard-coded object ID to variable\n  t1405: make hash size independent\n  t1406: make hash-size independent\n  t1407: make hash size independent\n\n t/oid-info/hash-info           |   8 ++\n t/oid-info/oid                 |  29 ++++++\n t/oid-info/t0000               |  38 ++++++++\n t/t0000-basic.sh               | 168 ++++++++++++++++++++-------------\n t/t0002-gitfile.sh             |  26 ++---\n t/t0027-auto-crlf.sh           |   3 +-\n t/t0064-sha1-array.sh          |  49 +++++-----\n t/t1006-cat-file.sh            |   6 +-\n t/t1400-update-ref.sh          |   2 +-\n t/t1405-main-ref-store.sh      |   4 +-\n t/t1406-submodule-ref-store.sh |   6 +-\n t/t1407-worktree-ref-store.sh  |   4 +-\n t/test-lib-functions.sh        |  36 +++++++\n 13 files changed, 267 insertions(+), 112 deletions(-)\n create mode 100644 t/oid-info/hash-info\n create mode 100644 t/oid-info/oid\n create mode 100644 t/oid-info/t0000\n\n"},{"id":"356039","messageId":"20180819175351.449973-2-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 01/11] t: add tool to translate hash-related values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:41Z","receivedAt":"2018-08-19T17:54:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Add a test function helper, test_oid, that produces output that varies\ndepending on the hash in use.  Add an additional helper, test_oid_cache,\nthat can be used to load data for test_oid, either through a list of\nfilenames or on standard input.  Check that these functions work in\nt0000, as the rest of the testsuite will soon come to depend on them.\n\nImplement two basic lookup charts, one for common invalid or synthesized\nobject IDs, and one for various facts about the hash function in use.\nIndividual tests can use their own translation files to map object IDs\nor other data that needs to vary across hash functions.  Provide\nversions for both SHA-1 and SHA-256.\n\nNote that due to the implementation used, keys cannot be anything but\nshell identifier characters.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/oid-info/hash-info    |  8 ++++++++\n t/oid-info/oid          | 29 +++++++++++++++++++++++++++++\n t/t0000-basic.sh        | 29 +++++++++++++++++++++++++++++\n t/test-lib-functions.sh | 36 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 102 insertions(+)\n create mode 100644 t/oid-info/hash-info\n create mode 100644 t/oid-info/oid\n\ndiff --git a/t/oid-info/hash-info b/t/oid-info/hash-info\nnew file mode 100644\nindex 0000000000..ccdbfdf974\n--- /dev/null\n+++ b/t/oid-info/hash-info\n@@ -0,0 +1,8 @@\n+rawsz sha1:20\n+rawsz sha256:32\n+\n+hexsz sha1:40\n+hexsz sha256:64\n+\n+zero sha1:0000000000000000000000000000000000000000\n+zero sha256:0000000000000000000000000000000000000000000000000000000000000000\ndiff --git a/t/oid-info/oid b/t/oid-info/oid\nnew file mode 100644\nindex 0000000000..6f1c49dbc1\n--- /dev/null\n+++ b/t/oid-info/oid\n@@ -0,0 +1,29 @@\n+# These are some common invalid and partial object IDs used in tests.\n+001\tsha1:0000000000000000000000000000000000000001\n+001\tsha256:0000000000000000000000000000000000000000000000000000000000000001\n+002\tsha1:0000000000000000000000000000000000000002\n+002\tsha256:0000000000000000000000000000000000000000000000000000000000000002\n+003\tsha1:0000000000000000000000000000000000000003\n+003\tsha256:0000000000000000000000000000000000000000000000000000000000000003\n+004\tsha1:0000000000000000000000000000000000000004\n+004\tsha256:0000000000000000000000000000000000000000000000000000000000000004\n+005\tsha1:0000000000000000000000000000000000000005\n+005\tsha256:0000000000000000000000000000000000000000000000000000000000000005\n+006\tsha1:0000000000000000000000000000000000000006\n+006\tsha256:0000000000000000000000000000000000000000000000000000000000000006\n+007\tsha1:0000000000000000000000000000000000000007\n+007\tsha256:0000000000000000000000000000000000000000000000000000000000000007\n+# All zeros or Fs missing one or two hex segments.\n+zero_1\t\tsha1:000000000000000000000000000000000000000\n+zero_2\t\tsha256:000000000000000000000000000000000000000000000000000000000000000\n+zero_2\t\tsha1:00000000000000000000000000000000000000\n+zero_2\t\tsha256:00000000000000000000000000000000000000000000000000000000000000\n+ff_1\t\tsha1:fffffffffffffffffffffffffffffffffffffff\n+ff_1\t\tsha256:fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\n+ff_2\t\tsha1:ffffffffffffffffffffffffffffffffffffff\n+ff_2\t\tsha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\n+# More various invalid OIDs.\n+numeric\t\tsha1:0123456789012345678901234567890123456789\n+numeric\t\tsha256:0123456789012345678901234567890123456789012345678901234567890123\n+deadbeef\tsha1:deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n+deadbeef\tsha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 34859fe4a5..ace779bf7d 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -821,6 +821,35 @@ test_expect_success 'tests clean up even on failures' \"\n \tEOF\n \"\n \n+test_oid_cache hash-info oid\n+\n+test_expect_success 'test_oid_info provides sane info by default' '\n+\ttest_oid zero &&\n+\ttest_oid zero >actual &&\n+\tgrep \"00*\" actual &&\n+\ttest \"$(test_oid hexsz)\" -eq $(wc -c <actual) &&\n+\ttest $(( $(test_oid rawsz) * 2)) -eq \"$(test_oid hexsz)\"\n+'\n+\n+test_expect_success 'test_oid_info can look up data for SHA-1' '\n+\ttest_detect_hash sha1 &&\n+\ttest_oid zero >actual &&\n+\tgrep \"00*\" actual &&\n+\ttest $(wc -c <actual) -eq 40 &&\n+\ttest \"$(test_oid rawsz)\" -eq 20 &&\n+\ttest \"$(test_oid hexsz)\" -eq 40\n+'\n+\n+test_expect_success 'test_oid_info can look up data for SHA-256' '\n+\ttest_when_finished \"test_detect_hash\" &&\n+\ttest_detect_hash sha256 &&\n+\ttest_oid zero >actual &&\n+\tgrep \"00*\" actual &&\n+\ttest $(wc -c <actual) -eq 64 &&\n+\ttest \"$(test_oid rawsz)\" -eq 32 &&\n+\ttest \"$(test_oid hexsz)\" -eq 64\n+'\n+\n ################################################################\n # Basics of the basics\n \ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 2b2181dca0..ac18789a70 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1147,3 +1147,39 @@ depacketize () {\n \t\t}\n \t'\n }\n+\n+test_detect_hash () {\n+\tif test -n \"$1\"\n+\tthen\n+\t\ttest_hash_algo=\"$1\"\n+\telse\n+\t\ttest_hash_algo='sha1'\n+\tfi\n+}\n+\n+test_oid_cache () {\n+\ttest -n \"$test_hash_algo\" || test_detect_hash\n+\tif test -n \"$1\"\n+\tthen\n+\t\twhile test -n \"$1\"\n+\t\tdo\n+\t\t\ttest_oid_cache <\"$TEST_DIRECTORY/oid-info/$1\"\n+\t\t\tshift\n+\t\tdone\n+\t\treturn $?\n+\tfi\n+\twhile read _tag _rest\n+\tdo\n+\t\tcase $_tag in \\#*)\n+\t\t\tcontinue;;\n+\t\tesac\n+\n+\t\t_k=\"${_rest%:*}\"\n+\t\t_v=\"${_rest#*:}\"\n+\t\teval \"test_oid_${_k}_$_tag=\\\"$_v\\\"\"\n+\tdone\n+}\n+\n+test_oid () {\n+\teval \"printf '%s' \\\"\\${test_oid_${test_hash_algo}_$1}\\\"\"\n+}\n"},{"id":"356040","messageId":"20180819175351.449973-3-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 02/11] t0000: use hash translation table","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:42Z","receivedAt":"2018-08-19T17:54:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"If the hash we're using is 32 bytes in size, attempting to insert a\n20-byte object name won't work.  Since these are synthesized objects\nthat are almost all zeros, look them up in a translation table.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t0000-basic.sh | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex ace779bf7d..57d374f2dc 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -1007,12 +1007,13 @@ test_expect_success SHA1 'validate object ID for a known tree' '\n \n test_expect_success 'put invalid objects into the index' '\n \trm -f .git/index &&\n-\tcat >badobjects <<-\\EOF &&\n-\t100644 blob 1000000000000000000000000000000000000000\tdir/file1\n-\t100644 blob 2000000000000000000000000000000000000000\tdir/file2\n-\t100644 blob 3000000000000000000000000000000000000000\tdir/file3\n-\t100644 blob 4000000000000000000000000000000000000000\tdir/file4\n-\t100644 blob 5000000000000000000000000000000000000000\tdir/file5\n+\tsuffix=$(echo $ZERO_OID | sed -e \"s/^.//\") &&\n+\tcat >badobjects <<-EOF &&\n+\t100644 blob $(test_oid 001)\tdir/file1\n+\t100644 blob $(test_oid 002)\tdir/file2\n+\t100644 blob $(test_oid 003)\tdir/file3\n+\t100644 blob $(test_oid 004)\tdir/file4\n+\t100644 blob $(test_oid 005)\tdir/file5\n \tEOF\n \tgit update-index --index-info <badobjects\n '\n"},{"id":"356041","messageId":"20180819175351.449973-4-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 03/11] t0000: update tests for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:43Z","receivedAt":"2018-08-19T17:54:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Test t0000 tests the \"basics of the basics\" and as such, checks that we\nhave various fixed hard-coded object IDs.  The tests relying on these\nassertions have been marked with the SHA1 prerequisite, as they will\nobviously not function in their current form with SHA-256.\n\nUse the test_oid helper to update these assertions and provide values\nfor both SHA-1 and SHA-256.\n\nThese object IDs were synthesized using a set of scripts that created\nthe objects for both SHA-1 and SHA-256 using the same method to ensure\nthat they are indeed the correct values.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/oid-info/t0000 |  38 ++++++++++++++\n t/t0000-basic.sh | 126 ++++++++++++++++++++++++-----------------------\n 2 files changed, 103 insertions(+), 61 deletions(-)\n create mode 100644 t/oid-info/t0000\n\ndiff --git a/t/oid-info/t0000 b/t/oid-info/t0000\nnew file mode 100644\nindex 0000000000..e62aefba4d\n--- /dev/null\n+++ b/t/oid-info/t0000\n@@ -0,0 +1,38 @@\n+path0f sha1:f87290f8eb2cbbea7857214459a0739927eab154\n+path0f sha256:638106af7c38be056f3212cbd7ac65bc1bac74f420ca5a436ff006a9d025d17d\n+\n+path0s sha1:15a98433ae33114b085f3eb3bb03b832b3180a01\n+path0s sha256:3a24cc53cf68edddac490bbf94a418a52932130541361f685df685e41dd6c363\n+\n+path2f sha1:3feff949ed00a62d9f7af97c15cd8a30595e7ac7\n+path2f sha256:2a7f36571c6fdbaf0e3f62751a0b25a3f4c54d2d1137b3f4af9cb794bb498e5f\n+\n+path2s sha1:d8ce161addc5173867a3c3c730924388daedbc38\n+path2s sha256:18fd611b787c2e938ddcc248fabe4d66a150f9364763e9ec133dd01d5bb7c65a\n+\n+path2d sha1:58a09c23e2ca152193f2786e06986b7b6712bdbe\n+path2d sha256:00e4b32b96e7e3d65d79112dcbea53238a22715f896933a62b811377e2650c17\n+\n+path3f sha1:0aa34cae68d0878578ad119c86ca2b5ed5b28376\n+path3f sha256:09f58616b951bd571b8cb9dc76d372fbb09ab99db2393f5ab3189d26c45099ad\n+\n+path3s sha1:8599103969b43aff7e430efea79ca4636466794f\n+path3s sha256:fce1aed087c053306f3f74c32c1a838c662bbc4551a7ac2420f5d6eb061374d0\n+\n+path3d sha1:21ae8269cacbe57ae09138dcc3a2887f904d02b3\n+path3d sha256:9b60497be959cb830bf3f0dc82bcc9ad9e925a24e480837ade46b2295e47efe1\n+\n+subp3f sha1:00fb5908cb97c2564a9783c0c64087333b3b464f\n+subp3f sha256:a1a9e16998c988453f18313d10375ee1d0ddefe757e710dcae0d66aa1e0c58b3\n+\n+subp3s sha1:6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c\n+subp3s sha256:81759d9f5e93c6546ecfcadb560c1ff057314b09f93fe8ec06e2d8610d34ef10\n+\n+subp3d sha1:3c5e5399f3a333eddecce7a9b9465b63f65f51e2\n+subp3d sha256:76b4ef482d4fa1c754390344cf3851c7f883b27cf9bc999c6547928c46aeafb7\n+\n+root sha1:087704a96baf1c2d1c869a8b084481e121c88b5b\n+root sha256:9481b52abab1b2ffeedbf9de63ce422b929f179c1b98ff7bee5f8f1bc0710751\n+\n+simpletree sha1:7bb943559a305bdd6bdee2cef6e5df2413c3d30a\n+simpletree sha256:1710c07a6c86f9a3c7376364df04c47ee39e5a5e221fcdd84b743bc9bb7e2bc5\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 57d374f2dc..180576293a 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -853,6 +853,8 @@ test_expect_success 'test_oid_info can look up data for SHA-256' '\n ################################################################\n # Basics of the basics\n \n+test_oid_cache t0000\n+\n # updating a new file without --add should fail.\n test_expect_success 'git update-index without --add should fail adding' '\n \ttest_must_fail git update-index should-be-empty\n@@ -868,8 +870,9 @@ test_expect_success 'writing tree out with git write-tree' '\n '\n \n # we know the shape and contents of the tree and know the object ID for it.\n-test_expect_success SHA1 'validate object ID of a known tree' '\n-\ttest \"$tree\" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a\n+test_expect_success 'validate object ID of a known tree' '\n+echo $tree &&\n+\ttest \"$tree\" = $(test_oid simpletree)\n     '\n \n # Removing paths.\n@@ -911,17 +914,18 @@ test_expect_success 'showing stage with git ls-files --stage' '\n \tgit ls-files --stage >current\n '\n \n-test_expect_success SHA1 'validate git ls-files output for a known tree' '\n-\tcat >expected <<-\\EOF &&\n-\t100644 f87290f8eb2cbbea7857214459a0739927eab154 0\tpath0\n-\t120000 15a98433ae33114b085f3eb3bb03b832b3180a01 0\tpath0sym\n-\t100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0\tpath2/file2\n-\t120000 d8ce161addc5173867a3c3c730924388daedbc38 0\tpath2/file2sym\n-\t100644 0aa34cae68d0878578ad119c86ca2b5ed5b28376 0\tpath3/file3\n-\t120000 8599103969b43aff7e430efea79ca4636466794f 0\tpath3/file3sym\n-\t100644 00fb5908cb97c2564a9783c0c64087333b3b464f 0\tpath3/subp3/file3\n-\t120000 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c 0\tpath3/subp3/file3sym\n+test_expect_success 'validate git ls-files output for a known tree' '\n+\tcat >expected <<-EOF &&\n+\t100644 $(test_oid path0f) 0\tpath0\n+\t120000 $(test_oid path0s) 0\tpath0sym\n+\t100644 $(test_oid path2f) 0\tpath2/file2\n+\t120000 $(test_oid path2s) 0\tpath2/file2sym\n+\t100644 $(test_oid path3f) 0\tpath3/file3\n+\t120000 $(test_oid path3s) 0\tpath3/file3sym\n+\t100644 $(test_oid subp3f) 0\tpath3/subp3/file3\n+\t120000 $(test_oid subp3s) 0\tpath3/subp3/file3sym\n \tEOF\n+\tcat expected &&\n \ttest_cmp expected current\n '\n \n@@ -929,20 +933,20 @@ test_expect_success 'writing tree out with git write-tree' '\n \ttree=$(git write-tree)\n '\n \n-test_expect_success SHA1 'validate object ID for a known tree' '\n-\ttest \"$tree\" = 087704a96baf1c2d1c869a8b084481e121c88b5b\n+test_expect_success 'validate object ID for a known tree' '\n+\ttest \"$tree\" = $(test_oid root)\n '\n \n test_expect_success 'showing tree with git ls-tree' '\n     git ls-tree $tree >current\n '\n \n-test_expect_success SHA1 'git ls-tree output for a known tree' '\n-\tcat >expected <<-\\EOF &&\n-\t100644 blob f87290f8eb2cbbea7857214459a0739927eab154\tpath0\n-\t120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01\tpath0sym\n-\t040000 tree 58a09c23e2ca152193f2786e06986b7b6712bdbe\tpath2\n-\t040000 tree 21ae8269cacbe57ae09138dcc3a2887f904d02b3\tpath3\n+test_expect_success 'git ls-tree output for a known tree' '\n+\tcat >expected <<-EOF &&\n+\t100644 blob $(test_oid path0f)\tpath0\n+\t120000 blob $(test_oid path0s)\tpath0sym\n+\t040000 tree $(test_oid path2d)\tpath2\n+\t040000 tree $(test_oid path3d)\tpath3\n \tEOF\n \ttest_cmp expected current\n '\n@@ -953,16 +957,16 @@ test_expect_success 'showing tree with git ls-tree -r' '\n \tgit ls-tree -r $tree >current\n '\n \n-test_expect_success SHA1 'git ls-tree -r output for a known tree' '\n-\tcat >expected <<-\\EOF &&\n-\t100644 blob f87290f8eb2cbbea7857214459a0739927eab154\tpath0\n-\t120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01\tpath0sym\n-\t100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7\tpath2/file2\n-\t120000 blob d8ce161addc5173867a3c3c730924388daedbc38\tpath2/file2sym\n-\t100644 blob 0aa34cae68d0878578ad119c86ca2b5ed5b28376\tpath3/file3\n-\t120000 blob 8599103969b43aff7e430efea79ca4636466794f\tpath3/file3sym\n-\t100644 blob 00fb5908cb97c2564a9783c0c64087333b3b464f\tpath3/subp3/file3\n-\t120000 blob 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c\tpath3/subp3/file3sym\n+test_expect_success 'git ls-tree -r output for a known tree' '\n+\tcat >expected <<-EOF &&\n+\t100644 blob $(test_oid path0f)\tpath0\n+\t120000 blob $(test_oid path0s)\tpath0sym\n+\t100644 blob $(test_oid path2f)\tpath2/file2\n+\t120000 blob $(test_oid path2s)\tpath2/file2sym\n+\t100644 blob $(test_oid path3f)\tpath3/file3\n+\t120000 blob $(test_oid path3s)\tpath3/file3sym\n+\t100644 blob $(test_oid subp3f)\tpath3/subp3/file3\n+\t120000 blob $(test_oid subp3s)\tpath3/subp3/file3sym\n \tEOF\n \ttest_cmp expected current\n '\n@@ -972,19 +976,19 @@ test_expect_success 'showing tree with git ls-tree -r -t' '\n \tgit ls-tree -r -t $tree >current\n '\n \n-test_expect_success SHA1 'git ls-tree -r output for a known tree' '\n-\tcat >expected <<-\\EOF &&\n-\t100644 blob f87290f8eb2cbbea7857214459a0739927eab154\tpath0\n-\t120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01\tpath0sym\n-\t040000 tree 58a09c23e2ca152193f2786e06986b7b6712bdbe\tpath2\n-\t100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7\tpath2/file2\n-\t120000 blob d8ce161addc5173867a3c3c730924388daedbc38\tpath2/file2sym\n-\t040000 tree 21ae8269cacbe57ae09138dcc3a2887f904d02b3\tpath3\n-\t100644 blob 0aa34cae68d0878578ad119c86ca2b5ed5b28376\tpath3/file3\n-\t120000 blob 8599103969b43aff7e430efea79ca4636466794f\tpath3/file3sym\n-\t040000 tree 3c5e5399f3a333eddecce7a9b9465b63f65f51e2\tpath3/subp3\n-\t100644 blob 00fb5908cb97c2564a9783c0c64087333b3b464f\tpath3/subp3/file3\n-\t120000 blob 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c\tpath3/subp3/file3sym\n+test_expect_success 'git ls-tree -r output for a known tree' '\n+\tcat >expected <<-EOF &&\n+\t100644 blob $(test_oid path0f)\tpath0\n+\t120000 blob $(test_oid path0s)\tpath0sym\n+\t040000 tree $(test_oid path2d)\tpath2\n+\t100644 blob $(test_oid path2f)\tpath2/file2\n+\t120000 blob $(test_oid path2s)\tpath2/file2sym\n+\t040000 tree $(test_oid path3d)\tpath3\n+\t100644 blob $(test_oid path3f)\tpath3/file3\n+\t120000 blob $(test_oid path3s)\tpath3/file3sym\n+\t040000 tree $(test_oid subp3d)\tpath3/subp3\n+\t100644 blob $(test_oid subp3f)\tpath3/subp3/file3\n+\t120000 blob $(test_oid subp3s)\tpath3/subp3/file3sym\n \tEOF\n \ttest_cmp expected current\n '\n@@ -993,16 +997,16 @@ test_expect_success 'writing partial tree out with git write-tree --prefix' '\n \tptree=$(git write-tree --prefix=path3)\n '\n \n-test_expect_success SHA1 'validate object ID for a known tree' '\n-\ttest \"$ptree\" = 21ae8269cacbe57ae09138dcc3a2887f904d02b3\n+test_expect_success 'validate object ID for a known tree' '\n+\ttest \"$ptree\" = $(test_oid path3d)\n '\n \n test_expect_success 'writing partial tree out with git write-tree --prefix' '\n \tptree=$(git write-tree --prefix=path3/subp3)\n '\n \n-test_expect_success SHA1 'validate object ID for a known tree' '\n-\ttest \"$ptree\" = 3c5e5399f3a333eddecce7a9b9465b63f65f51e2\n+test_expect_success 'validate object ID for a known tree' '\n+\ttest \"$ptree\" = $(test_oid subp3d)\n '\n \n test_expect_success 'put invalid objects into the index' '\n@@ -1036,16 +1040,16 @@ test_expect_success 'git read-tree followed by write-tree should be idempotent'\n \ttest \"$newtree\" = \"$tree\"\n '\n \n-test_expect_success SHA1 'validate git diff-files output for a know cache/work tree state' '\n-\tcat >expected <<\\EOF &&\n-:100644 100644 f87290f8eb2cbbea7857214459a0739927eab154 0000000000000000000000000000000000000000 M\tpath0\n-:120000 120000 15a98433ae33114b085f3eb3bb03b832b3180a01 0000000000000000000000000000000000000000 M\tpath0sym\n-:100644 100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0000000000000000000000000000000000000000 M\tpath2/file2\n-:120000 120000 d8ce161addc5173867a3c3c730924388daedbc38 0000000000000000000000000000000000000000 M\tpath2/file2sym\n-:100644 100644 0aa34cae68d0878578ad119c86ca2b5ed5b28376 0000000000000000000000000000000000000000 M\tpath3/file3\n-:120000 120000 8599103969b43aff7e430efea79ca4636466794f 0000000000000000000000000000000000000000 M\tpath3/file3sym\n-:100644 100644 00fb5908cb97c2564a9783c0c64087333b3b464f 0000000000000000000000000000000000000000 M\tpath3/subp3/file3\n-:120000 120000 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c 0000000000000000000000000000000000000000 M\tpath3/subp3/file3sym\n+test_expect_success 'validate git diff-files output for a know cache/work tree state' '\n+\tcat >expected <<EOF &&\n+:100644 100644 $(test_oid path0f) $ZERO_OID M\tpath0\n+:120000 120000 $(test_oid path0s) $ZERO_OID M\tpath0sym\n+:100644 100644 $(test_oid path2f) $ZERO_OID M\tpath2/file2\n+:120000 120000 $(test_oid path2s) $ZERO_OID M\tpath2/file2sym\n+:100644 100644 $(test_oid path3f) $ZERO_OID M\tpath3/file3\n+:120000 120000 $(test_oid path3s) $ZERO_OID M\tpath3/file3sym\n+:100644 100644 $(test_oid subp3f) $ZERO_OID M\tpath3/subp3/file3\n+:120000 120000 $(test_oid subp3s) $ZERO_OID M\tpath3/subp3/file3sym\n EOF\n \tgit diff-files >current &&\n \ttest_cmp current expected\n@@ -1061,23 +1065,23 @@ test_expect_success 'no diff after checkout and git update-index --refresh' '\n '\n \n ################################################################\n-P=087704a96baf1c2d1c869a8b084481e121c88b5b\n+P=$(test_oid root)\n \n-test_expect_success SHA1 'git commit-tree records the correct tree in a commit' '\n+test_expect_success 'git commit-tree records the correct tree in a commit' '\n \tcommit0=$(echo NO | git commit-tree $P) &&\n \ttree=$(git show --pretty=raw $commit0 |\n \t\t sed -n -e \"s/^tree //p\" -e \"/^author /q\") &&\n \ttest \"z$tree\" = \"z$P\"\n '\n \n-test_expect_success SHA1 'git commit-tree records the correct parent in a commit' '\n+test_expect_success 'git commit-tree records the correct parent in a commit' '\n \tcommit1=$(echo NO | git commit-tree $P -p $commit0) &&\n \tparent=$(git show --pretty=raw $commit1 |\n \t\tsed -n -e \"s/^parent //p\" -e \"/^author /q\") &&\n \ttest \"z$commit0\" = \"z$parent\"\n '\n \n-test_expect_success SHA1 'git commit-tree omits duplicated parent in a commit' '\n+test_expect_success 'git commit-tree omits duplicated parent in a commit' '\n \tcommit2=$(echo NO | git commit-tree $P -p $commit0 -p $commit0) &&\n \t     parent=$(git show --pretty=raw $commit2 |\n \t\tsed -n -e \"s/^parent //p\" -e \"/^author /q\" |\n"},{"id":"356042","messageId":"20180819175351.449973-5-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 04/11] t0002: abstract away SHA-1 specific constants","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:44Z","receivedAt":"2018-08-19T17:54:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Adjust the test so that it computes variables for object IDs instead of\nusing hard-coded hashes.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t0002-gitfile.sh | 25 +++++++++++++------------\n 1 file changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh\nindex 3691023d51..aa17023774 100755\n--- a/t/t0002-gitfile.sh\n+++ b/t/t0002-gitfile.sh\n@@ -92,11 +92,12 @@ test_expect_success 'enter_repo non-strict mode' '\n \t\tmv .git .realgit &&\n \t\techo \"gitdir: .realgit\" >.git\n \t) &&\n+\thead=$(git -C enter_repo rev-parse HEAD) &&\n \tgit ls-remote enter_repo >actual &&\n-\tcat >expected <<-\\EOF &&\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\tHEAD\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\trefs/heads/master\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\trefs/tags/foo\n+\tcat >expected <<-EOF &&\n+\t$head\tHEAD\n+\t$head\trefs/heads/master\n+\t$head\trefs/tags/foo\n \tEOF\n \ttest_cmp expected actual\n '\n@@ -107,20 +108,20 @@ test_expect_success 'enter_repo linked checkout' '\n \t\tgit worktree add  ../foo refs/tags/foo\n \t) &&\n \tgit ls-remote foo >actual &&\n-\tcat >expected <<-\\EOF &&\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\tHEAD\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\trefs/heads/master\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\trefs/tags/foo\n+\tcat >expected <<-EOF &&\n+\t$head\tHEAD\n+\t$head\trefs/heads/master\n+\t$head\trefs/tags/foo\n \tEOF\n \ttest_cmp expected actual\n '\n \n test_expect_success 'enter_repo strict mode' '\n \tgit ls-remote --upload-pack=\"git upload-pack --strict\" foo/.git >actual &&\n-\tcat >expected <<-\\EOF &&\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\tHEAD\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\trefs/heads/master\n-\t946e985ab20de757ca5b872b16d64e92ff3803a9\trefs/tags/foo\n+\tcat >expected <<-EOF &&\n+\t$head\tHEAD\n+\t$head\trefs/heads/master\n+\t$head\trefs/tags/foo\n \tEOF\n \ttest_cmp expected actual\n '\n"},{"id":"356043","messageId":"20180819175351.449973-6-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 05/11] t0027: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:45Z","receivedAt":"2018-08-19T17:54:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We transform various object IDs into all-zero object IDs for comparison.\nAdjust the length as well so that this works for all hash algorithms.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t0027-auto-crlf.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\nindex beb5927f77..e748060489 100755\n--- a/t/t0027-auto-crlf.sh\n+++ b/t/t0027-auto-crlf.sh\n@@ -15,8 +15,9 @@ compare_ws_file () {\n \tpfx=$1\n \texp=$2.expect\n \tact=$pfx.actual.$3\n-\ttr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp\" &&\n+\ttr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp+\" &&\n \ttr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$3\" >\"$act\" &&\n+\tsed -e \"s/0000+/$ZERO_OID/\" \"$exp+\" >\"$exp\" &&\n \ttest_cmp \"$exp\" \"$act\" &&\n \trm \"$exp\" \"$act\"\n }\n"},{"id":"356044","messageId":"20180819175351.449973-7-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 06/11] t0064: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:46Z","receivedAt":"2018-08-19T17:54:35Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Compute test values of the appropriate size instead of hard-coding\n40-character values.  Rename the echo20 function to echoid, since the\nvalues may be of varying sizes.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t0064-sha1-array.sh | 49 ++++++++++++++++++++++++-------------------\n 1 file changed, 27 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t0064-sha1-array.sh b/t/t0064-sha1-array.sh\nindex 67484502a0..5dda570b9a 100755\n--- a/t/t0064-sha1-array.sh\n+++ b/t/t0064-sha1-array.sh\n@@ -3,30 +3,30 @@\n test_description='basic tests for the SHA1 array implementation'\n . ./test-lib.sh\n \n-echo20 () {\n+echoid () {\n \tprefix=\"${1:+$1 }\"\n \tshift\n \twhile test $# -gt 0\n \tdo\n-\t\techo \"$prefix$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1\"\n+\t\techo \"$prefix$ZERO_OID\" | sed -e \"s/00/$1/g\"\n \t\tshift\n \tdone\n }\n \n test_expect_success 'ordered enumeration' '\n-\techo20 \"\" 44 55 88 aa >expect &&\n+\techoid \"\" 44 55 88 aa >expect &&\n \t{\n-\t\techo20 append 88 44 aa 55 &&\n+\t\techoid append 88 44 aa 55 &&\n \t\techo for_each_unique\n \t} | test-tool sha1-array >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'ordered enumeration with duplicate suppression' '\n-\techo20 \"\" 44 55 88 aa >expect &&\n+\techoid \"\" 44 55 88 aa >expect &&\n \t{\n-\t\techo20 append 88 44 aa 55 &&\n-\t\techo20 append 88 44 aa 55 &&\n+\t\techoid append 88 44 aa 55 &&\n+\t\techoid append 88 44 aa 55 &&\n \t\techo for_each_unique\n \t} | test-tool sha1-array >actual &&\n \ttest_cmp expect actual\n@@ -34,8 +34,8 @@ test_expect_success 'ordered enumeration with duplicate suppression' '\n \n test_expect_success 'lookup' '\n \t{\n-\t\techo20 append 88 44 aa 55 &&\n-\t\techo20 lookup 55\n+\t\techoid append 88 44 aa 55 &&\n+\t\techoid lookup 55\n \t} | test-tool sha1-array >actual &&\n \tn=$(cat actual) &&\n \ttest \"$n\" -eq 1\n@@ -43,8 +43,8 @@ test_expect_success 'lookup' '\n \n test_expect_success 'lookup non-existing entry' '\n \t{\n-\t\techo20 append 88 44 aa 55 &&\n-\t\techo20 lookup 33\n+\t\techoid append 88 44 aa 55 &&\n+\t\techoid lookup 33\n \t} | test-tool sha1-array >actual &&\n \tn=$(cat actual) &&\n \ttest \"$n\" -lt 0\n@@ -52,9 +52,9 @@ test_expect_success 'lookup non-existing entry' '\n \n test_expect_success 'lookup with duplicates' '\n \t{\n-\t\techo20 append 88 44 aa 55 &&\n-\t\techo20 append 88 44 aa 55 &&\n-\t\techo20 lookup 55\n+\t\techoid append 88 44 aa 55 &&\n+\t\techoid append 88 44 aa 55 &&\n+\t\techoid lookup 55\n \t} | test-tool sha1-array >actual &&\n \tn=$(cat actual) &&\n \ttest \"$n\" -ge 2 &&\n@@ -63,19 +63,24 @@ test_expect_success 'lookup with duplicates' '\n \n test_expect_success 'lookup non-existing entry with duplicates' '\n \t{\n-\t\techo20 append 88 44 aa 55 &&\n-\t\techo20 append 88 44 aa 55 &&\n-\t\techo20 lookup 66\n+\t\techoid append 88 44 aa 55 &&\n+\t\techoid append 88 44 aa 55 &&\n+\t\techoid lookup 66\n \t} | test-tool sha1-array >actual &&\n \tn=$(cat actual) &&\n \ttest \"$n\" -lt 0\n '\n \n test_expect_success 'lookup with almost duplicate values' '\n+\t# n-1 5s\n+\troot=$(echoid \"\" 55) &&\n+\troot=${root%5} &&\n \t{\n-\t\techo \"append 5555555555555555555555555555555555555555\" &&\n-\t\techo \"append 555555555555555555555555555555555555555f\" &&\n-\t\techo20 lookup 55\n+\t\tid1=\"${root}5\" &&\n+\t\tid2=\"${root}f\" &&\n+\t\techo \"append $id1\" &&\n+\t\techo \"append $id2\" &&\n+\t\techoid lookup 55\n \t} | test-tool sha1-array >actual &&\n \tn=$(cat actual) &&\n \ttest \"$n\" -eq 0\n@@ -83,8 +88,8 @@ test_expect_success 'lookup with almost duplicate values' '\n \n test_expect_success 'lookup with single duplicate value' '\n \t{\n-\t\techo20 append 55 55 &&\n-\t\techo20 lookup 55\n+\t\techoid append 55 55 &&\n+\t\techoid lookup 55\n \t} | test-tool sha1-array >actual &&\n \tn=$(cat actual) &&\n \ttest \"$n\" -ge 0 &&\n"},{"id":"356045","messageId":"20180819175351.449973-8-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 07/11] t1006: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:47Z","receivedAt":"2018-08-19T17:54:36Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Compute the size of the tree and commit objects we're creating by\nchecking for the size of an object ID and computing the resulting sizes\naccordingly.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t1006-cat-file.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 13dd510b2e..345ed70e53 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -140,15 +140,17 @@ test_expect_success '--batch-check without %(rest) considers whole line' '\n \ttest_cmp expect actual\n '\n \n+test_oid_cache hash-info\n+\n tree_sha1=$(git write-tree)\n-tree_size=33\n+tree_size=$(($(test_oid rawsz) + 13))\n tree_pretty_content=\"100644 blob $hello_sha1\thello\"\n \n run_tests 'tree' $tree_sha1 $tree_size \"\" \"$tree_pretty_content\"\n \n commit_message=\"Initial commit\"\n commit_sha1=$(echo_without_newline \"$commit_message\" | git commit-tree $tree_sha1)\n-commit_size=177\n+commit_size=$(($(test_oid hexsz) + 137))\n commit_content=\"tree $tree_sha1\n author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 0000000000 +0000\n committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 0000000000 +0000\n"},{"id":"356047","messageId":"20180819175351.449973-9-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 08/11] t1400: switch hard-coded object ID to variable","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:48Z","receivedAt":"2018-08-19T17:54:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Switch a hard-coded all-zeros object ID to use a variable instead.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t1400-update-ref.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 7c8df20955..6072650686 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -346,7 +346,7 @@ test_expect_success \"verifying $m's log (logged by config)\" '\n \n git update-ref $m $D\n cat >.git/logs/$m <<EOF\n-0000000000000000000000000000000000000000 $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500\n+$Z $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500\n $C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500\n $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500\n $F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500\n"},{"id":"356046","messageId":"20180819175351.449973-10-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 09/11] t1405: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:49Z","receivedAt":"2018-08-19T17:54:38Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Instead of hard-coding a 40-based constant, split the output of\nfor-each-ref and for-each-reflog by field.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t1405-main-ref-store.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh\nindex a74c38b5fb..331899ddc4 100755\n--- a/t/t1405-main-ref-store.sh\n+++ b/t/t1405-main-ref-store.sh\n@@ -54,7 +54,7 @@ test_expect_success 'for_each_ref(refs/heads/)' '\n '\n \n test_expect_success 'for_each_ref() is sorted' '\n-\t$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&\n+\t$RUN for-each-ref refs/heads/ | cut -d\" \" -f 2- >actual &&\n \tsort actual > expected &&\n \ttest_cmp expected actual\n '\n@@ -71,7 +71,7 @@ test_expect_success 'verify_ref(new-master)' '\n '\n \n test_expect_success 'for_each_reflog()' '\n-\t$RUN for-each-reflog | sort -k2 | cut -c 42- >actual &&\n+\t$RUN for-each-reflog | sort -k2 | cut -d\" \" -f 2- >actual &&\n \tcat >expected <<-\\EOF &&\n \tHEAD 0x1\n \trefs/heads/master 0x0\n"},{"id":"356048","messageId":"20180819175351.449973-11-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 10/11] t1406: make hash-size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:50Z","receivedAt":"2018-08-19T17:54:39Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Instead of hard-coding a 40-based constant, split the output of\nfor-each-ref and for-each-reflog by field.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t1406-submodule-ref-store.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh\nindex e093782cc3..d199d872fb 100755\n--- a/t/t1406-submodule-ref-store.sh\n+++ b/t/t1406-submodule-ref-store.sh\n@@ -39,7 +39,7 @@ test_expect_success 'rename_refs() not allowed' '\n '\n \n test_expect_success 'for_each_ref(refs/heads/)' '\n-\t$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&\n+\t$RUN for-each-ref refs/heads/ | cut -d\" \" -f 2- >actual &&\n \tcat >expected <<-\\EOF &&\n \tmaster 0x0\n \tnew-master 0x0\n@@ -48,7 +48,7 @@ test_expect_success 'for_each_ref(refs/heads/)' '\n '\n \n test_expect_success 'for_each_ref() is sorted' '\n-\t$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&\n+\t$RUN for-each-ref refs/heads/ | cut -d\" \" -f 2- >actual &&\n \tsort actual > expected &&\n \ttest_cmp expected actual\n '\n@@ -65,7 +65,7 @@ test_expect_success 'verify_ref(new-master)' '\n '\n \n test_expect_success 'for_each_reflog()' '\n-\t$RUN for-each-reflog | sort | cut -c 42- >actual &&\n+\t$RUN for-each-reflog | sort | cut -d\" \" -f 2- >actual &&\n \tcat >expected <<-\\EOF &&\n \tHEAD 0x1\n \trefs/heads/master 0x0\n"},{"id":"356049","messageId":"20180819175351.449973-12-sandals@crustytoothpaste.net","threadId":"49164","inReplyTo":"20180819175351.449973-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 11/11] t1407: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T17:53:51Z","receivedAt":"2018-08-19T17:54:40Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Instead of hard-coding a 40-based constant, split the output of\nfor-each-ref and for-each-reflog by field.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t1407-worktree-ref-store.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nindex 4623ae15c4..9a84858118 100755\n--- a/t/t1407-worktree-ref-store.sh\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -58,7 +58,7 @@ test_expect_success 'for_each_reflog()' '\n \tmkdir -p     .git/worktrees/wt/logs/refs/bisect &&\n \techo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&\n \n-\t$RWT for-each-reflog | cut -c 42- | sort >actual &&\n+\t$RWT for-each-reflog | cut -d\" \" -f 2- | sort >actual &&\n \tcat >expected <<-\\EOF &&\n \tHEAD 0x1\n \tPSEUDO-WT 0x0\n@@ -68,7 +68,7 @@ test_expect_success 'for_each_reflog()' '\n \tEOF\n \ttest_cmp expected actual &&\n \n-\t$RMAIN for-each-reflog | cut -c 42- | sort >actual &&\n+\t$RMAIN for-each-reflog | cut -d\" \" -f 2- | sort >actual &&\n \tcat >expected <<-\\EOF &&\n \tHEAD 0x1\n \tPSEUDO-MAIN 0x0\n"},{"id":"356050","messageId":"CAPig+cRZsJ00wNW08-jxmD=aW0V1hJJYEa9EVTMQT4=r0B+jeQ@mail.gmail.com","threadId":"49164","inReplyTo":"20180819175351.449973-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 01/11] t: add tool to translate hash-related values","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-19T19:40:22Z","receivedAt":"2018-08-19T19:40:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> diff --git a/t/oid-info/oid b/t/oid-info/oid\n> @@ -0,0 +1,29 @@\n> +# All zeros or Fs missing one or two hex segments.\n> +zero_1         sha1:000000000000000000000000000000000000000\n> +zero_2         sha256:000000000000000000000000000000000000000000000000000000000000000\n> +zero_2         sha1:00000000000000000000000000000000000000\n> +zero_2         sha256:00000000000000000000000000000000000000000000000000000000000000\n\nToo many zero_2's. I guess the first one should be zero_1.\n\n> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> @@ -821,6 +821,35 @@ test_expect_success 'tests clean up even on failures' \"\n> +test_oid_cache hash-info oid\n> +\n> +test_expect_success 'test_oid_info provides sane info by default' '\n\nIs \"test_oid_info\" in the test title outdated? I don't see anything by\nthat name. Same question regarding the other new tests.\n\n> +       test_oid zero &&\n> +       test_oid zero >actual &&\n\nIf the lookup \"test_oid zero\" fails, it's going to fail whether\nredirected or not, so the first invocation can be dropped.\n\n> +       grep \"00*\" actual &&\n\nDo you want to anchor this regex? ^00*$\n\nSame comment regarding the other new tests.\n\n> +       test \"$(test_oid hexsz)\" -eq $(wc -c <actual) &&\n> +       test $(( $(test_oid rawsz) * 2)) -eq \"$(test_oid hexsz)\"\n> +'\n\nIf $(test_oid rawsz) fails to find \"rawsz\" and returns nothing, this\nexpression will be \"*2\", which will error out in a\nless-than-meaningful way. Perhaps it would make more sense to dump the\nresults of the two test_oid() to file and use test_cmp()?\n\nSimilar comment regarding all the other \"test ... -eq ...\" expressions\nhere and below: I wonder if it would be better to dump them to files\nand compare the files.\n\n> +test_expect_success 'test_oid_info can look up data for SHA-1' '\n> +       test_detect_hash sha1 &&\n> +       test_oid zero >actual &&\n> +       grep \"00*\" actual &&\n> +       test $(wc -c <actual) -eq 40 &&\n> +       test \"$(test_oid rawsz)\" -eq 20 &&\n> +       test \"$(test_oid hexsz)\" -eq 40\n> +'\n> +\n> +test_expect_success 'test_oid_info can look up data for SHA-256' '\n> +       test_when_finished \"test_detect_hash\" &&\n\nShould the previous test also do this test_when_finished() to protect\nagainst someone coming along and re-ordering them some day? Or someone\ninserting a test between the two?\n\n> +       test_detect_hash sha256 &&\n> +       test_oid zero >actual &&\n> +       grep \"00*\" actual &&\n> +       test $(wc -c <actual) -eq 64 &&\n> +       test \"$(test_oid rawsz)\" -eq 32 &&\n> +       test \"$(test_oid hexsz)\" -eq 64\n> +'\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> @@ -1147,3 +1147,39 @@ depacketize () {\n> +test_detect_hash () {\n> +       if test -n \"$1\"\n> +       then\n> +               test_hash_algo=\"$1\"\n> +       else\n> +               test_hash_algo='sha1'\n> +       fi\n> +}\n\nMmph. Outside of t0000, do you expect other callers which will need to\nset the hash algorithm to an explicit value? If not, this sort of\npolymorphic behavior adds extra, perhaps unnecessary, complexity. Even\nif you do expect a few other callers, a dedicated test_set_hash()\nfunction might be clearer since test_detect_hash() has a very specific\nmeaning.\n\n> +test_oid_cache () {\n> +       test -n \"$test_hash_algo\" || test_detect_hash\n> +       if test -n \"$1\"\n> +       then\n> +               while test -n \"$1\"\n> +               do\n> +                       test_oid_cache <\"$TEST_DIRECTORY/oid-info/$1\"\n\nWhat is the benefit of placing test-specific OID info in some\ndedicated directory? I suppose the idea of lumping all these OID files\ntogether in a single oid-info/ directory is that it makes it easier to\nsee at a glance what needs to be changed next time a new hash\nalgorithm is selected. However, that potential long-term benefit comes\nat the cost of divorcing test-specific information from the tests\nthemselves. I imagine (for myself, at least) that it would be easier\nto grok a test if its OID's were specified and cached directly in the\ntest script itself (via test_oid_cache <<here-doc). I could even\nimagine a test script invoking test_oid_cache<<here-doc before each\ntest which has unique OID's in order to localize OID information to\nthe test which needs it, or perhaps even within a test.\n\nSo, I'm having trouble understanding the benefit of the current\napproach over merely loading OID's via here-docs in the test scripts\nthemselves. Perhaps the commit message could say something about why\nthe current approach was taken.\n\n> +                       shift\n> +               done\n> +               return $?\n\nWhy, specifically, return $? here, as opposed to simply returning?\n\n> +       fi\n\nMmph. This polymorphic, recursive behavior in which test_oid_cache()\ncan load data from a list of files or from its own standard input adds\ncomplexity. One alternative would be to have a separate\ntest_oid_cache_file() function. However, I'm wondering why such\nfunctionality needs to be built in, in the first place, as opposed to\nclients merely doing the redirect themselves (test_oid_cache\n<whatever). I see that you want to support specifying multiple files\nfor a single invocation, but is that likely to come up (aside from\nt0000)?\n\n> +       while read _tag _rest\n> +       do\n> +               case $_tag in \\#*)\n> +                       continue;;\n> +               esac\n\nThis handles \"# comment\" lines, but what about blank lines?\n\n> +               _k=\"${_rest%:*}\"\n> +               _v=\"${_rest#*:}\"\n\nShould this be doing any sort of validation, such as complaining if _k\nor _v is empty? Or if _k contains weird characters?\n\n> +               eval \"test_oid_${_k}_$_tag=\\\"$_v\\\"\"\n> +       done\n> +}\n> +\n> +test_oid () {\n> +       eval \"printf '%s' \\\"\\${test_oid_${test_hash_algo}_$1}\\\"\"\n> +}\n"},{"id":"356051","messageId":"CAPig+cTmSN3CGGW0ws2K-CHrvVon4W+iFS_MdTxSY-TP6OUh1Q@mail.gmail.com","threadId":"49164","inReplyTo":"20180819175351.449973-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 02/11] t0000: use hash translation table","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-19T19:54:09Z","receivedAt":"2018-08-19T19:54:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> If the hash we're using is 32 bytes in size, attempting to insert a\n> 20-byte object name won't work.  Since these are synthesized objects\n> that are almost all zeros, look them up in a translation table.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> @@ -1007,12 +1007,13 @@ test_expect_success SHA1 'validate object ID for a known tree' '\n>  test_expect_success 'put invalid objects into the index' '\n>         rm -f .git/index &&\n> -       [...]\n> +       suffix=$(echo $ZERO_OID | sed -e \"s/^.//\") &&\n\nWhat's this \"suffix\" thing for? I don't see it used anywhere.\n\n> +       cat >badobjects <<-EOF &&\n> +       100644 blob $(test_oid 001)     dir/file1\n> +       100644 blob $(test_oid 002)     dir/file2\n> +       100644 blob $(test_oid 003)     dir/file3\n> +       100644 blob $(test_oid 004)     dir/file4\n> +       100644 blob $(test_oid 005)     dir/file5\n\nSo, test_oid() knows about these keys because the patch 1/11 loaded\nthem via test_oid_cache(). Are the keys in oid-info/hash-info and\noid-info/oid going to be needed by multiple scripts? If so, I'm\nwondering if it would make more sense to have test-lib-functions.sh\nload them instead of expecting each script to do so. Another\npossibility is to have a test_oid_init() function in\ntest-lib-functions.sh which both loads that low-level information and\ninitializes the hash algorithm.\n\n>         EOF\n>         git update-index --index-info <badobjects\n>  '\n"},{"id":"356052","messageId":"CAPig+cSRYZ6X-71oZ473g_pChcfNcFyFPO3e8YoJmYyg+KwoyQ@mail.gmail.com","threadId":"49164","inReplyTo":"20180819175351.449973-4-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 03/11] t0000: update tests for SHA-256","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-19T20:01:05Z","receivedAt":"2018-08-19T20:01:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> Test t0000 tests the \"basics of the basics\" and as such, checks that we\n> have various fixed hard-coded object IDs.  The tests relying on these\n> assertions have been marked with the SHA1 prerequisite, as they will\n> obviously not function in their current form with SHA-256.\n>\n> Use the test_oid helper to update these assertions and provide values\n> for both SHA-1 and SHA-256.\n> [...]\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> @@ -868,8 +870,9 @@ test_expect_success 'writing tree out with git write-tree' '\n> +test_expect_success 'validate object ID of a known tree' '\n> +echo $tree &&\n\nDebugging gunk?\n\n> +       test \"$tree\" = $(test_oid simpletree)\n\nIf test_oid() fails to find key \"simpletree\", this expression will be\ninvalid. Therefore, it probably would be a good idea to quote the\n$(test_oid ...) invocation.\n\n> @@ -911,17 +914,18 @@ test_expect_success 'showing stage with git ls-files --stage' '\n> +test_expect_success 'validate git ls-files output for a known tree' '\n> +       cat >expected <<-EOF &&\n> +       100644 $(test_oid path0f) 0     path0\n> +       120000 $(test_oid path0s) 0     path0sym\n> +       100644 $(test_oid path2f) 0     path2/file2\n> +       120000 $(test_oid path2s) 0     path2/file2sym\n> +       100644 $(test_oid path3f) 0     path3/file3\n> +       120000 $(test_oid path3s) 0     path3/file3sym\n> +       100644 $(test_oid subp3f) 0     path3/subp3/file3\n> +       120000 $(test_oid subp3s) 0     path3/subp3/file3sym\n>         EOF\n> +       cat expected &&\n\nDebugging gunk?\n\n>         test_cmp expected current\n>  '\n"},{"id":"356053","messageId":"CAPig+cQz4oM-csmniSe5LPkHixUOh04i+CLWbJB4dn-NeYS3jg@mail.gmail.com","threadId":"49164","inReplyTo":"20180819175351.449973-5-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 04/11] t0002: abstract away SHA-1 specific constants","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-19T20:05:25Z","receivedAt":"2018-08-19T20:05:39Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> Adjust the test so that it computes variables for object IDs instead of\n> using hard-coded hashes.\n\nNit: s/hashes/hash values/\n\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh\n> @@ -92,11 +92,12 @@ test_expect_success 'enter_repo non-strict mode' '\n>         ) &&\n> +       head=$(git -C enter_repo rev-parse HEAD) &&\n>         [...]\n> +       cat >expected <<-EOF &&\n> +       $head   HEAD\n> +       $head   refs/heads/master\n> +       $head   refs/tags/foo\n>         EOF\n>         test_cmp expected actual\n>  '\n\nOkay, but...\n\n> @@ -107,20 +108,20 @@ test_expect_success 'enter_repo linked checkout' '\n> -       [...]\n> +       cat >expected <<-EOF &&\n> +       $head   HEAD\n> +       $head   refs/heads/master\n> +       $head   refs/tags/foo\n>         EOF\n>         test_cmp expected actual\n>  '\n\nThis is relying upon 'head' set inside an earlier test, which seems\nfragile. More robust would be to compute 'head' anew within each test\nwhich needs it (including the other new test added by this patch).\n"},{"id":"356054","messageId":"CAPig+cS6Z+U-Z26m-eW-JX5D29ONSb_Zzur2fvWGb_ZMbmF4UQ@mail.gmail.com","threadId":"49164","inReplyTo":"20180819175351.449973-6-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 05/11] t0027: make hash size independent","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-19T20:10:21Z","receivedAt":"2018-08-19T20:10:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> We transform various object IDs into all-zero object IDs for comparison.\n> Adjust the length as well so that this works for all hash algorithms.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\n> @@ -15,8 +15,9 @@ compare_ws_file () {\n> -       tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp\" &&\n> +       tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp+\" &&\n\nMy immediate thought upon reading this was whether \"+\" is valid in\nWindows filenames. Apparently, it is, but perhaps (if you re-roll) it\nwould make sense to use a character less likely to cause brain\nhiccups; for instance, \"exp0'.\n\n>         tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$3\" >\"$act\" &&\n> +       sed -e \"s/0000+/$ZERO_OID/\" \"$exp+\" >\"$exp\" &&\n"},{"id":"356059","messageId":"20180819215022.GH70480@genre.crustytoothpaste.net","threadId":"49164","inReplyTo":"CAPig+cRZsJ00wNW08-jxmD=aW0V1hJJYEa9EVTMQT4=r0B+jeQ@mail.gmail.com","subject":"Re: [PATCH v2 01/11] t: add tool to translate hash-related values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T21:50:22Z","receivedAt":"2018-08-19T21:50:32Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Aug 19, 2018 at 03:40:22PM -0400, Eric Sunshine wrote:\n> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > diff --git a/t/oid-info/oid b/t/oid-info/oid\n> > @@ -0,0 +1,29 @@\n> > +# All zeros or Fs missing one or two hex segments.\n> > +zero_1         sha1:000000000000000000000000000000000000000\n> > +zero_2         sha256:000000000000000000000000000000000000000000000000000000000000000\n> > +zero_2         sha1:00000000000000000000000000000000000000\n> > +zero_2         sha256:00000000000000000000000000000000000000000000000000000000000000\n> \n> Too many zero_2's. I guess the first one should be zero_1.\n\nAh, yes, you're right.\n\n> > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> > @@ -821,6 +821,35 @@ test_expect_success 'tests clean up even on failures' \"\n> > +test_oid_cache hash-info oid\n> > +\n> > +test_expect_success 'test_oid_info provides sane info by default' '\n> \n> Is \"test_oid_info\" in the test title outdated? I don't see anything by\n> that name. Same question regarding the other new tests.\n\nProbably.\n\n> > +       test_oid zero &&\n> > +       test_oid zero >actual &&\n> \n> If the lookup \"test_oid zero\" fails, it's going to fail whether\n> redirected or not, so the first invocation can be dropped.\n\nRight.  I think that might have been a debugging statement.  Will fix.\n\n> > +       grep \"00*\" actual &&\n> \n> Do you want to anchor this regex? ^00*$\n\nI could.  That would probably be a bit more robust.\n\n> > +       test \"$(test_oid hexsz)\" -eq $(wc -c <actual) &&\n> > +       test $(( $(test_oid rawsz) * 2)) -eq \"$(test_oid hexsz)\"\n> > +'\n> \n> If $(test_oid rawsz) fails to find \"rawsz\" and returns nothing, this\n> expression will be \"*2\", which will error out in a\n> less-than-meaningful way. Perhaps it would make more sense to dump the\n> results of the two test_oid() to file and use test_cmp()?\n> \n> Similar comment regarding all the other \"test ... -eq ...\" expressions\n> here and below: I wonder if it would be better to dump them to files\n> and compare the files.\n\nI think what I'd like to do instead is store in a variable and make\ntest_oid return unsuccessfully if the value doesn't exist.  I think\nthat's better for writing tests overall and will accomplish the same\ngoal.\n\n> > +test_expect_success 'test_oid_info can look up data for SHA-1' '\n> > +       test_detect_hash sha1 &&\n> > +       test_oid zero >actual &&\n> > +       grep \"00*\" actual &&\n> > +       test $(wc -c <actual) -eq 40 &&\n> > +       test \"$(test_oid rawsz)\" -eq 20 &&\n> > +       test \"$(test_oid hexsz)\" -eq 40\n> > +'\n> > +\n> > +test_expect_success 'test_oid_info can look up data for SHA-256' '\n> > +       test_when_finished \"test_detect_hash\" &&\n> \n> Should the previous test also do this test_when_finished() to protect\n> against someone coming along and re-ordering them some day? Or someone\n> inserting a test between the two?\n\nYeah, that sounds like a more robust solution.\n\n> > +       test_detect_hash sha256 &&\n> > +       test_oid zero >actual &&\n> > +       grep \"00*\" actual &&\n> > +       test $(wc -c <actual) -eq 64 &&\n> > +       test \"$(test_oid rawsz)\" -eq 32 &&\n> > +       test \"$(test_oid hexsz)\" -eq 64\n> > +'\n> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> > @@ -1147,3 +1147,39 @@ depacketize () {\n> > +test_detect_hash () {\n> > +       if test -n \"$1\"\n> > +       then\n> > +               test_hash_algo=\"$1\"\n> > +       else\n> > +               test_hash_algo='sha1'\n> > +       fi\n> > +}\n> \n> Mmph. Outside of t0000, do you expect other callers which will need to\n> set the hash algorithm to an explicit value? If not, this sort of\n> polymorphic behavior adds extra, perhaps unnecessary, complexity. Even\n> if you do expect a few other callers, a dedicated test_set_hash()\n> function might be clearer since test_detect_hash() has a very specific\n> meaning.\n\nI'm not anticipating one.  I can split it out into a separate function.\n\n> > +test_oid_cache () {\n> > +       test -n \"$test_hash_algo\" || test_detect_hash\n> > +       if test -n \"$1\"\n> > +       then\n> > +               while test -n \"$1\"\n> > +               do\n> > +                       test_oid_cache <\"$TEST_DIRECTORY/oid-info/$1\"\n> \n> What is the benefit of placing test-specific OID info in some\n> dedicated directory? I suppose the idea of lumping all these OID files\n> together in a single oid-info/ directory is that it makes it easier to\n> see at a glance what needs to be changed next time a new hash\n> algorithm is selected. However, that potential long-term benefit comes\n> at the cost of divorcing test-specific information from the tests\n> themselves. I imagine (for myself, at least) that it would be easier\n> to grok a test if its OID's were specified and cached directly in the\n> test script itself (via test_oid_cache <<here-doc). I could even\n> imagine a test script invoking test_oid_cache<<here-doc before each\n> test which has unique OID's in order to localize OID information to\n> the test which needs it, or perhaps even within a test.\n\nPutting them in a separate directory makes it possible to do something\nlike this:\n\n   (cd t && ./t0002-* --verbose)\n\nand then use shell editing to change the command line.  If we put them\nin the same directory as the tests, we make developers' lives a bit\nharder.\n\nYou mentioned the desire to experiment with additional hash algorithms\nas a hope for this series.  I don't know if that's still something\ndesirable to have, now that we've decided on SHA-256, but I felt it\nwould make it easier to find all the places that need updating.\n\nSince you seem to have a strong preference about keeping them in the\ntest script, I'm happy to make that change unless other people have\nstrong feelings one way or the other.\n\n> So, I'm having trouble understanding the benefit of the current\n> approach over merely loading OID's via here-docs in the test scripts\n> themselves. Perhaps the commit message could say something about why\n> the current approach was taken.\n\nI can do that.  The idea is that we have lots of common uses of certain\nvalues that will need to be loaded, and it's better to load some of\nthose values from a file.  I felt it would be ugly to have to write out\nthe full \"$TEST_DIRECTORY...\" piece every time.\n\n> > +                       shift\n> > +               done\n> > +               return $?\n> \n> Why, specifically, return $? here, as opposed to simply returning?\n\nA simple return is probably better.  I think I wasn't clear on whether\nPOSIX required a bare \"return\" to return $? and may not have been online\nat that time to look.  I remember being very clear that I didn't want to\nreturn 0 unconditionally, though.\n\n> Mmph. This polymorphic, recursive behavior in which test_oid_cache()\n> can load data from a list of files or from its own standard input adds\n> complexity. One alternative would be to have a separate\n> test_oid_cache_file() function. However, I'm wondering why such\n> functionality needs to be built in, in the first place, as opposed to\n> clients merely doing the redirect themselves (test_oid_cache\n> <whatever). I see that you want to support specifying multiple files\n> for a single invocation, but is that likely to come up (aside from\n> t0000)?\n\nI expect that we are going to have a lot of uses of the hash-info and\noid files.  A separate command should be fine.\n\n> > +       while read _tag _rest\n> > +       do\n> > +               case $_tag in \\#*)\n> > +                       continue;;\n> > +               esac\n> \n> This handles \"# comment\" lines, but what about blank lines?\n\nGood catch.\n\n> > +               _k=\"${_rest%:*}\"\n> > +               _v=\"${_rest#*:}\"\n> \n> Should this be doing any sort of validation, such as complaining if _k\n> or _v is empty? Or if _k contains weird characters?\n\n_v could be validly empty in some cases.  _k is probably worth checking.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"356060","messageId":"20180819215318.GI70480@genre.crustytoothpaste.net","threadId":"49164","inReplyTo":"CAPig+cSRYZ6X-71oZ473g_pChcfNcFyFPO3e8YoJmYyg+KwoyQ@mail.gmail.com","subject":"Re: [PATCH v2 03/11] t0000: update tests for SHA-256","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T21:53:18Z","receivedAt":"2018-08-19T21:53:25Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Aug 19, 2018 at 04:01:05PM -0400, Eric Sunshine wrote:\n> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n> > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> > @@ -868,8 +870,9 @@ test_expect_success 'writing tree out with git write-tree' '\n> > +test_expect_success 'validate object ID of a known tree' '\n> > +echo $tree &&\n> \n> Debugging gunk?\n\nYup.  Will fix both.\n\n> > +       test \"$tree\" = $(test_oid simpletree)\n> \n> If test_oid() fails to find key \"simpletree\", this expression will be\n> invalid. Therefore, it probably would be a good idea to quote the\n> $(test_oid ...) invocation.\n\nGood idea.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"356061","messageId":"20180819215732.GJ70480@genre.crustytoothpaste.net","threadId":"49164","inReplyTo":"CAPig+cS6Z+U-Z26m-eW-JX5D29ONSb_Zzur2fvWGb_ZMbmF4UQ@mail.gmail.com","subject":"Re: [PATCH v2 05/11] t0027: make hash size independent'","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T21:57:32Z","receivedAt":"2018-08-19T21:57:40Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:\n> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > We transform various object IDs into all-zero object IDs for comparison.\n> > Adjust the length as well so that this works for all hash algorithms.\n> >\n> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> > ---\n> > diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\n> > @@ -15,8 +15,9 @@ compare_ws_file () {\n> > -       tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp\" &&\n> > +       tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp+\" &&\n> \n> My immediate thought upon reading this was whether \"+\" is valid in\n> Windows filenames. Apparently, it is, but perhaps (if you re-roll) it\n> would make sense to use a character less likely to cause brain\n> hiccups; for instance, \"exp0'.\n\nThe reason I picked that is that we use it quite a bit in the Makefile,\nso it seemed like an obvious choice for a temporary file name.  If you\nfeel strongly I can pick something else, but I thought it would be\nreasonably intuitive for other developers.  Maybe I was wrong, though.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"356067","messageId":"CAPig+cTTmOw-d7umES9p2xv4uCkjTnNtjTni_DaVFKbhc8WTgA@mail.gmail.com","threadId":"49164","inReplyTo":"20180819215732.GJ70480@genre.crustytoothpaste.net","subject":"Re: [PATCH v2 05/11] t0027: make hash size independent'","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-19T22:10:38Z","receivedAt":"2018-08-19T22:13:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 19, 2018 at 5:57 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:\n> > On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n> > > -       tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp\" &&\n> > > +       tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp+\" &&\n> >\n> > My immediate thought upon reading this was whether \"+\" is valid in\n> > Windows filenames. Apparently, it is, but perhaps (if you re-roll) it\n> > would make sense to use a character less likely to cause brain\n> > hiccups; for instance, \"exp0'.\n>\n> The reason I picked that is that we use it quite a bit in the Makefile,\n> so it seemed like an obvious choice for a temporary file name.  If you\n> feel strongly I can pick something else, but I thought it would be\n> reasonably intuitive for other developers.  Maybe I was wrong, though.\n\nI don't feel strongly about it. My brain tripped over it probably\nbecause it's not an idiom in Git tests. In fact, I see just one place\nwhere \"+\" has been used like this, in t6026.\n"},{"id":"356068","messageId":"CAPig+cRSUDAdE2ECcH+b8h4urJySaS_VybQdHNv24rad_kFEag@mail.gmail.com","threadId":"49164","inReplyTo":"20180819215022.GH70480@genre.crustytoothpaste.net","subject":"Re: [PATCH v2 01/11] t: add tool to translate hash-related values","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-19T23:06:14Z","receivedAt":"2018-08-19T23:06:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 19, 2018 at 5:50 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> On Sun, Aug 19, 2018 at 03:40:22PM -0400, Eric Sunshine wrote:\n> > On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n> > <sandals@crustytoothpaste.net> wrote:\n> > > +       test \"$(test_oid hexsz)\" -eq $(wc -c <actual) &&\n> > > +       test $(( $(test_oid rawsz) * 2)) -eq \"$(test_oid hexsz)\"\n> > > +'\n> >\n> > If $(test_oid rawsz) fails to find \"rawsz\" and returns nothing, this\n> > expression will be \"*2\", which will error out in a\n> > less-than-meaningful way. Perhaps it would make more sense to dump the\n> > results of the two test_oid() to file and use test_cmp()?\n>\n> I think what I'd like to do instead is store in a variable and make\n> test_oid return unsuccessfully if the value doesn't exist.  I think\n> that's better for writing tests overall and will accomplish the same\n> goal.\n\nMy knee-jerk reaction is that could get clunky in practice, but\nperhaps I'm not seeing the full picture.\n\nAn alternative would be to return a distinguished error value.\n\n> > > + test_oid_cache <\"$TEST_DIRECTORY/oid-info/$1\"\n> >\n> > What is the benefit of placing test-specific OID info in some\n> > dedicated directory? I suppose the idea of lumping all these OID files\n> > together in a single oid-info/ directory is that it makes it easier to\n> > see at a glance what needs to be changed next time a new hash\n> > algorithm is selected. However, that potential long-term benefit comes\n> > at the cost of divorcing test-specific information from the tests\n> > themselves. I imagine (for myself, at least) that it would be easier\n> > to grok a test if its OID's were specified and cached directly in the\n> > test script itself (via test_oid_cache <<here-doc). I could even\n> > imagine a test script invoking test_oid_cache<<here-doc before each\n> > test which has unique OID's in order to localize OID information to\n> > the test which needs it, or perhaps even within a test.\n>\n> Putting them in a separate directory makes it possible to do something\n> like this:\n>\n>    (cd t && ./t0002-* --verbose)\n>\n> and then use shell editing to change the command line.  If we put them\n> in the same directory as the tests, we make developers' lives a bit\n> harder.\n\nPerhaps I'm missing something obvious, but I'm not following what\nyou're trying to convey.\n\nOkay, thinking upon this further, I guess you mean people who type\nyour example directly rather than using \"./t0002-*.sh\" or something.\n\n> You mentioned the desire to experiment with additional hash algorithms\n> as a hope for this series.  I don't know if that's still something\n> desirable to have, now that we've decided on SHA-256, but I felt it\n> would make it easier to find all the places that need updating.\n\nI'm still open to the idea of supporting experimentation with other\nhash algorithms and I see how lumping all the OID tables into a single\ndirectory can make it easy to locate everything that will require\nadjustment for a new algorithm. But, this information can also be\ngleaned via a simple grep for \"test_oid_cache\", so I'm not sure the\nlumped-directory approach buys much.\n\nAlso, my gut feeling is that such experimentation will be very rare,\nwhereas the addition of new tests which have some sort of\nOID-dependency will likely be a more frequent occurrence. And, the\nlocality of an OID-table plopped down right next to the test which\nrequires it seems a win (in my mind).\n\n> > So, I'm having trouble understanding the benefit of the current\n> > approach over merely loading OID's via here-docs in the test scripts\n> > themselves. Perhaps the commit message could say something about why\n> > the current approach was taken.\n>\n> I can do that.  The idea is that we have lots of common uses of certain\n> values that will need to be loaded, and it's better to load some of\n> those values from a file.  I felt it would be ugly to have to write out\n> the full \"$TEST_DIRECTORY...\" piece every time.\n\nI do favor the simplicity of the caller deciding whether to use\n\"test_oid_cache <file\" or \"test_oid_cache <<here-doc\"; it provides\nextra flexibility over a function which loads the OID tables from a\nfixed path.\n\nHowever, I'm sympathetic to the ugliness each caller having to specify\n\"$TEST_DIRECTORY/...\". In my review of 2/11, I suggested the idea of a\ntest_oid_init() function which could load those common OID tables for\nscripts which need them, thus hiding that ugliness. Another idea which\nhas some appeal is to define an $OIDDB variable (or some such name)\nwith value \"$TEST_DIRECTORY/oid-info\", which lets callers use:\n\n    test_oid_cache <\"$OIDDB/bloop\"\n\nwhich isn't so bad.\n\n> > Why, specifically, return $? here, as opposed to simply returning?\n>\n> A simple return is probably better.  I think I wasn't clear on whether\n> POSIX required a bare \"return\" to return $? and may not have been online\n> at that time to look.  I remember being very clear that I didn't want to\n> return 0 unconditionally, though.\n\nI asked because, as far as I can see, this _is_ returning 0\nunconditionally since $? will always be 0 by the time it gets to the\n'return' (since it's not making any effort to break from the 'while'\nloop with a meaningful value upon failure.\n\nFixing this function to return a meaningful value (success or failure)\nmight be sensible since that would allow it to be used directly in a\ntest rather than only outside.\n"},{"id":"356069","messageId":"20180819235603.GK70480@genre.crustytoothpaste.net","threadId":"49164","inReplyTo":"CAPig+cRSUDAdE2ECcH+b8h4urJySaS_VybQdHNv24rad_kFEag@mail.gmail.com","subject":"Re: [PATCH v2 01/11] t: add tool to translate hash-related values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-08-19T23:56:03Z","receivedAt":"2018-08-19T23:56:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Aug 19, 2018 at 07:06:14PM -0400, Eric Sunshine wrote:\n> On Sun, Aug 19, 2018 at 5:50 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > I think what I'd like to do instead is store in a variable and make\n> > test_oid return unsuccessfully if the value doesn't exist.  I think\n> > that's better for writing tests overall and will accomplish the same\n> > goal.\n> \n> My knee-jerk reaction is that could get clunky in practice, but\n> perhaps I'm not seeing the full picture.\n> \n> An alternative would be to return a distinguished error value.\n\nWe already do it in several other places in the testsuite when we want\nto check the exit status of a command substitution, so I think it will\nend up being fine.  I'd like to try it and see.\n\nI think that we need to fix the return code either way, though.\n\n> > Putting them in a separate directory makes it possible to do something\n> > like this:\n> >\n> >    (cd t && ./t0002-* --verbose)\n> >\n> > and then use shell editing to change the command line.  If we put them\n> > in the same directory as the tests, we make developers' lives a bit\n> > harder.\n> \n> Perhaps I'm missing something obvious, but I'm not following what\n> you're trying to convey.\n> \n> Okay, thinking upon this further, I guess you mean people who type\n> your example directly rather than using \"./t0002-*.sh\" or something.\n\nRight.  It also makes completion nicer (in Vim at least).\n\n> I'm still open to the idea of supporting experimentation with other\n> hash algorithms and I see how lumping all the OID tables into a single\n> directory can make it easy to locate everything that will require\n> adjustment for a new algorithm. But, this information can also be\n> gleaned via a simple grep for \"test_oid_cache\", so I'm not sure the\n> lumped-directory approach buys much.\n> \n> Also, my gut feeling is that such experimentation will be very rare,\n> whereas the addition of new tests which have some sort of\n> OID-dependency will likely be a more frequent occurrence. And, the\n> locality of an OID-table plopped down right next to the test which\n> requires it seems a win (in my mind).\n\nOkay.  I'll do that, and we'll see how it works.\n\n> However, I'm sympathetic to the ugliness each caller having to specify\n> \"$TEST_DIRECTORY/...\". In my review of 2/11, I suggested the idea of a\n> test_oid_init() function which could load those common OID tables for\n> scripts which need them, thus hiding that ugliness. Another idea which\n> has some appeal is to define an $OIDDB variable (or some such name)\n> with value \"$TEST_DIRECTORY/oid-info\", which lets callers use:\n> \n>     test_oid_cache <\"$OIDDB/bloop\"\n> \n> which isn't so bad.\n\nI'll stuff in a test_oid_init function.  I think I like that approach\nthe best.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"356088","messageId":"31634401-f889-c551-4232-4745b9ccce44@web.de","threadId":"49164","inReplyTo":"CAPig+cTTmOw-d7umES9p2xv4uCkjTnNtjTni_DaVFKbhc8WTgA@mail.gmail.com","subject":"Re: [PATCH v2 05/11] t0027: make hash size independent'","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-08-20T14:29:31Z","receivedAt":"2018-08-20T14:30:26Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 20.08.18 00:10, Eric Sunshine wrote:\n> On Sun, Aug 19, 2018 at 5:57 PM brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n>> On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:\n>>> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson\n>>>> -       tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp\" &&\n>>>> +       tr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" >\"$exp+\" &&\n>>>\n>>> My immediate thought upon reading this was whether \"+\" is valid in\n>>> Windows filenames. Apparently, it is, but perhaps (if you re-roll) it\n>>> would make sense to use a character less likely to cause brain\n>>> hiccups; for instance, \"exp0'.\n>>\n>> The reason I picked that is that we use it quite a bit in the Makefile,\n>> so it seemed like an obvious choice for a temporary file name.  If you\n>> feel strongly I can pick something else, but I thought it would be\n>> reasonably intuitive for other developers.  Maybe I was wrong, though.\n> \n> I don't feel strongly about it. My brain tripped over it probably\n> because it's not an idiom in Git tests. In fact, I see just one place\n> where \"+\" has been used like this, in t6026.\n> \n\nProbably \"tmp\" is a better name than \"exp+\" \n(Why the '+' ? Is it better that the non-'+' ?)\n\nAnyway,\nIf we re-order a little bit, can we use a simple '|' instead ?\n\ntr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$2\" |\n\tsed -e \"s/0000+/$ZERO_OID/\"               >\"$exp\" &&\ntr '\\015\\000abcdef0123456789' QN00000000000000000 <\"$3\" >\"$act\" &&\n"}]}