{"thread":{"id":"48651","subject":"[PATCH 00/10] Hash-independent tests (part 3)","startedAt":"2018-06-04T23:52:46Z","lastAt":"2018-07-25T08:34:00Z","messageCount":26,"participants":["brian m. carlson","Torsten Bögershausen","Jeff King","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"349303","messageId":"20180604235229.279814-1-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":null,"subject":"[PATCH 00/10] Hash-independent tests (part 3)","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:19Z","receivedAt":"2018-06-04T23:52:46Z","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\nIntroduced here is a test helper, test_translate, that allows lookups in\ntests based on the hash algorithm in use.  Alternatives are either\nspecified inline (the former for SHA-1, the latter for NewHash), or\n(more commonly) from a file based on a key.  Some basic examples of\ntranslations are provided and used throughout the tests.\n\nThe ultimate idea is that tests can simply drop a file into the\ntranslation directory and use their own test-specific translations if\nconvenient.\n\nThis series (and subsequent series) attempt to standardize slightly on\nour use of invalid object IDs in tests.  Since the actual invalid IDs\naren't usually very important, the translations I've made using the\ntables aren't necessarily entirely faithful: we will sometimes use\ndifferent SHA-1 object IDs than before, but we substitute only invalid\nobject IDs for different invalid ones.\n\nIt's likely in the future that test_translate will support additional\noptions depending on whether we want input, output, or internal formats.\n\nI had mentioned in a previous comment that a given test would be\nincluded in \"the next series\" (this one), but I redid the series and\ndecided to split it into smaller pieces, so it isn't included.  Sorry.\n\nComments on any aspect of the series are welcome, but thoughts on design\nand naming would be especially valuable.\n\nbrian m. carlson (10):\n  t: add tool to translate hash-related values\n  t0000: use hash translation table\n  t0002: abstract away SHA-1-specific constants\n  t0027: use $ZERO_OID\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/t0000-basic.sh               | 13 ++++-----\n t/t0002-gitfile.sh             | 26 +++++++++---------\n t/t0027-auto-crlf.sh           | 14 +++++-----\n t/t0064-sha1-array.sh          | 49 +++++++++++++++++++---------------\n t/t1006-cat-file.sh            |  4 +--\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        | 40 +++++++++++++++++++++++++++\n t/translate/hash-info          |  9 +++++++\n t/translate/oid                | 15 +++++++++++\n 12 files changed, 129 insertions(+), 57 deletions(-)\n create mode 100644 t/translate/hash-info\n create mode 100644 t/translate/oid\n\n"},{"id":"349304","messageId":"20180604235229.279814-2-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 01/10] t: add tool to translate hash-related values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:20Z","receivedAt":"2018-06-04T23:52:49Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Add a test function helper, test_translate, that will produce its first\nargument if the hash in use is SHA-1 and the second if its argument is\nNewHash.  Implement a mode that can read entries from a file as well for\nreusability across tests.\n\nFor the moment, use the length of the empty blob to determine the hash\nin use.  In the future, we can change this code so that it can use the\nconfiguration and learn about the difference in input, output, and\non-disk formats.\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.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/test-lib-functions.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n t/translate/hash-info   |  9 +++++++++\n t/translate/oid         | 15 +++++++++++++++\n 3 files changed, 64 insertions(+)\n create mode 100644 t/translate/hash-info\n create mode 100644 t/translate/oid\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 2b2181dca0..0e7067460b 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1147,3 +1147,43 @@ depacketize () {\n \t\t}\n \t'\n }\n+\n+test_translate_f_ () {\n+\tlocal file=\"$TEST_DIRECTORY/translate/$2\" &&\n+\tperl -e '\n+\t\t$delim = \"\\t\";\n+\t\t($hoidlen, $file, $arg) = @ARGV;\n+\t\topen($fh, \"<\", $file) or die \"open: $!\";\n+\t\twhile (<$fh>) {\n+\t\t\t# Allow specifying other delimiters.\n+\t\t\t$delim = $1 if /^#!\\sdelimiter\\s(.)/;\n+\t\t\tnext if /^#/;\n+\t\t\t@fields = split /$delim/, $_, 3;\n+\t\t\tif ($fields[0] eq $arg) {\n+\t\t\t\tprint($hoidlen == 40 ? $fields[1] : $fields[2]);\n+\t\t\t\tlast;\n+\t\t\t}\n+\t\t}\n+\t' \"$1\" \"$file\" \"$3\"\n+}\n+\n+# Without -f, print the first argument if we are using SHA-1 and the second if\n+# we're using NewHash.\n+# With -f FILE ARG, read the (by default) tab-delimited file from\n+# t/translate/FILE, finding the first field matching ARG and printing either the\n+# second or third field depending on the hash in use.\n+test_translate () {\n+\tlocal hoidlen=$(printf \"%s\" \"$EMPTY_BLOB\" | wc -c) &&\n+\tif [ \"$1\" = \"-f\" ]\n+\tthen\n+\t\tshift &&\n+\t\ttest_translate_f_ \"$hoidlen\" \"$@\"\n+\telse\n+\t\tif [ \"$hoidlen\" -eq 40 ]\n+\t\tthen\n+\t\t\tprintf \"%s\" \"$1\"\n+\t\telse\n+\t\t\tprintf \"%s\" \"$2\"\n+\t\tfi\n+\tfi\n+}\ndiff --git a/t/translate/hash-info b/t/translate/hash-info\nnew file mode 100644\nindex 0000000000..36cbd9a8eb\n--- /dev/null\n+++ b/t/translate/hash-info\n@@ -0,0 +1,9 @@\n+# Various facts about the hash algorithm in use for easy access in tests.\n+#\n+# Several aliases are provided for easy recall.\n+rawsz\t20\t32\n+oidlen\t20\t32\n+hexsz\t40\t64\n+hexoidlen\t40\t64\n+zero\t0000000000000000000000000000000000000000\t0000000000000000000000000000000000000000000000000000000000000000\n+zero-oid\t0000000000000000000000000000000000000000\t0000000000000000000000000000000000000000000000000000000000000000\ndiff --git a/t/translate/oid b/t/translate/oid\nnew file mode 100644\nindex 0000000000..8de0fd64af\n--- /dev/null\n+++ b/t/translate/oid\n@@ -0,0 +1,15 @@\n+# These are some common invalid and partial object IDs used in tests.\n+001\t0000000000000000000000000000000000000001\t0000000000000000000000000000000000000000000000000000000000000001\n+002\t0000000000000000000000000000000000000002\t0000000000000000000000000000000000000000000000000000000000000002\n+003\t0000000000000000000000000000000000000003\t0000000000000000000000000000000000000000000000000000000000000003\n+004\t0000000000000000000000000000000000000004\t0000000000000000000000000000000000000000000000000000000000000004\n+005\t0000000000000000000000000000000000000005\t0000000000000000000000000000000000000000000000000000000000000005\n+006\t0000000000000000000000000000000000000006\t0000000000000000000000000000000000000000000000000000000000000006\n+007\t0000000000000000000000000000000000000007\t0000000000000000000000000000000000000000000000000000000000000007\n+# All zeros or Fs missing one or two hex segments.\n+zero-1\t000000000000000000000000000000000000000\t000000000000000000000000000000000000000000000000000000000000000\n+zero-2\t00000000000000000000000000000000000000\t00000000000000000000000000000000000000000000000000000000000000\n+ff-1\tfffffffffffffffffffffffffffffffffffffff\tfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\n+ff-2\tffffffffffffffffffffffffffffffffffffff\tffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\n+numeric\t0123456789012345678901234567890123456789\t0123456789012345678901234567890123456789012345678901234567890123\n+deadbeef\tdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef\tdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n"},{"id":"349305","messageId":"20180604235229.279814-6-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 05/10] t0064: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:24Z","receivedAt":"2018-06-04T23:52:53Z","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..ab230179d2 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=$(test_translate 555555555555555555555555555555555555555 \\\n+\t\t555555555555555555555555555555555555555555555555555555555555555) &&\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":"349306","messageId":"20180604235229.279814-8-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 07/10] t1400: switch hard-coded object ID to variable","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:26Z","receivedAt":"2018-06-04T23:52:55Z","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 e1fd0f0ca8..ffaadf5f2d 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":"349307","messageId":"20180604235229.279814-5-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 04/10] t0027: use $ZERO_OID","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:23Z","receivedAt":"2018-06-04T23:52:58Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Use the ZERO_OID variable to express the all-zeros object ID so that it\nworks with hash algorithms of all sizes.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n t/t0027-auto-crlf.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\nindex beb5927f77..14fcd3f49f 100755\n--- a/t/t0027-auto-crlf.sh\n+++ b/t/t0027-auto-crlf.sh\n@@ -371,13 +371,13 @@ test_expect_success 'setup master' '\n \tgit checkout -b master &&\n \tgit add .gitattributes &&\n \tgit commit -m \"add .gitattributes\" . &&\n-\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\nLINEONE\\nLINETWO\\nLINETHREE\"     >LF &&\n-\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\r\\nLINEONE\\r\\nLINETWO\\r\\nLINETHREE\" >CRLF &&\n-\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\nLINEONE\\r\\nLINETWO\\nLINETHREE\"   >CRLF_mix_LF &&\n-\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\nLINEONE\\nLINETWO\\rLINETHREE\"     >LF_mix_CR &&\n-\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\r\\nLINEONE\\r\\nLINETWO\\rLINETHREE\"   >CRLF_mix_CR &&\n-\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\r\\nLINEONEQ\\r\\nLINETWO\\r\\nLINETHREE\" | q_to_nul >CRLF_nul &&\n-\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\nLINEONEQ\\nLINETWO\\nLINETHREE\" | q_to_nul >LF_nul &&\n+\tprintf \"\\$Id: $ZERO_OID \\$\\nLINEONE\\nLINETWO\\nLINETHREE\"     >LF &&\n+\tprintf \"\\$Id: $ZERO_OID \\$\\r\\nLINEONE\\r\\nLINETWO\\r\\nLINETHREE\" >CRLF &&\n+\tprintf \"\\$Id: $ZERO_OID \\$\\nLINEONE\\r\\nLINETWO\\nLINETHREE\"   >CRLF_mix_LF &&\n+\tprintf \"\\$Id: $ZERO_OID \\$\\nLINEONE\\nLINETWO\\rLINETHREE\"     >LF_mix_CR &&\n+\tprintf \"\\$Id: $ZERO_OID \\$\\r\\nLINEONE\\r\\nLINETWO\\rLINETHREE\"   >CRLF_mix_CR &&\n+\tprintf \"\\$Id: $ZERO_OID \\$\\r\\nLINEONEQ\\r\\nLINETWO\\r\\nLINETHREE\" | q_to_nul >CRLF_nul &&\n+\tprintf \"\\$Id: $ZERO_OID \\$\\nLINEONEQ\\nLINETWO\\nLINETHREE\" | q_to_nul >LF_nul &&\n \tcreate_NNO_MIX_files &&\n \tgit -c core.autocrlf=false add NNO_*.txt MIX_*.txt &&\n \tgit commit -m \"mixed line endings\" &&\n"},{"id":"349308","messageId":"20180604235229.279814-9-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 08/10] t1405: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:27Z","receivedAt":"2018-06-04T23:52:59Z","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":"349309","messageId":"20180604235229.279814-7-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 06/10] t1006: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:25Z","receivedAt":"2018-06-04T23:53:00Z","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 | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 13dd510b2e..3b82051e2b 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -141,14 +141,14 @@ test_expect_success '--batch-check without %(rest) considers whole line' '\n '\n \n tree_sha1=$(git write-tree)\n-tree_size=33\n+tree_size=$(($(test_translate -f hash-info 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_translate -f hash-info 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":"349310","messageId":"20180604235229.279814-4-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 03/10] t0002: abstract away SHA-1-specific constants","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:22Z","receivedAt":"2018-06-04T23:53:05Z","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 | 26 ++++++++++++++------------\n 1 file changed, 14 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh\nindex 3691023d51..020af4c53c 100755\n--- a/t/t0002-gitfile.sh\n+++ b/t/t0002-gitfile.sh\n@@ -89,14 +89,16 @@ test_expect_success 'enter_repo non-strict mode' '\n \t\tcd enter_repo &&\n \t\ttest_tick &&\n \t\ttest_commit foo &&\n+\t\tgit rev-parse HEAD >head-revision &&\n \t\tmv .git .realgit &&\n \t\techo \"gitdir: .realgit\" >.git\n \t) &&\n+\thead=$(cat enter_repo/head-revision) &&\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 +109,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":"349311","messageId":"20180604235229.279814-10-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 09/10] t1406: make hash-size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:28Z","receivedAt":"2018-06-04T23:53:07Z","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":"349312","messageId":"20180604235229.279814-11-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 10/10] t1407: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:29Z","receivedAt":"2018-06-04T23:53:09Z","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":"349313","messageId":"20180604235229.279814-3-sandals@crustytoothpaste.net","threadId":"48651","inReplyTo":"20180604235229.279814-1-sandals@crustytoothpaste.net","subject":"[PATCH 02/10] t0000: use hash translation table","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-04T23:52:21Z","receivedAt":"2018-06-04T23:53:14Z","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 af61d083b4..27ef9ecab2 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -978,12 +978,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_translate -f oid 001)\tdir/file1\n+\t100644 blob $(test_translate -f oid 002)\tdir/file2\n+\t100644 blob $(test_translate -f oid 003)\tdir/file3\n+\t100644 blob $(test_translate -f oid 004)\tdir/file4\n+\t100644 blob $(test_translate -f oid 005)\tdir/file5\n \tEOF\n \tgit update-index --index-info <badobjects\n '\n"},{"id":"349439","messageId":"20180606061927.GA7098@tor.lan","threadId":"48651","inReplyTo":"20180604235229.279814-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-06-06T06:19:27Z","receivedAt":"2018-06-06T06:19:57Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Some style nite inline\n\nOn Mon, Jun 04, 2018 at 11:52:20PM +0000, brian m. carlson wrote:\n> Add a test function helper, test_translate, that will produce its first\n> argument if the hash in use is SHA-1 and the second if its argument is\n> NewHash.  Implement a mode that can read entries from a file as well for\n> reusability across tests.\n> \n> For the moment, use the length of the empty blob to determine the hash\n> in use.  In the future, we can change this code so that it can use the\n> configuration and learn about the difference in input, output, and\n> on-disk formats.\n> \n> Implement two basic lookup charts, one for common invalid or synthesized\n> object IDs, and one for various facts about the hash function in use.\n> \n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  t/test-lib-functions.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n>  t/translate/hash-info   |  9 +++++++++\n>  t/translate/oid         | 15 +++++++++++++++\n>  3 files changed, 64 insertions(+)\n>  create mode 100644 t/translate/hash-info\n>  create mode 100644 t/translate/oid\n> \n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 2b2181dca0..0e7067460b 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -1147,3 +1147,43 @@ depacketize () {\n>  \t\t}\n>  \t'\n>  }\n> +\n> +test_translate_f_ () {\n> +\tlocal file=\"$TEST_DIRECTORY/translate/$2\" &&\n\nUnless I'm wrong, we don't use the \"local\" keyword ?\n\n> +\tperl -e '\n\nThe bare \"perl\" is better spelled as \"$PERL_PATH\"\n\n\n> +\t\t$delim = \"\\t\";\n> +\t\t($hoidlen, $file, $arg) = @ARGV;\n> +\t\topen($fh, \"<\", $file) or die \"open: $!\";\n> +\t\twhile (<$fh>) {\n> +\t\t\t# Allow specifying other delimiters.\n> +\t\t\t$delim = $1 if /^#!\\sdelimiter\\s(.)/;\n> +\t\t\tnext if /^#/;\n> +\t\t\t@fields = split /$delim/, $_, 3;\n> +\t\t\tif ($fields[0] eq $arg) {\n> +\t\t\t\tprint($hoidlen == 40 ? $fields[1] : $fields[2]);\n> +\t\t\t\tlast;\n> +\t\t\t}\n> +\t\t}\n> +\t' \"$1\" \"$file\" \"$3\"\n> +}\n> +\n> +# Without -f, print the first argument if we are using SHA-1 and the second if\n> +# we're using NewHash.\n> +# With -f FILE ARG, read the (by default) tab-delimited file from\n> +# t/translate/FILE, finding the first field matching ARG and printing either the\n> +# second or third field depending on the hash in use.\n> +test_translate () {\n> +\tlocal hoidlen=$(printf \"%s\" \"$EMPTY_BLOB\" | wc -c) &&\n> +\tif [ \"$1\" = \"-f\" ]\n\nStyle nit, please avoid [] and use test:\n\tif test \"$1\" = \"-f\"\n\nAnd more [] below\n\n> +\tthen\n> +\t\tshift &&\n> +\t\ttest_translate_f_ \"$hoidlen\" \"$@\"\n> +\telse\n> +\t\tif [ \"$hoidlen\" -eq 40 ]\n> +\t\tthen\n> +\t\t\tprintf \"%s\" \"$1\"\n> +\t\telse\n> +\t\t\tprintf \"%s\" \"$2\"\n> +\t\tfi\n> +\tfi\n> +}\n> diff --git a/t/translate/hash-info b/t/translate/hash-info\n> new file mode 100644\n> index 0000000000..36cbd9a8eb\n> --- /dev/null\n> +++ b/t/translate/hash-info\n> @@ -0,0 +1,9 @@\n> +# Various facts about the hash algorithm in use for easy access in tests.\n> +#\n> +# Several aliases are provided for easy recall.\n> +rawsz\t20\t32\n> +oidlen\t20\t32\n> +hexsz\t40\t64\n> +hexoidlen\t40\t64\n> +zero\t0000000000000000000000000000000000000000\t0000000000000000000000000000000000000000000000000000000000000000\n> +zero-oid\t0000000000000000000000000000000000000000\t0000000000000000000000000000000000000000000000000000000000000000\n> diff --git a/t/translate/oid b/t/translate/oid\n> new file mode 100644\n> index 0000000000..8de0fd64af\n> --- /dev/null\n> +++ b/t/translate/oid\n> @@ -0,0 +1,15 @@\n> +# These are some common invalid and partial object IDs used in tests.\n> +001\t0000000000000000000000000000000000000001\t0000000000000000000000000000000000000000000000000000000000000001\n> +002\t0000000000000000000000000000000000000002\t0000000000000000000000000000000000000000000000000000000000000002\n> +003\t0000000000000000000000000000000000000003\t0000000000000000000000000000000000000000000000000000000000000003\n> +004\t0000000000000000000000000000000000000004\t0000000000000000000000000000000000000000000000000000000000000004\n> +005\t0000000000000000000000000000000000000005\t0000000000000000000000000000000000000000000000000000000000000005\n> +006\t0000000000000000000000000000000000000006\t0000000000000000000000000000000000000000000000000000000000000006\n> +007\t0000000000000000000000000000000000000007\t0000000000000000000000000000000000000000000000000000000000000007\n> +# All zeros or Fs missing one or two hex segments.\n> +zero-1\t000000000000000000000000000000000000000\t000000000000000000000000000000000000000000000000000000000000000\n> +zero-2\t00000000000000000000000000000000000000\t00000000000000000000000000000000000000000000000000000000000000\n> +ff-1\tfffffffffffffffffffffffffffffffffffffff\tfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\n> +ff-2\tffffffffffffffffffffffffffffffffffffff\tffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff\n> +numeric\t0123456789012345678901234567890123456789\t0123456789012345678901234567890123456789012345678901234567890123\n> +deadbeef\tdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef\tdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n"},{"id":"349440","messageId":"20180606070222.GA11992@tor.lan","threadId":"48651","inReplyTo":"20180604235229.279814-5-sandals@crustytoothpaste.net","subject":"Re: [PATCH 04/10] t0027: use $ZERO_OID","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-06-06T07:02:23Z","receivedAt":"2018-06-06T07:02:43Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Mon, Jun 04, 2018 at 11:52:23PM +0000, brian m. carlson wrote:\n> Use the ZERO_OID variable to express the all-zeros object ID so that it\n> works with hash algorithms of all sizes.\n> \n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  t/t0027-auto-crlf.sh | 14 +++++++-------\n>  1 file changed, 7 insertions(+), 7 deletions(-)\n> \n> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\n> index beb5927f77..14fcd3f49f 100755\n> --- a/t/t0027-auto-crlf.sh\n> +++ b/t/t0027-auto-crlf.sh\n> @@ -371,13 +371,13 @@ test_expect_success 'setup master' '\n>  \tgit checkout -b master &&\n>  \tgit add .gitattributes &&\n>  \tgit commit -m \"add .gitattributes\" . &&\n> -\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\nLINEONE\\nLINETWO\\nLINETHREE\"     >LF &&\n> -\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\r\\nLINEONE\\r\\nLINETWO\\r\\nLINETHREE\" >CRLF &&\n> -\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\nLINEONE\\r\\nLINETWO\\nLINETHREE\"   >CRLF_mix_LF &&\n> -\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\nLINEONE\\nLINETWO\\rLINETHREE\"     >LF_mix_CR &&\n> -\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\r\\nLINEONE\\r\\nLINETWO\\rLINETHREE\"   >CRLF_mix_CR &&\n> -\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\r\\nLINEONEQ\\r\\nLINETWO\\r\\nLINETHREE\" | q_to_nul >CRLF_nul &&\n> -\tprintf \"\\$Id: 0000000000000000000000000000000000000000 \\$\\nLINEONEQ\\nLINETWO\\nLINETHREE\" | q_to_nul >LF_nul &&\n> +\tprintf \"\\$Id: $ZERO_OID \\$\\nLINEONE\\nLINETWO\\nLINETHREE\"     >LF &&\n> +\tprintf \"\\$Id: $ZERO_OID \\$\\r\\nLINEONE\\r\\nLINETWO\\r\\nLINETHREE\" >CRLF &&\n> +\tprintf \"\\$Id: $ZERO_OID \\$\\nLINEONE\\r\\nLINETWO\\nLINETHREE\"   >CRLF_mix_LF &&\n> +\tprintf \"\\$Id: $ZERO_OID \\$\\nLINEONE\\nLINETWO\\rLINETHREE\"     >LF_mix_CR &&\n> +\tprintf \"\\$Id: $ZERO_OID \\$\\r\\nLINEONE\\r\\nLINETWO\\rLINETHREE\"   >CRLF_mix_CR &&\n> +\tprintf \"\\$Id: $ZERO_OID \\$\\r\\nLINEONEQ\\r\\nLINETWO\\r\\nLINETHREE\" | q_to_nul >CRLF_nul &&\n> +\tprintf \"\\$Id: $ZERO_OID \\$\\nLINEONEQ\\nLINETWO\\nLINETHREE\" | q_to_nul >LF_nul &&\n>  \tcreate_NNO_MIX_files &&\n>  \tgit -c core.autocrlf=false add NNO_*.txt MIX_*.txt &&\n>  \tgit commit -m \"mixed line endings\" &&\n\nNothing wrong with the patch.\nThere is, however, a trick in t0027 to transform the different IDs back to a bunch of '0'.\nThe content of the file use only uppercase letters, and all lowercase ad digits\nare converted like this:\n\ncompare_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 <\"$3\" >\"$act\" &&\n\ttest_cmp \"$exp\" \"$act\" &&\n\trm \"$exp\" \"$act\"\n}\n\nIn the long term the 'tr' may need an additional 'sed' expression.\n"},{"id":"349565","messageId":"20180606205845.GB1381@sigill.intra.peff.net","threadId":"48651","inReplyTo":"20180606061927.GA7098@tor.lan","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-06T20:58:46Z","receivedAt":"2018-06-06T20:58:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 06, 2018 at 08:19:27AM +0200, Torsten Bögershausen wrote:\n\n> > +test_translate_f_ () {\n> > +\tlocal file=\"$TEST_DIRECTORY/translate/$2\" &&\n> \n> Unless I'm wrong, we don't use the \"local\" keyword ?\n\nWe've got a test balloon out; see 01d3a526ad (t0000: check whether the\nshell supports the \"local\" keyword, 2017-10-26). I think it's reasonable\nto consider starting its use.\n\n> > +\tperl -e '\n> \n> The bare \"perl\" is better spelled as \"$PERL_PATH\"\n\nThis use is OK. Since a0e0ec9f7d (t: provide a perl() function which\nuses $PERL_PATH, 2013-10-28), most common uses handle this automatically\n(there are some exceptions, covered in t/README).\n\n> > +\tif [ \"$1\" = \"-f\" ]\n> \n> Style nit, please avoid [] and use test:\n> \tif test \"$1\" = \"-f\"\n\nThis I agree with. :)\n\n-Peff\n"},{"id":"349575","messageId":"20180607005704.GA862596@genre.crustytoothpaste.net","threadId":"48651","inReplyTo":"20180606205845.GB1381@sigill.intra.peff.net","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-07T00:57:04Z","receivedAt":"2018-06-07T00:58:49Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Jun 06, 2018 at 04:58:46PM -0400, Jeff King wrote:\n> On Wed, Jun 06, 2018 at 08:19:27AM +0200, Torsten Bögershausen wrote:\n> \n> > > +test_translate_f_ () {\n> > > +\tlocal file=\"$TEST_DIRECTORY/translate/$2\" &&\n> > \n> > Unless I'm wrong, we don't use the \"local\" keyword ?\n> \n> We've got a test balloon out; see 01d3a526ad (t0000: check whether the\n> shell supports the \"local\" keyword, 2017-10-26). I think it's reasonable\n> to consider starting its use.\n\nI used it because it's already in use earlier in the file in some of the\nmingw_* functions.  Perhaps we happen to know that our mingw systems\nwill always have a suitable /bin/sh, but I suppose some less capable\nshells would still have choked on it by now.\n\nI can remove it if necessary, but it didn't seem necessary.\n\n> > > +\tperl -e '\n> > \n> > The bare \"perl\" is better spelled as \"$PERL_PATH\"\n> \n> This use is OK. Since a0e0ec9f7d (t: provide a perl() function which\n> uses $PERL_PATH, 2013-10-28), most common uses handle this automatically\n> (there are some exceptions, covered in t/README).\n\nThis was exactly my reasoning.\n\n> > > +\tif [ \"$1\" = \"-f\" ]\n> > \n> > Style nit, please avoid [] and use test:\n> > \tif test \"$1\" = \"-f\"\n> \n> This I agree with. :)\n\nYeah, I forgot that that's our style.  I'll fix that.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"349576","messageId":"20180607012549.GB862596@genre.crustytoothpaste.net","threadId":"48651","inReplyTo":"20180606070222.GA11992@tor.lan","subject":"Re: [PATCH 04/10] t0027: use $ZERO_OID","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-07T01:25:50Z","receivedAt":"2018-06-07T01:26:01Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Jun 06, 2018 at 09:02:23AM +0200, Torsten Bögershausen wrote:\n> Nothing wrong with the patch.\n> There is, however, a trick in t0027 to transform the different IDs back to a bunch of '0'.\n> The content of the file use only uppercase letters, and all lowercase ad digits\n> are converted like this:\n> \n> 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 <\"$3\" >\"$act\" &&\n> \ttest_cmp \"$exp\" \"$act\" &&\n> \trm \"$exp\" \"$act\"\n> }\n> \n> In the long term the 'tr' may need an additional 'sed' expression.\n\nI'll take a look.  That may end up being a more robust solution.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"349581","messageId":"20180607024048.GC3898@sigill.intra.peff.net","threadId":"48651","inReplyTo":"20180607005704.GA862596@genre.crustytoothpaste.net","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-06-07T02:40:49Z","receivedAt":"2018-06-07T02:40:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 07, 2018 at 12:57:04AM +0000, brian m. carlson wrote:\n\n> > > Unless I'm wrong, we don't use the \"local\" keyword ?\n> > \n> > We've got a test balloon out; see 01d3a526ad (t0000: check whether the\n> > shell supports the \"local\" keyword, 2017-10-26). I think it's reasonable\n> > to consider starting its use.\n> \n> I used it because it's already in use earlier in the file in some of the\n> mingw_* functions.  Perhaps we happen to know that our mingw systems\n> will always have a suitable /bin/sh, but I suppose some less capable\n> shells would still have choked on it by now.\n\nWe do know in that case; it's always bash under mingw.\n\nThat said...\n\n> I can remove it if necessary, but it didn't seem necessary.\n\nI feel OK about starting to use it, with the knowledge that we may get a\nlate-comer who hasn't even tested v2.16.0 yet and says \"no, wait! My\nshell doesn't support local!\". And then we'd have to deal with it then.\nBut I suspect that won't happen, or it will turn out that the shell in\nquestion is unusable for some other reason anyway.\n\n-Peff\n"},{"id":"349877","messageId":"20180611074743.GA24066@flurp.local","threadId":"48651","inReplyTo":"20180604235229.279814-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-11T07:47:43Z","receivedAt":"2018-06-11T07:47:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 04, 2018 at 11:52:20PM +0000, brian m. carlson wrote:\n> Add a test function helper, test_translate, that will produce its first\n> argument if the hash in use is SHA-1 and the second if its argument is\n> NewHash.  Implement a mode that can read entries from a file as well for\n> reusability across tests.\n\nThe word \"translate\" is very generic and is (at least in my mind)\nstrongly associated with i18n/l10n, so the name test_translate() may\nbe confusing for readers. Perhaps test_oid_lookup() or test_oid_get()\nor even just test_oid()?\n\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> @@ -1147,3 +1147,43 @@ depacketize () {\n> +test_translate_f_ () {\n> +\tlocal file=\"$TEST_DIRECTORY/translate/$2\" &&\n> +\tperl -e '\n> +\t\t$delim = \"\\t\";\n> +\t\t($hoidlen, $file, $arg) = @ARGV;\n> +\t\topen($fh, \"<\", $file) or die \"open: $!\";\n> +\t\twhile (<$fh>) {\n> +\t\t\t# Allow specifying other delimiters.\n> +\t\t\t$delim = $1 if /^#!\\sdelimiter\\s(.)/;\n> +\t\t\tnext if /^#/;\n> +\t\t\t@fields = split /$delim/, $_, 3;\n> +\t\t\tif ($fields[0] eq $arg) {\n> +\t\t\t\tprint($hoidlen == 40 ? $fields[1] : $fields[2]);\n> +\t\t\t\tlast;\n> +\t\t\t}\n> +\t\t}\n> +\t' \"$1\" \"$file\" \"$3\"\n> +}\n\nThis is a very expensive lookup since it invokes a heavyweight command\n(perl, in this case) for *every* OID it needs to retrieve from the\nfile. Windows users, especially, will likely not be happy about this.\nSee below for an alternative.\n\n> +# Without -f, print the first argument if we are using SHA-1 and the second if\n> +# we're using NewHash.\n> +# With -f FILE ARG, read the (by default) tab-delimited file from\n> +# t/translate/FILE, finding the first field matching ARG and printing either the\n> +# second or third field depending on the hash in use.\n> +test_translate () {\n> +\tlocal hoidlen=$(printf \"%s\" \"$EMPTY_BLOB\" | wc -c) &&\n> +\tif [ \"$1\" = \"-f\" ]\n> +\tthen\n> +\t\tshift &&\n> +\t\ttest_translate_f_ \"$hoidlen\" \"$@\"\n> +\telse\n> +\t\tif [ \"$hoidlen\" -eq 40 ]\n> +\t\tthen\n> +\t\t\tprintf \"%s\" \"$1\"\n> +\t\telse\n> +\t\t\tprintf \"%s\" \"$2\"\n> +\t\tfi\n> +\tfi\n> +}\n\nThis is less flexible than I had expected, allowing for only SHA1 and\nNewHash. When you had written about OID lookup table functionality in\nemail previously, my impression was that the tables would allow values\nfor arbitrary hash algorithms. Such flexibility would allow people to\nexperiment with hash algorithms without having to once again retrofit\nthe test suite machinery.\n\nHere's what I had envisioned when reading your emails about OID lookup\ntable functionality:\n\n--- >8 ---\ntest_detect_hash () {\n    test_hash_algo=...\n}\n    \ntest_oid_cache () {\n    while read tag rest\n    do\n        case $tag in \\#*) continue ;; esac\n\n        for x in $rest\n        do\n            k=${x%:*}\n            v=${x#*:}\n            if test \"$k\" = $test_hash_algo\n            then\n                eval \"test_oid_$tag=$v\"\n                break\n            fi\n        done\n    done\n}\n\ntest_oid () {\n    if test $# -gt 1\n    then\n        test_oid_cache <<-EOF\n        $*\n        EOF\n    fi\n    eval \"echo \\$test_oid_$1\"\n}\n--- >8 ---\n\ntest_detect_hash() would detect the hash algorithm and record it\ninstead of having to determine it each time an OID needs to be\n\"translated\". It probably would be called by test-lib.sh.\n\ntest_oid_cache() reads a table of OID's and caches them so that\nsubsequent look-ups are fast. It would be called (possibly multiple\ntimes) by any script needing to \"translate\" OID's. Each line of the\ntable has form \"tag algo:value ...\". Lines starting with '#' are\ncomments (as in your implementation). Since it reads stdin, it works\nequally well reading OID tables from files and here-docs, which\nprovides extra flexibility for test authors. For instance:\n\n    test_oid_cache <translate/hash-info\n\nor:\n\n    test_oid_cache <<-\\EOF\n    rawsz sha1:20 NewHash:32\n    hexsz sha1:40 NewHash:64\n    EOF\n\ntest_oid() does the actual OID lookup/translation. It looks up a\npre-cached value or, for convenience (as per your implementation), can\nchoose between values specified at invocation time. For example, the\nsimple case:\n\n    $(test_oid hexsz)\n\nAnd, when specifying values from which to choose based upon hash\nalgorithm:\n\n    $(test_oid bored sha1:deadbeef NewHash:feedface)\n\nA nice property of how this implementation caches values is that you\ndon't need test_oid() for really simple cases. You can just access the\nvariable directly. For instance: $test_oid_hexsz\n\nAnother nice property of how caching is implemented is that someone\ntesting a new hash algorithm doesn't have edit the existing tables to\ntack the value for the new algorithm onto the end of each line. It\nworks equally well to place those values in a new file or new here-doc\nor simply append new lines to existing files or here-docs. For\ninstance, someone testing algorithm \"NewShiny\" can just add those\nlines without having to modify existing lines:\n\n    test_oid_cache <<-\\EOF\n    rawsz sha1:20 NewHash:32\n    hexsz sha1:40 NewHash:64\n    # testing NewShiny algorithm\n    rawsz: NewShiny:42\n    hexsz: NewShiny:84\n    EOF\n"},{"id":"349878","messageId":"CAPig+cSKCCzVtCqK7WctfZkc+jLUYAhDY_7L+YPkMx-3wanHHQ@mail.gmail.com","threadId":"48651","inReplyTo":"20180604235229.279814-4-sandals@crustytoothpaste.net","subject":"Re: [PATCH 03/10] t0002: abstract away SHA-1-specific constants","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-11T07:59:21Z","receivedAt":"2018-06-11T07:59:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 4, 2018 at 7:52 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>\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> @@ -89,14 +89,16 @@ test_expect_success 'enter_repo non-strict mode' '\n>                 cd enter_repo &&\n>                 test_tick &&\n>                 test_commit foo &&\n> +               git rev-parse HEAD >head-revision &&\n>                 mv .git .realgit &&\n>                 echo \"gitdir: .realgit\" >.git\n>         ) &&\n> +       head=$(cat enter_repo/head-revision) &&\n\nStashing the value temporarily in a file (\"head-revision\") is\nunnecessary and somewhat ugly. Just grab it directly after the\nsubshell exits:\n\n    (\n        cd enter_repo &&\n        ...\n    ) &&\n    head=$(git -C enter_repo rev-parse HEAD) &&\n    ...\n"},{"id":"349879","messageId":"CAPig+cTm4BYZjsFSV8AZ-jMjVPtze5H_7XUG1wBhmuFghJbc=g@mail.gmail.com","threadId":"48651","inReplyTo":"20180604235229.279814-6-sandals@crustytoothpaste.net","subject":"Re: [PATCH 05/10] t0064: make hash size independent","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-11T08:09:05Z","receivedAt":"2018-06-11T08:09:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 4, 2018 at 7:52 PM, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> Compute test values of the appropriate size instead of hard-coding\n> 40-character values.  Rename the echo20 function to echoid, since the\n> values may be of varying sizes.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> diff --git a/t/t0064-sha1-array.sh b/t/t0064-sha1-array.sh\n> @@ -3,30 +3,30 @@\n> -echo20 () {\n> +echoid () {\n>         prefix=\"${1:+$1 }\"\n>         shift\n>         while test $# -gt 0\n>         do\n> -               echo \"$prefix$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1\"\n> +               echo \"$prefix$ZERO_OID\" | sed -e \"s/00/$1/g\"\n>                 shift\n>         done\n>  }\n>\n>\n>  test_expect_success 'lookup with almost duplicate values' '\n> +       # n-1 5s\n> +       root=$(test_translate 555555555555555555555555555555555555555 \\\n> +               555555555555555555555555555555555555555555555555555555555555555) &&\n\nThis is rather unwieldy and ugly. How about simply re-using echoid()\nto compute the value, like this:\n\n    root=$(echoid \"\" 55) &&\n    root=${root%5} &&\n\nThat is, use echoid() to generate the all-5's OID of correct length\nand then chop the last '5' off the end.\n\n>         {\n> -               echo \"append 5555555555555555555555555555555555555555\" &&\n> -               echo \"append 555555555555555555555555555555555555555f\" &&\n> -               echo20 lookup 55\n> +               id1=\"${root}5\" &&\n> +               id2=\"${root}f\" &&\n> +               echo \"append $id1\" &&\n> +               echo \"append $id2\" &&\n> +               echoid lookup 55\n"},{"id":"349962","messageId":"20180612010513.GH38834@genre.crustytoothpaste.net","threadId":"48651","inReplyTo":"20180611074743.GA24066@flurp.local","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-12T01:05:13Z","receivedAt":"2018-06-12T01:05:24Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Jun 11, 2018 at 03:47:43AM -0400, Eric Sunshine wrote:\n> On Mon, Jun 04, 2018 at 11:52:20PM +0000, brian m. carlson wrote:\n> > Add a test function helper, test_translate, that will produce its first\n> > argument if the hash in use is SHA-1 and the second if its argument is\n> > NewHash.  Implement a mode that can read entries from a file as well for\n> > reusability across tests.\n> \n> The word \"translate\" is very generic and is (at least in my mind)\n> strongly associated with i18n/l10n, so the name test_translate() may\n> be confusing for readers. Perhaps test_oid_lookup() or test_oid_get()\n> or even just test_oid()?\n\ntest_oid would be fine.  One note is that this doesn't always produce\nOIDs; sometimes it will produce other values, but as long as you don't\nthink that's too confusing, I'm fine with it.\n\n> This is a very expensive lookup since it invokes a heavyweight command\n> (perl, in this case) for *every* OID it needs to retrieve from the\n> file. Windows users, especially, will likely not be happy about this.\n> See below for an alternative.\n\nI agree perl would be expensive if it were invoked frequently, but\nexcepting SHA1-prerequisite tests, this function is invoked 32 times in\nthe entire testsuite.\n\nOne of the reasons I chose perl was because we have a variety of cases\nwhere we'll need spaces in values, and those tend to be complex in\nshell.\n\n> This is less flexible than I had expected, allowing for only SHA1 and\n> NewHash. When you had written about OID lookup table functionality in\n> email previously, my impression was that the tables would allow values\n> for arbitrary hash algorithms. Such flexibility would allow people to\n> experiment with hash algorithms without having to once again retrofit\n> the test suite machinery.\n\nI wasn't thinking of that as a goal, but that would be a nice\nimprovement.\n\n> Here's what I had envisioned when reading your emails about OID lookup\n> table functionality:\n> \n> --- >8 ---\n> test_detect_hash () {\n>     test_hash_algo=...\n> }\n>     \n> test_oid_cache () {\n>     while read tag rest\n>     do\n>         case $tag in \\#*) continue ;; esac\n> \n>         for x in $rest\n>         do\n>             k=${x%:*}\n>             v=${x#*:}\n>             if test \"$k\" = $test_hash_algo\n>             then\n>                 eval \"test_oid_$tag=$v\"\n>                 break\n>             fi\n>         done\n>     done\n> }\n> \n> test_oid () {\n>     if test $# -gt 1\n>     then\n>         test_oid_cache <<-EOF\n>         $*\n>         EOF\n>     fi\n>     eval \"echo \\$test_oid_$1\"\n> }\n\nUsing shell variables like this does have the downside that we're\nrestricted to only characters allowed in shell variables.  That was\nsomething I was trying to avoid, but it certainly isn't fatal.\n\n> test_detect_hash() would detect the hash algorithm and record it\n> instead of having to determine it each time an OID needs to be\n> \"translated\". It probably would be called by test-lib.sh.\n\nWe'll probably have to deal with multiple hashes in the future,\nincluding for input and output, but this could probably be coerced to\nhandle that case.\n\n> And, when specifying values from which to choose based upon hash\n> algorithm:\n> \n>     $(test_oid bored sha1:deadbeef NewHash:feedface)\n\nThis syntax won't exactly be usable, because we have to deal with spaces\nin values.  It shouldn't be too much of a problem to just use\ntest_oid_cache at the top of the file, though.\n\n> A nice property of how this implementation caches values is that you\n> don't need test_oid() for really simple cases. You can just access the\n> variable directly. For instance: $test_oid_hexsz\n\nBecause we're going to need multiple hash support in the future, for\ninput, output, and on-disk, I feel like this is not a good direction for\nus to go in the testsuite.  Internally, those variable names are likely\nto change.\n\n> Another nice property of how caching is implemented is that someone\n> testing a new hash algorithm doesn't have edit the existing tables to\n> tack the value for the new algorithm onto the end of each line. It\n> works equally well to place those values in a new file or new here-doc\n> or simply append new lines to existing files or here-docs. For\n> instance, someone testing algorithm \"NewShiny\" can just add those\n> lines without having to modify existing lines:\n\nThat would be convenient.\n\nI like a lot of things about your implementation.  This has been helpful\nfeedback; let me think about it some and reroll.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"349963","messageId":"20180612010858.GI38834@genre.crustytoothpaste.net","threadId":"48651","inReplyTo":"CAPig+cTm4BYZjsFSV8AZ-jMjVPtze5H_7XUG1wBhmuFghJbc=g@mail.gmail.com","subject":"Re: [PATCH 05/10] t0064: make hash size independent","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-12T01:08:59Z","receivedAt":"2018-06-12T01:09:07Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Jun 11, 2018 at 04:09:05AM -0400, Eric Sunshine wrote:\n> On Mon, Jun 4, 2018 at 7:52 PM, brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >  test_expect_success 'lookup with almost duplicate values' '\n> > +       # n-1 5s\n> > +       root=$(test_translate 555555555555555555555555555555555555555 \\\n> > +               555555555555555555555555555555555555555555555555555555555555555) &&\n> \n> This is rather unwieldy and ugly. How about simply re-using echoid()\n> to compute the value, like this:\n> \n>     root=$(echoid \"\" 55) &&\n>     root=${root%5} &&\n> \n> That is, use echoid() to generate the all-5's OID of correct length\n> and then chop the last '5' off the end.\n\nSure, that would be nicer.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"349970","messageId":"CAPig+cSEpCTg+rYQYNQ5wUz3PL+51dr7bQdn-2cOWgg2ScdXNA@mail.gmail.com","threadId":"48651","inReplyTo":"20180612010513.GH38834@genre.crustytoothpaste.net","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-12T07:29:47Z","receivedAt":"2018-06-12T07:29:51Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 11, 2018 at 9:05 PM, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> On Mon, Jun 11, 2018 at 03:47:43AM -0400, Eric Sunshine wrote:\n>> The word \"translate\" is very generic and is (at least in my mind)\n>> strongly associated with i18n/l10n, so the name test_translate() may\n>> be confusing for readers. Perhaps test_oid_lookup() or test_oid_get()\n>> or even just test_oid()?\n>\n> test_oid would be fine.  One note is that this doesn't always produce\n> OIDs; sometimes it will produce other values, but as long as you don't\n> think that's too confusing, I'm fine with it.\n\nIt was surprising to see it used for non-OID's (such as hash\ncharacteristics), but not hard to deal with.\n\nOne could also view this as a generic key/value cache (not specific to\nOID's) with overriding super-key (the hash algorithm, in this case),\nwhich would allow for more generic name than test_oid(), but we don't\nhave to go there presently.\n\n>> This is a very expensive lookup since it invokes a heavyweight command\n>> (perl, in this case) for *every* OID it needs to retrieve from the\n>> file. Windows users, especially, will likely not be happy about this.\n>> See below for an alternative.\n>\n> I agree perl would be expensive if it were invoked frequently, but\n> excepting SHA1-prerequisite tests, this function is invoked 32 times in\n> the entire testsuite.\n>\n> One of the reasons I chose perl was because we have a variety of cases\n> where we'll need spaces in values, and those tend to be complex in\n> shell.\n\nCan you give examples of cases in which values will contain spaces? It\nwasn't obvious from this patch series that such a need would arise.\n\nAre these values totally free-form? If not, some character (such as\n\"_\", \"-\", \".\", etc.) could act as a stand-in for space. That shouldn't\nbe too hard to handle.\n\n>> Here's what I had envisioned when reading your emails about OID lookup\n>> table functionality:\n>>\n>> --- >8 ---\n>> test_oid_cache () {\n>>     while read tag rest\n>>     do\n>>         case $tag in \\#*) continue ;; esac\n>>\n>>         for x in $rest\n>>         do\n>>             k=${x%:*}\n>>             v=${x#*:}\n>>             if test \"$k\" = $test_hash_algo\n>>             then\n>>                 eval \"test_oid_$tag=$v\"\n>>                 break\n>>             fi\n>>         done\n>>     done\n>> }\n>\n> Using shell variables like this does have the downside that we're\n> restricted to only characters allowed in shell variables.  That was\n> something I was trying to avoid, but it certainly isn't fatal.\n\nIs that just a general concern or do you have specific \"weird\" keys in mind?\n\n>> test_detect_hash() would detect the hash algorithm and record it\n>> instead of having to determine it each time an OID needs to be\n>> \"translated\". It probably would be called by test-lib.sh.\n>\n> We'll probably have to deal with multiple hashes in the future,\n> including for input and output, but this could probably be coerced to\n> handle that case.\n\nMy original version of test_oid_cache() actually allowed for that by\ncaching _all_ information from the tables rather than only values\ncorresponding to $test_hash_algo. It looked like this:\n\n--- >8 ---\ntest_oid_cache () {\n    while read tag rest\n    do\n        case $tag in \\#*) continue ;; esac\n\n        for x in $rest\n        do\n            eval \"test_oid_${tag}_${x%:*}=${x#*:}\"\n        done\n    done\n}\n--- >8 ---\n\nThe hash algorithm is incorporated into the cache variable name like\nthis: \"test_oid_hexsz_sha256\"\n\n>> And, when specifying values from which to choose based upon hash\n>> algorithm:\n>>\n>>     $(test_oid bored sha1:deadbeef NewHash:feedface)\n>\n> This syntax won't exactly be usable, because we have to deal with spaces\n> in values.  It shouldn't be too much of a problem to just use\n> test_oid_cache at the top of the file, though.\n\nSee above about possibly using a stand-in character for space.\n\n>> A nice property of how this implementation caches values is that you\n>> don't need test_oid() for really simple cases. You can just access the\n>> variable directly. For instance: $test_oid_hexsz\n>\n> Because we're going to need multiple hash support in the future, for\n> input, output, and on-disk, I feel like this is not a good direction for\n> us to go in the testsuite.  Internally, those variable names are likely\n> to change.\n\nIndeed, this isn't a real selling point; I only just thought of it\nwhile composing the mail. Going through the API is more robust.\n(Although, see above how the revised test_oid_cache() incorporates the\nhash algorithm into the cache variable name.)\n"},{"id":"350135","messageId":"20180614002205.GN38834@genre.crustytoothpaste.net","threadId":"48651","inReplyTo":"CAPig+cSEpCTg+rYQYNQ5wUz3PL+51dr7bQdn-2cOWgg2ScdXNA@mail.gmail.com","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-06-14T00:22:06Z","receivedAt":"2018-06-14T00:26:09Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Jun 12, 2018 at 03:29:47AM -0400, Eric Sunshine wrote:\n> On Mon, Jun 11, 2018 at 9:05 PM, brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> > test_oid would be fine.  One note is that this doesn't always produce\n> > OIDs; sometimes it will produce other values, but as long as you don't\n> > think that's too confusing, I'm fine with it.\n> \n> It was surprising to see it used for non-OID's (such as hash\n> characteristics), but not hard to deal with.\n> \n> One could also view this as a generic key/value cache (not specific to\n> OID's) with overriding super-key (the hash algorithm, in this case),\n> which would allow for more generic name than test_oid(), but we don't\n> have to go there presently.\n\nIt is essentially that.  I'm happy with the test_oid name provided\nothers are.  My only concern is that it would be confusing.\n\nI opted to use the same mechanism for hash characteristics because it\nseemed better than creating a lot of one-off functions that might have\nduplicative implementations.  But I'm open to arguments that having\ntest_oid_rawsz, test_oid_hexsz, etc. is better.\n\n> > I agree perl would be expensive if it were invoked frequently, but\n> > excepting SHA1-prerequisite tests, this function is invoked 32 times in\n> > the entire testsuite.\n> >\n> > One of the reasons I chose perl was because we have a variety of cases\n> > where we'll need spaces in values, and those tend to be complex in\n> > shell.\n> \n> Can you give examples of cases in which values will contain spaces? It\n> wasn't obvious from this patch series that such a need would arise.\n> \n> Are these values totally free-form? If not, some character (such as\n> \"_\", \"-\", \".\", etc.) could act as a stand-in for space. That shouldn't\n> be too hard to handle.\n\nt6030, which tests the bisect porcelain, is sensitive to the hash\nalgorithm because hash values are used as a secondary sort for the\nclosest commit.  Without totally gutting the test and redoing it, some\nsolution to produce something resembling a sane commit message would be\nhelpful.  We will also have cases where we need to provide strings to\nprintf(1), such as in some of the pack tests.\n\nI have a minor modification to your code which handles that at the cost\nof restricting us to one hash-key-value tuple on a line, which I think\nis an acceptable tradeoff.\n\n> > Using shell variables like this does have the downside that we're\n> > restricted to only characters allowed in shell variables.  That was\n> > something I was trying to avoid, but it certainly isn't fatal.\n> \n> Is that just a general concern or do you have specific \"weird\" keys in mind?\n\nI had originally thought of providing only the SHA-1 value instead of a\nnamed key, which would have necessitated allowing arbitrary inputs, but\nI ultimately decided that named keys were better.  I also tend to prefer\ndashes in inputs over underscores, since it's a bit easier to type, but\nthat's really a secondary concern.\n\nI think the benefits of an implementation closer to your outweigh the\ndownsides.\n\n> My original version of test_oid_cache() actually allowed for that by\n> caching _all_ information from the tables rather than only values\n> corresponding to $test_hash_algo. It looked like this:\n> \n> --- >8 ---\n> test_oid_cache () {\n>     while read tag rest\n>     do\n>         case $tag in \\#*) continue ;; esac\n> \n>         for x in $rest\n>         do\n>             eval \"test_oid_${tag}_${x%:*}=${x#*:}\"\n>         done\n>     done\n> }\n> --- >8 ---\n> \n> The hash algorithm is incorporated into the cache variable name like\n> this: \"test_oid_hexsz_sha256\"\n\nYeah, I basically reimplemented something similar to that based off your\ncode.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"353542","messageId":"20180724221712.GJ18502@genre.crustytoothpaste.net","threadId":"48651","inReplyTo":"20180611074743.GA24066@flurp.local","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-07-24T22:17:12Z","receivedAt":"2018-07-24T22:17:21Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Jun 11, 2018 at 03:47:43AM -0400, Eric Sunshine wrote:\n> Here's what I had envisioned when reading your emails about OID lookup\n> table functionality:\n> \n> --- >8 ---\n> test_detect_hash () {\n>     test_hash_algo=...\n> }\n> \n> test_oid_cache () {\n>     while read tag rest\n>     do\n>         case $tag in \\#*) continue ;; esac\n> \n>         for x in $rest\n>         do\n>             k=${x%:*}\n>             v=${x#*:}\n>             if test \"$k\" = $test_hash_algo\n>             then\n>                 eval \"test_oid_$tag=$v\"\n>                 break\n>             fi\n>         done\n>     done\n> }\n> \n> test_oid () {\n>     if test $# -gt 1\n>     then\n>         test_oid_cache <<-EOF\n>         $*\n>         EOF\n>     fi\n>     eval \"echo \\$test_oid_$1\"\n> }\n> --- >8 ---\n\nI'd like to use this as a basis for my v2, but I would need your\nsign-off in order to do that.  Would that be okay?\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"353553","messageId":"CAPig+cT44egNgzMKLHwJb-WHkpK1gnVrre_Ebv+O8xnemRRWRw@mail.gmail.com","threadId":"48651","inReplyTo":"20180724221712.GJ18502@genre.crustytoothpaste.net","subject":"Re: [PATCH 01/10] t: add tool to translate hash-related values","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-25T08:33:47Z","receivedAt":"2018-07-25T08:34:00Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jul 24, 2018 at 6:17 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> On Mon, Jun 11, 2018 at 03:47:43AM -0400, Eric Sunshine wrote:\n> > Here's what I had envisioned when reading your emails about OID lookup\n> > table functionality:\n> >\n> > --- >8 ---\n> > test_oid_cache () {\n> >     while read tag rest\n> >     do\n> >         case $tag in \\#*) continue ;; esac\n> >\n> >         for x in $rest\n> >         do\n> >             k=${x%:*}\n> >             v=${x#*:}\n> >             if test \"$k\" = $test_hash_algo\n> >             then\n> >                 eval \"test_oid_$tag=$v\"\n> >                 break\n> >             fi\n> >         done\n> >     done\n> > }\n> >\n> > test_oid () {\n> >     if test $# -gt 1\n> >     then\n> >         test_oid_cache <<-EOF\n> >         $*\n> >         EOF\n> >     fi\n> >     eval \"echo \\$test_oid_$1\"\n> > }\n> > --- >8 ---\n>\n> I'd like to use this as a basis for my v2, but I would need your\n> sign-off in order to do that.  Would that be okay?\n\nYou can have my sign-off for the code above.\n"}]}