[PATCH v2 00/11] Hash-independent tests (part 3)

[PATCH v2 00/11] Hash-independent tests (part 3)

[brian m. carlson]

This is next in the series of improvements to make tests
hash-independent.

This round modifies the helpers to more closely follow the
lighter-weight design that Eric Sunshine suggested in v1, at the cost of
requiring lookup keys to be shell identifiers.  If that's judged to be
undesirable, I can always hash the key before use.

Since we've decided on SHA-256, I put in an extra commit to update t0000
and remove the use of the SHA1 prerequisite.  As described in the commit
message, I wrote a Ruby script to synthesize the SHA-1 and SHA-256 blobs
and then used a second one to generate the trees, using the same
methodology for both algorithms. I did this specifically to ensure that
the object IDs are exactly the ones we think they are.

Changes from v1:
* Adopt pure shell approach for helper.
* Add tests for the helpers.
* Explicitly refer to SHA-256 now that we know it will be NewHash.
* Updated t0000 to remove SHA1 prerequisite.
* Change name of helper from test_translate to test_oid.
* Add helper to cache information in the shell.
* Simplified lookup of HEAD in t0002.
* Switched to using existing helper function in t0027.
* Simplified handling of IDs in t0064.

brian m. carlson (11):
  t: add tool to translate hash-related values
  t0000: use hash translation table
  t0000: update tests for SHA-256
  t0002: abstract away SHA-1 specific constants
  t0027: make hash size independent
  t0064: make hash size independent
  t1006: make hash size independent
  t1400: switch hard-coded object ID to variable
  t1405: make hash size independent
  t1406: make hash-size independent
  t1407: make hash size independent

 t/oid-info/hash-info           |   8 ++
 t/oid-info/oid                 |  29 ++++++
 t/oid-info/t0000               |  38 ++++++++
 t/t0000-basic.sh               | 168 ++++++++++++++++++++-------------
 t/t0002-gitfile.sh             |  26 ++---
 t/t0027-auto-crlf.sh           |   3 +-
 t/t0064-sha1-array.sh          |  49 +++++-----
 t/t1006-cat-file.sh            |   6 +-
 t/t1400-update-ref.sh          |   2 +-
 t/t1405-main-ref-store.sh      |   4 +-
 t/t1406-submodule-ref-store.sh |   6 +-
 t/t1407-worktree-ref-store.sh  |   4 +-
 t/test-lib-functions.sh        |  36 +++++++
 13 files changed, 267 insertions(+), 112 deletions(-)
 create mode 100644 t/oid-info/hash-info
 create mode 100644 t/oid-info/oid
 create mode 100644 t/oid-info/t0000

[PATCH v2 01/11] t: add tool to translate hash-related values

[brian m. carlson]

Add a test function helper, test_oid, that produces output that varies
depending on the hash in use.  Add an additional helper, test_oid_cache,
that can be used to load data for test_oid, either through a list of
filenames or on standard input.  Check that these functions work in
t0000, as the rest of the testsuite will soon come to depend on them.

Implement two basic lookup charts, one for common invalid or synthesized
object IDs, and one for various facts about the hash function in use.
Individual tests can use their own translation files to map object IDs
or other data that needs to vary across hash functions.  Provide
versions for both SHA-1 and SHA-256.

Note that due to the implementation used, keys cannot be anything but
shell identifier characters.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/oid-info/hash-info    |  8 ++++++++
 t/oid-info/oid          | 29 +++++++++++++++++++++++++++++
 t/t0000-basic.sh        | 29 +++++++++++++++++++++++++++++
 t/test-lib-functions.sh | 36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 102 insertions(+)
 create mode 100644 t/oid-info/hash-info
 create mode 100644 t/oid-info/oid

diff --git a/t/oid-info/hash-info b/t/oid-info/hash-info
new file mode 100644
index 0000000000..ccdbfdf974
--- /dev/null
+++ b/t/oid-info/hash-info
@@ -0,0 +1,8 @@
+rawsz sha1:20
+rawsz sha256:32
+
+hexsz sha1:40
+hexsz sha256:64
+
+zero sha1:0000000000000000000000000000000000000000
+zero sha256:0000000000000000000000000000000000000000000000000000000000000000
diff --git a/t/oid-info/oid b/t/oid-info/oid
new file mode 100644
index 0000000000..6f1c49dbc1
--- /dev/null
+++ b/t/oid-info/oid
@@ -0,0 +1,29 @@
+# These are some common invalid and partial object IDs used in tests.
+001	sha1:0000000000000000000000000000000000000001
+001	sha256:0000000000000000000000000000000000000000000000000000000000000001
+002	sha1:0000000000000000000000000000000000000002
+002	sha256:0000000000000000000000000000000000000000000000000000000000000002
+003	sha1:0000000000000000000000000000000000000003
+003	sha256:0000000000000000000000000000000000000000000000000000000000000003
+004	sha1:0000000000000000000000000000000000000004
+004	sha256:0000000000000000000000000000000000000000000000000000000000000004
+005	sha1:0000000000000000000000000000000000000005
+005	sha256:0000000000000000000000000000000000000000000000000000000000000005
+006	sha1:0000000000000000000000000000000000000006
+006	sha256:0000000000000000000000000000000000000000000000000000000000000006
+007	sha1:0000000000000000000000000000000000000007
+007	sha256:0000000000000000000000000000000000000000000000000000000000000007
+# All zeros or Fs missing one or two hex segments.
+zero_1		sha1:000000000000000000000000000000000000000
+zero_2		sha256:000000000000000000000000000000000000000000000000000000000000000
+zero_2		sha1:00000000000000000000000000000000000000
+zero_2		sha256:00000000000000000000000000000000000000000000000000000000000000
+ff_1		sha1:fffffffffffffffffffffffffffffffffffffff
+ff_1		sha256:fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
+ff_2		sha1:ffffffffffffffffffffffffffffffffffffff
+ff_2		sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
+# More various invalid OIDs.
+numeric		sha1:0123456789012345678901234567890123456789
+numeric		sha256:0123456789012345678901234567890123456789012345678901234567890123
+deadbeef	sha1:deadbeefdeadbeefdeadbeefdeadbeefdeadbeef
+deadbeef	sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 34859fe4a5..ace779bf7d 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -821,6 +821,35 @@ test_expect_success 'tests clean up even on failures' "
 	EOF
 "
 
+test_oid_cache hash-info oid
+
+test_expect_success 'test_oid_info provides sane info by default' '
+	test_oid zero &&
+	test_oid zero >actual &&
+	grep "00*" actual &&
+	test "$(test_oid hexsz)" -eq $(wc -c <actual) &&
+	test $(( $(test_oid rawsz) * 2)) -eq "$(test_oid hexsz)"
+'
+
+test_expect_success 'test_oid_info can look up data for SHA-1' '
+	test_detect_hash sha1 &&
+	test_oid zero >actual &&
+	grep "00*" actual &&
+	test $(wc -c <actual) -eq 40 &&
+	test "$(test_oid rawsz)" -eq 20 &&
+	test "$(test_oid hexsz)" -eq 40
+'
+
+test_expect_success 'test_oid_info can look up data for SHA-256' '
+	test_when_finished "test_detect_hash" &&
+	test_detect_hash sha256 &&
+	test_oid zero >actual &&
+	grep "00*" actual &&
+	test $(wc -c <actual) -eq 64 &&
+	test "$(test_oid rawsz)" -eq 32 &&
+	test "$(test_oid hexsz)" -eq 64
+'
+
 ################################################################
 # Basics of the basics
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 2b2181dca0..ac18789a70 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1147,3 +1147,39 @@ depacketize () {
 		}
 	'
 }
+
+test_detect_hash () {
+	if test -n "$1"
+	then
+		test_hash_algo="$1"
+	else
+		test_hash_algo='sha1'
+	fi
+}
+
+test_oid_cache () {
+	test -n "$test_hash_algo" || test_detect_hash
+	if test -n "$1"
+	then
+		while test -n "$1"
+		do
+			test_oid_cache <"$TEST_DIRECTORY/oid-info/$1"
+			shift
+		done
+		return $?
+	fi
+	while read _tag _rest
+	do
+		case $_tag in \#*)
+			continue;;
+		esac
+
+		_k="${_rest%:*}"
+		_v="${_rest#*:}"
+		eval "test_oid_${_k}_$_tag=\"$_v\""
+	done
+}
+
+test_oid () {
+	eval "printf '%s' \"\${test_oid_${test_hash_algo}_$1}\""
+}

[PATCH v2 02/11] t0000: use hash translation table

[brian m. carlson]

If the hash we're using is 32 bytes in size, attempting to insert a
20-byte object name won't work.  Since these are synthesized objects
that are almost all zeros, look them up in a translation table.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0000-basic.sh | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ace779bf7d..57d374f2dc 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -1007,12 +1007,13 @@ test_expect_success SHA1 'validate object ID for a known tree' '
 
 test_expect_success 'put invalid objects into the index' '
 	rm -f .git/index &&
-	cat >badobjects <<-\EOF &&
-	100644 blob 1000000000000000000000000000000000000000	dir/file1
-	100644 blob 2000000000000000000000000000000000000000	dir/file2
-	100644 blob 3000000000000000000000000000000000000000	dir/file3
-	100644 blob 4000000000000000000000000000000000000000	dir/file4
-	100644 blob 5000000000000000000000000000000000000000	dir/file5
+	suffix=$(echo $ZERO_OID | sed -e "s/^.//") &&
+	cat >badobjects <<-EOF &&
+	100644 blob $(test_oid 001)	dir/file1
+	100644 blob $(test_oid 002)	dir/file2
+	100644 blob $(test_oid 003)	dir/file3
+	100644 blob $(test_oid 004)	dir/file4
+	100644 blob $(test_oid 005)	dir/file5
 	EOF
 	git update-index --index-info <badobjects
 '

[PATCH v2 03/11] t0000: update tests for SHA-256

[brian m. carlson]

Test t0000 tests the "basics of the basics" and as such, checks that we
have various fixed hard-coded object IDs.  The tests relying on these
assertions have been marked with the SHA1 prerequisite, as they will
obviously not function in their current form with SHA-256.

Use the test_oid helper to update these assertions and provide values
for both SHA-1 and SHA-256.

These object IDs were synthesized using a set of scripts that created
the objects for both SHA-1 and SHA-256 using the same method to ensure
that they are indeed the correct values.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/oid-info/t0000 |  38 ++++++++++++++
 t/t0000-basic.sh | 126 ++++++++++++++++++++++++-----------------------
 2 files changed, 103 insertions(+), 61 deletions(-)
 create mode 100644 t/oid-info/t0000

diff --git a/t/oid-info/t0000 b/t/oid-info/t0000
new file mode 100644
index 0000000000..e62aefba4d
--- /dev/null
+++ b/t/oid-info/t0000
@@ -0,0 +1,38 @@
+path0f sha1:f87290f8eb2cbbea7857214459a0739927eab154
+path0f sha256:638106af7c38be056f3212cbd7ac65bc1bac74f420ca5a436ff006a9d025d17d
+
+path0s sha1:15a98433ae33114b085f3eb3bb03b832b3180a01
+path0s sha256:3a24cc53cf68edddac490bbf94a418a52932130541361f685df685e41dd6c363
+
+path2f sha1:3feff949ed00a62d9f7af97c15cd8a30595e7ac7
+path2f sha256:2a7f36571c6fdbaf0e3f62751a0b25a3f4c54d2d1137b3f4af9cb794bb498e5f
+
+path2s sha1:d8ce161addc5173867a3c3c730924388daedbc38
+path2s sha256:18fd611b787c2e938ddcc248fabe4d66a150f9364763e9ec133dd01d5bb7c65a
+
+path2d sha1:58a09c23e2ca152193f2786e06986b7b6712bdbe
+path2d sha256:00e4b32b96e7e3d65d79112dcbea53238a22715f896933a62b811377e2650c17
+
+path3f sha1:0aa34cae68d0878578ad119c86ca2b5ed5b28376
+path3f sha256:09f58616b951bd571b8cb9dc76d372fbb09ab99db2393f5ab3189d26c45099ad
+
+path3s sha1:8599103969b43aff7e430efea79ca4636466794f
+path3s sha256:fce1aed087c053306f3f74c32c1a838c662bbc4551a7ac2420f5d6eb061374d0
+
+path3d sha1:21ae8269cacbe57ae09138dcc3a2887f904d02b3
+path3d sha256:9b60497be959cb830bf3f0dc82bcc9ad9e925a24e480837ade46b2295e47efe1
+
+subp3f sha1:00fb5908cb97c2564a9783c0c64087333b3b464f
+subp3f sha256:a1a9e16998c988453f18313d10375ee1d0ddefe757e710dcae0d66aa1e0c58b3
+
+subp3s sha1:6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c
+subp3s sha256:81759d9f5e93c6546ecfcadb560c1ff057314b09f93fe8ec06e2d8610d34ef10
+
+subp3d sha1:3c5e5399f3a333eddecce7a9b9465b63f65f51e2
+subp3d sha256:76b4ef482d4fa1c754390344cf3851c7f883b27cf9bc999c6547928c46aeafb7
+
+root sha1:087704a96baf1c2d1c869a8b084481e121c88b5b
+root sha256:9481b52abab1b2ffeedbf9de63ce422b929f179c1b98ff7bee5f8f1bc0710751
+
+simpletree sha1:7bb943559a305bdd6bdee2cef6e5df2413c3d30a
+simpletree sha256:1710c07a6c86f9a3c7376364df04c47ee39e5a5e221fcdd84b743bc9bb7e2bc5
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 57d374f2dc..180576293a 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -853,6 +853,8 @@ test_expect_success 'test_oid_info can look up data for SHA-256' '
 ################################################################
 # Basics of the basics
 
+test_oid_cache t0000
+
 # updating a new file without --add should fail.
 test_expect_success 'git update-index without --add should fail adding' '
 	test_must_fail git update-index should-be-empty
@@ -868,8 +870,9 @@ test_expect_success 'writing tree out with git write-tree' '
 '
 
 # we know the shape and contents of the tree and know the object ID for it.
-test_expect_success SHA1 'validate object ID of a known tree' '
-	test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
+test_expect_success 'validate object ID of a known tree' '
+echo $tree &&
+	test "$tree" = $(test_oid simpletree)
     '
 
 # Removing paths.
@@ -911,17 +914,18 @@ test_expect_success 'showing stage with git ls-files --stage' '
 	git ls-files --stage >current
 '
 
-test_expect_success SHA1 'validate git ls-files output for a known tree' '
-	cat >expected <<-\EOF &&
-	100644 f87290f8eb2cbbea7857214459a0739927eab154 0	path0
-	120000 15a98433ae33114b085f3eb3bb03b832b3180a01 0	path0sym
-	100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0	path2/file2
-	120000 d8ce161addc5173867a3c3c730924388daedbc38 0	path2/file2sym
-	100644 0aa34cae68d0878578ad119c86ca2b5ed5b28376 0	path3/file3
-	120000 8599103969b43aff7e430efea79ca4636466794f 0	path3/file3sym
-	100644 00fb5908cb97c2564a9783c0c64087333b3b464f 0	path3/subp3/file3
-	120000 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c 0	path3/subp3/file3sym
+test_expect_success 'validate git ls-files output for a known tree' '
+	cat >expected <<-EOF &&
+	100644 $(test_oid path0f) 0	path0
+	120000 $(test_oid path0s) 0	path0sym
+	100644 $(test_oid path2f) 0	path2/file2
+	120000 $(test_oid path2s) 0	path2/file2sym
+	100644 $(test_oid path3f) 0	path3/file3
+	120000 $(test_oid path3s) 0	path3/file3sym
+	100644 $(test_oid subp3f) 0	path3/subp3/file3
+	120000 $(test_oid subp3s) 0	path3/subp3/file3sym
 	EOF
+	cat expected &&
 	test_cmp expected current
 '
 
@@ -929,20 +933,20 @@ test_expect_success 'writing tree out with git write-tree' '
 	tree=$(git write-tree)
 '
 
-test_expect_success SHA1 'validate object ID for a known tree' '
-	test "$tree" = 087704a96baf1c2d1c869a8b084481e121c88b5b
+test_expect_success 'validate object ID for a known tree' '
+	test "$tree" = $(test_oid root)
 '
 
 test_expect_success 'showing tree with git ls-tree' '
     git ls-tree $tree >current
 '
 
-test_expect_success SHA1 'git ls-tree output for a known tree' '
-	cat >expected <<-\EOF &&
-	100644 blob f87290f8eb2cbbea7857214459a0739927eab154	path0
-	120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01	path0sym
-	040000 tree 58a09c23e2ca152193f2786e06986b7b6712bdbe	path2
-	040000 tree 21ae8269cacbe57ae09138dcc3a2887f904d02b3	path3
+test_expect_success 'git ls-tree output for a known tree' '
+	cat >expected <<-EOF &&
+	100644 blob $(test_oid path0f)	path0
+	120000 blob $(test_oid path0s)	path0sym
+	040000 tree $(test_oid path2d)	path2
+	040000 tree $(test_oid path3d)	path3
 	EOF
 	test_cmp expected current
 '
@@ -953,16 +957,16 @@ test_expect_success 'showing tree with git ls-tree -r' '
 	git ls-tree -r $tree >current
 '
 
-test_expect_success SHA1 'git ls-tree -r output for a known tree' '
-	cat >expected <<-\EOF &&
-	100644 blob f87290f8eb2cbbea7857214459a0739927eab154	path0
-	120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01	path0sym
-	100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7	path2/file2
-	120000 blob d8ce161addc5173867a3c3c730924388daedbc38	path2/file2sym
-	100644 blob 0aa34cae68d0878578ad119c86ca2b5ed5b28376	path3/file3
-	120000 blob 8599103969b43aff7e430efea79ca4636466794f	path3/file3sym
-	100644 blob 00fb5908cb97c2564a9783c0c64087333b3b464f	path3/subp3/file3
-	120000 blob 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c	path3/subp3/file3sym
+test_expect_success 'git ls-tree -r output for a known tree' '
+	cat >expected <<-EOF &&
+	100644 blob $(test_oid path0f)	path0
+	120000 blob $(test_oid path0s)	path0sym
+	100644 blob $(test_oid path2f)	path2/file2
+	120000 blob $(test_oid path2s)	path2/file2sym
+	100644 blob $(test_oid path3f)	path3/file3
+	120000 blob $(test_oid path3s)	path3/file3sym
+	100644 blob $(test_oid subp3f)	path3/subp3/file3
+	120000 blob $(test_oid subp3s)	path3/subp3/file3sym
 	EOF
 	test_cmp expected current
 '
@@ -972,19 +976,19 @@ test_expect_success 'showing tree with git ls-tree -r -t' '
 	git ls-tree -r -t $tree >current
 '
 
-test_expect_success SHA1 'git ls-tree -r output for a known tree' '
-	cat >expected <<-\EOF &&
-	100644 blob f87290f8eb2cbbea7857214459a0739927eab154	path0
-	120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01	path0sym
-	040000 tree 58a09c23e2ca152193f2786e06986b7b6712bdbe	path2
-	100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7	path2/file2
-	120000 blob d8ce161addc5173867a3c3c730924388daedbc38	path2/file2sym
-	040000 tree 21ae8269cacbe57ae09138dcc3a2887f904d02b3	path3
-	100644 blob 0aa34cae68d0878578ad119c86ca2b5ed5b28376	path3/file3
-	120000 blob 8599103969b43aff7e430efea79ca4636466794f	path3/file3sym
-	040000 tree 3c5e5399f3a333eddecce7a9b9465b63f65f51e2	path3/subp3
-	100644 blob 00fb5908cb97c2564a9783c0c64087333b3b464f	path3/subp3/file3
-	120000 blob 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c	path3/subp3/file3sym
+test_expect_success 'git ls-tree -r output for a known tree' '
+	cat >expected <<-EOF &&
+	100644 blob $(test_oid path0f)	path0
+	120000 blob $(test_oid path0s)	path0sym
+	040000 tree $(test_oid path2d)	path2
+	100644 blob $(test_oid path2f)	path2/file2
+	120000 blob $(test_oid path2s)	path2/file2sym
+	040000 tree $(test_oid path3d)	path3
+	100644 blob $(test_oid path3f)	path3/file3
+	120000 blob $(test_oid path3s)	path3/file3sym
+	040000 tree $(test_oid subp3d)	path3/subp3
+	100644 blob $(test_oid subp3f)	path3/subp3/file3
+	120000 blob $(test_oid subp3s)	path3/subp3/file3sym
 	EOF
 	test_cmp expected current
 '
@@ -993,16 +997,16 @@ test_expect_success 'writing partial tree out with git write-tree --prefix' '
 	ptree=$(git write-tree --prefix=path3)
 '
 
-test_expect_success SHA1 'validate object ID for a known tree' '
-	test "$ptree" = 21ae8269cacbe57ae09138dcc3a2887f904d02b3
+test_expect_success 'validate object ID for a known tree' '
+	test "$ptree" = $(test_oid path3d)
 '
 
 test_expect_success 'writing partial tree out with git write-tree --prefix' '
 	ptree=$(git write-tree --prefix=path3/subp3)
 '
 
-test_expect_success SHA1 'validate object ID for a known tree' '
-	test "$ptree" = 3c5e5399f3a333eddecce7a9b9465b63f65f51e2
+test_expect_success 'validate object ID for a known tree' '
+	test "$ptree" = $(test_oid subp3d)
 '
 
 test_expect_success 'put invalid objects into the index' '
@@ -1036,16 +1040,16 @@ test_expect_success 'git read-tree followed by write-tree should be idempotent'
 	test "$newtree" = "$tree"
 '
 
-test_expect_success SHA1 'validate git diff-files output for a know cache/work tree state' '
-	cat >expected <<\EOF &&
-:100644 100644 f87290f8eb2cbbea7857214459a0739927eab154 0000000000000000000000000000000000000000 M	path0
-:120000 120000 15a98433ae33114b085f3eb3bb03b832b3180a01 0000000000000000000000000000000000000000 M	path0sym
-:100644 100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0000000000000000000000000000000000000000 M	path2/file2
-:120000 120000 d8ce161addc5173867a3c3c730924388daedbc38 0000000000000000000000000000000000000000 M	path2/file2sym
-:100644 100644 0aa34cae68d0878578ad119c86ca2b5ed5b28376 0000000000000000000000000000000000000000 M	path3/file3
-:120000 120000 8599103969b43aff7e430efea79ca4636466794f 0000000000000000000000000000000000000000 M	path3/file3sym
-:100644 100644 00fb5908cb97c2564a9783c0c64087333b3b464f 0000000000000000000000000000000000000000 M	path3/subp3/file3
-:120000 120000 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c 0000000000000000000000000000000000000000 M	path3/subp3/file3sym
+test_expect_success 'validate git diff-files output for a know cache/work tree state' '
+	cat >expected <<EOF &&
+:100644 100644 $(test_oid path0f) $ZERO_OID M	path0
+:120000 120000 $(test_oid path0s) $ZERO_OID M	path0sym
+:100644 100644 $(test_oid path2f) $ZERO_OID M	path2/file2
+:120000 120000 $(test_oid path2s) $ZERO_OID M	path2/file2sym
+:100644 100644 $(test_oid path3f) $ZERO_OID M	path3/file3
+:120000 120000 $(test_oid path3s) $ZERO_OID M	path3/file3sym
+:100644 100644 $(test_oid subp3f) $ZERO_OID M	path3/subp3/file3
+:120000 120000 $(test_oid subp3s) $ZERO_OID M	path3/subp3/file3sym
 EOF
 	git diff-files >current &&
 	test_cmp current expected
@@ -1061,23 +1065,23 @@ test_expect_success 'no diff after checkout and git update-index --refresh' '
 '
 
 ################################################################
-P=087704a96baf1c2d1c869a8b084481e121c88b5b
+P=$(test_oid root)
 
-test_expect_success SHA1 'git commit-tree records the correct tree in a commit' '
+test_expect_success 'git commit-tree records the correct tree in a commit' '
 	commit0=$(echo NO | git commit-tree $P) &&
 	tree=$(git show --pretty=raw $commit0 |
 		 sed -n -e "s/^tree //p" -e "/^author /q") &&
 	test "z$tree" = "z$P"
 '
 
-test_expect_success SHA1 'git commit-tree records the correct parent in a commit' '
+test_expect_success 'git commit-tree records the correct parent in a commit' '
 	commit1=$(echo NO | git commit-tree $P -p $commit0) &&
 	parent=$(git show --pretty=raw $commit1 |
 		sed -n -e "s/^parent //p" -e "/^author /q") &&
 	test "z$commit0" = "z$parent"
 '
 
-test_expect_success SHA1 'git commit-tree omits duplicated parent in a commit' '
+test_expect_success 'git commit-tree omits duplicated parent in a commit' '
 	commit2=$(echo NO | git commit-tree $P -p $commit0 -p $commit0) &&
 	     parent=$(git show --pretty=raw $commit2 |
 		sed -n -e "s/^parent //p" -e "/^author /q" |

[PATCH v2 04/11] t0002: abstract away SHA-1 specific constants

[brian m. carlson]

Adjust the test so that it computes variables for object IDs instead of
using hard-coded hashes.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0002-gitfile.sh | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 3691023d51..aa17023774 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -92,11 +92,12 @@ test_expect_success 'enter_repo non-strict mode' '
 		mv .git .realgit &&
 		echo "gitdir: .realgit" >.git
 	) &&
+	head=$(git -C enter_repo rev-parse HEAD) &&
 	git ls-remote enter_repo >actual &&
-	cat >expected <<-\EOF &&
-	946e985ab20de757ca5b872b16d64e92ff3803a9	HEAD
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/heads/master
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/tags/foo
+	cat >expected <<-EOF &&
+	$head	HEAD
+	$head	refs/heads/master
+	$head	refs/tags/foo
 	EOF
 	test_cmp expected actual
 '
@@ -107,20 +108,20 @@ test_expect_success 'enter_repo linked checkout' '
 		git worktree add  ../foo refs/tags/foo
 	) &&
 	git ls-remote foo >actual &&
-	cat >expected <<-\EOF &&
-	946e985ab20de757ca5b872b16d64e92ff3803a9	HEAD
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/heads/master
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/tags/foo
+	cat >expected <<-EOF &&
+	$head	HEAD
+	$head	refs/heads/master
+	$head	refs/tags/foo
 	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'enter_repo strict mode' '
 	git ls-remote --upload-pack="git upload-pack --strict" foo/.git >actual &&
-	cat >expected <<-\EOF &&
-	946e985ab20de757ca5b872b16d64e92ff3803a9	HEAD
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/heads/master
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/tags/foo
+	cat >expected <<-EOF &&
+	$head	HEAD
+	$head	refs/heads/master
+	$head	refs/tags/foo
 	EOF
 	test_cmp expected actual
 '

[PATCH v2 05/11] t0027: make hash size independent

[brian m. carlson]

We transform various object IDs into all-zero object IDs for comparison.
Adjust the length as well so that this works for all hash algorithms.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0027-auto-crlf.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index beb5927f77..e748060489 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -15,8 +15,9 @@ compare_ws_file () {
 	pfx=$1
 	exp=$2.expect
 	act=$pfx.actual.$3
-	tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp" &&
+	tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp+" &&
 	tr '\015\000abcdef0123456789' QN00000000000000000 <"$3" >"$act" &&
+	sed -e "s/0000+/$ZERO_OID/" "$exp+" >"$exp" &&
 	test_cmp "$exp" "$act" &&
 	rm "$exp" "$act"
 }

[PATCH v2 06/11] t0064: make hash size independent

[brian m. carlson]

Compute test values of the appropriate size instead of hard-coding
40-character values.  Rename the echo20 function to echoid, since the
values may be of varying sizes.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0064-sha1-array.sh | 49 ++++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/t/t0064-sha1-array.sh b/t/t0064-sha1-array.sh
index 67484502a0..5dda570b9a 100755
--- a/t/t0064-sha1-array.sh
+++ b/t/t0064-sha1-array.sh
@@ -3,30 +3,30 @@
 test_description='basic tests for the SHA1 array implementation'
 . ./test-lib.sh
 
-echo20 () {
+echoid () {
 	prefix="${1:+$1 }"
 	shift
 	while test $# -gt 0
 	do
-		echo "$prefix$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1"
+		echo "$prefix$ZERO_OID" | sed -e "s/00/$1/g"
 		shift
 	done
 }
 
 test_expect_success 'ordered enumeration' '
-	echo20 "" 44 55 88 aa >expect &&
+	echoid "" 44 55 88 aa >expect &&
 	{
-		echo20 append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
 		echo for_each_unique
 	} | test-tool sha1-array >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'ordered enumeration with duplicate suppression' '
-	echo20 "" 44 55 88 aa >expect &&
+	echoid "" 44 55 88 aa >expect &&
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
 		echo for_each_unique
 	} | test-tool sha1-array >actual &&
 	test_cmp expect actual
@@ -34,8 +34,8 @@ test_expect_success 'ordered enumeration with duplicate suppression' '
 
 test_expect_success 'lookup' '
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 lookup 55
+		echoid append 88 44 aa 55 &&
+		echoid lookup 55
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -eq 1
@@ -43,8 +43,8 @@ test_expect_success 'lookup' '
 
 test_expect_success 'lookup non-existing entry' '
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 lookup 33
+		echoid append 88 44 aa 55 &&
+		echoid lookup 33
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -lt 0
@@ -52,9 +52,9 @@ test_expect_success 'lookup non-existing entry' '
 
 test_expect_success 'lookup with duplicates' '
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 append 88 44 aa 55 &&
-		echo20 lookup 55
+		echoid append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
+		echoid lookup 55
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -ge 2 &&
@@ -63,19 +63,24 @@ test_expect_success 'lookup with duplicates' '
 
 test_expect_success 'lookup non-existing entry with duplicates' '
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 append 88 44 aa 55 &&
-		echo20 lookup 66
+		echoid append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
+		echoid lookup 66
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -lt 0
 '
 
 test_expect_success 'lookup with almost duplicate values' '
+	# n-1 5s
+	root=$(echoid "" 55) &&
+	root=${root%5} &&
 	{
-		echo "append 5555555555555555555555555555555555555555" &&
-		echo "append 555555555555555555555555555555555555555f" &&
-		echo20 lookup 55
+		id1="${root}5" &&
+		id2="${root}f" &&
+		echo "append $id1" &&
+		echo "append $id2" &&
+		echoid lookup 55
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -eq 0
@@ -83,8 +88,8 @@ test_expect_success 'lookup with almost duplicate values' '
 
 test_expect_success 'lookup with single duplicate value' '
 	{
-		echo20 append 55 55 &&
-		echo20 lookup 55
+		echoid append 55 55 &&
+		echoid lookup 55
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -ge 0 &&

[PATCH v2 07/11] t1006: make hash size independent

[brian m. carlson]

Compute the size of the tree and commit objects we're creating by
checking for the size of an object ID and computing the resulting sizes
accordingly.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1006-cat-file.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 13dd510b2e..345ed70e53 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -140,15 +140,17 @@ test_expect_success '--batch-check without %(rest) considers whole line' '
 	test_cmp expect actual
 '
 
+test_oid_cache hash-info
+
 tree_sha1=$(git write-tree)
-tree_size=33
+tree_size=$(($(test_oid rawsz) + 13))
 tree_pretty_content="100644 blob $hello_sha1	hello"
 
 run_tests 'tree' $tree_sha1 $tree_size "" "$tree_pretty_content"
 
 commit_message="Initial commit"
 commit_sha1=$(echo_without_newline "$commit_message" | git commit-tree $tree_sha1)
-commit_size=177
+commit_size=$(($(test_oid hexsz) + 137))
 commit_content="tree $tree_sha1
 author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 0000000000 +0000
 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 0000000000 +0000

[PATCH v2 08/11] t1400: switch hard-coded object ID to variable

[brian m. carlson]

Switch a hard-coded all-zeros object ID to use a variable instead.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1400-update-ref.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7c8df20955..6072650686 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -346,7 +346,7 @@ test_expect_success "verifying $m's log (logged by config)" '
 
 git update-ref $m $D
 cat >.git/logs/$m <<EOF
-0000000000000000000000000000000000000000 $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500
+$Z $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500
 $C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500
 $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500
 $F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500

[PATCH v2 09/11] t1405: make hash size independent

[brian m. carlson]

Instead of hard-coding a 40-based constant, split the output of
for-each-ref and for-each-reflog by field.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1405-main-ref-store.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index a74c38b5fb..331899ddc4 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -54,7 +54,7 @@ test_expect_success 'for_each_ref(refs/heads/)' '
 '
 
 test_expect_success 'for_each_ref() is sorted' '
-	$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+	$RUN for-each-ref refs/heads/ | cut -d" " -f 2- >actual &&
 	sort actual > expected &&
 	test_cmp expected actual
 '
@@ -71,7 +71,7 @@ test_expect_success 'verify_ref(new-master)' '
 '
 
 test_expect_success 'for_each_reflog()' '
-	$RUN for-each-reflog | sort -k2 | cut -c 42- >actual &&
+	$RUN for-each-reflog | sort -k2 | cut -d" " -f 2- >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	refs/heads/master 0x0

[PATCH v2 10/11] t1406: make hash-size independent

[brian m. carlson]

Instead of hard-coding a 40-based constant, split the output of
for-each-ref and for-each-reflog by field.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1406-submodule-ref-store.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
index e093782cc3..d199d872fb 100755
--- a/t/t1406-submodule-ref-store.sh
+++ b/t/t1406-submodule-ref-store.sh
@@ -39,7 +39,7 @@ test_expect_success 'rename_refs() not allowed' '
 '
 
 test_expect_success 'for_each_ref(refs/heads/)' '
-	$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+	$RUN for-each-ref refs/heads/ | cut -d" " -f 2- >actual &&
 	cat >expected <<-\EOF &&
 	master 0x0
 	new-master 0x0
@@ -48,7 +48,7 @@ test_expect_success 'for_each_ref(refs/heads/)' '
 '
 
 test_expect_success 'for_each_ref() is sorted' '
-	$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+	$RUN for-each-ref refs/heads/ | cut -d" " -f 2- >actual &&
 	sort actual > expected &&
 	test_cmp expected actual
 '
@@ -65,7 +65,7 @@ test_expect_success 'verify_ref(new-master)' '
 '
 
 test_expect_success 'for_each_reflog()' '
-	$RUN for-each-reflog | sort | cut -c 42- >actual &&
+	$RUN for-each-reflog | sort | cut -d" " -f 2- >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	refs/heads/master 0x0

[PATCH v2 11/11] t1407: make hash size independent

[brian m. carlson]

Instead of hard-coding a 40-based constant, split the output of
for-each-ref and for-each-reflog by field.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1407-worktree-ref-store.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index 4623ae15c4..9a84858118 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -58,7 +58,7 @@ test_expect_success 'for_each_reflog()' '
 	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
 	echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
 
-	$RWT for-each-reflog | cut -c 42- | sort >actual &&
+	$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	PSEUDO-WT 0x0
@@ -68,7 +68,7 @@ test_expect_success 'for_each_reflog()' '
 	EOF
 	test_cmp expected actual &&
 
-	$RMAIN for-each-reflog | cut -c 42- | sort >actual &&
+	$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	PSEUDO-MAIN 0x0

Re: [PATCH v2 01/11] t: add tool to translate hash-related values

[Eric Sunshine]

On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> diff --git a/t/oid-info/oid b/t/oid-info/oid
> @@ -0,0 +1,29 @@
> +# All zeros or Fs missing one or two hex segments.
> +zero_1         sha1:000000000000000000000000000000000000000
> +zero_2         sha256:000000000000000000000000000000000000000000000000000000000000000
> +zero_2         sha1:00000000000000000000000000000000000000
> +zero_2         sha256:00000000000000000000000000000000000000000000000000000000000000

Too many zero_2's. I guess the first one should be zero_1.

> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> @@ -821,6 +821,35 @@ test_expect_success 'tests clean up even on failures' "
> +test_oid_cache hash-info oid
> +
> +test_expect_success 'test_oid_info provides sane info by default' '

Is "test_oid_info" in the test title outdated? I don't see anything by
that name. Same question regarding the other new tests.

> +       test_oid zero &&
> +       test_oid zero >actual &&

If the lookup "test_oid zero" fails, it's going to fail whether
redirected or not, so the first invocation can be dropped.

> +       grep "00*" actual &&

Do you want to anchor this regex? ^00*$

Same comment regarding the other new tests.

> +       test "$(test_oid hexsz)" -eq $(wc -c <actual) &&
> +       test $(( $(test_oid rawsz) * 2)) -eq "$(test_oid hexsz)"
> +'

If $(test_oid rawsz) fails to find "rawsz" and returns nothing, this
expression will be "*2", which will error out in a
less-than-meaningful way. Perhaps it would make more sense to dump the
results of the two test_oid() to file and use test_cmp()?

Similar comment regarding all the other "test ... -eq ..." expressions
here and below: I wonder if it would be better to dump them to files
and compare the files.

> +test_expect_success 'test_oid_info can look up data for SHA-1' '
> +       test_detect_hash sha1 &&
> +       test_oid zero >actual &&
> +       grep "00*" actual &&
> +       test $(wc -c <actual) -eq 40 &&
> +       test "$(test_oid rawsz)" -eq 20 &&
> +       test "$(test_oid hexsz)" -eq 40
> +'
> +
> +test_expect_success 'test_oid_info can look up data for SHA-256' '
> +       test_when_finished "test_detect_hash" &&

Should the previous test also do this test_when_finished() to protect
against someone coming along and re-ordering them some day? Or someone
inserting a test between the two?

> +       test_detect_hash sha256 &&
> +       test_oid zero >actual &&
> +       grep "00*" actual &&
> +       test $(wc -c <actual) -eq 64 &&
> +       test "$(test_oid rawsz)" -eq 32 &&
> +       test "$(test_oid hexsz)" -eq 64
> +'
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -1147,3 +1147,39 @@ depacketize () {
> +test_detect_hash () {
> +       if test -n "$1"
> +       then
> +               test_hash_algo="$1"
> +       else
> +               test_hash_algo='sha1'
> +       fi
> +}

Mmph. Outside of t0000, do you expect other callers which will need to
set the hash algorithm to an explicit value? If not, this sort of
polymorphic behavior adds extra, perhaps unnecessary, complexity. Even
if you do expect a few other callers, a dedicated test_set_hash()
function might be clearer since test_detect_hash() has a very specific
meaning.

> +test_oid_cache () {
> +       test -n "$test_hash_algo" || test_detect_hash
> +       if test -n "$1"
> +       then
> +               while test -n "$1"
> +               do
> +                       test_oid_cache <"$TEST_DIRECTORY/oid-info/$1"

What is the benefit of placing test-specific OID info in some
dedicated directory? I suppose the idea of lumping all these OID files
together in a single oid-info/ directory is that it makes it easier to
see at a glance what needs to be changed next time a new hash
algorithm is selected. However, that potential long-term benefit comes
at the cost of divorcing test-specific information from the tests
themselves. I imagine (for myself, at least) that it would be easier
to grok a test if its OID's were specified and cached directly in the
test script itself (via test_oid_cache <<here-doc). I could even
imagine a test script invoking test_oid_cache<<here-doc before each
test which has unique OID's in order to localize OID information to
the test which needs it, or perhaps even within a test.

So, I'm having trouble understanding the benefit of the current
approach over merely loading OID's via here-docs in the test scripts
themselves. Perhaps the commit message could say something about why
the current approach was taken.

> +                       shift
> +               done
> +               return $?

Why, specifically, return $? here, as opposed to simply returning?

> +       fi

Mmph. This polymorphic, recursive behavior in which test_oid_cache()
can load data from a list of files or from its own standard input adds
complexity. One alternative would be to have a separate
test_oid_cache_file() function. However, I'm wondering why such
functionality needs to be built in, in the first place, as opposed to
clients merely doing the redirect themselves (test_oid_cache
<whatever). I see that you want to support specifying multiple files
for a single invocation, but is that likely to come up (aside from
t0000)?

> +       while read _tag _rest
> +       do
> +               case $_tag in \#*)
> +                       continue;;
> +               esac

This handles "# comment" lines, but what about blank lines?

> +               _k="${_rest%:*}"
> +               _v="${_rest#*:}"

Should this be doing any sort of validation, such as complaining if _k
or _v is empty? Or if _k contains weird characters?

> +               eval "test_oid_${_k}_$_tag=\"$_v\""
> +       done
> +}
> +
> +test_oid () {
> +       eval "printf '%s' \"\${test_oid_${test_hash_algo}_$1}\""
> +}

Re: [PATCH v2 02/11] t0000: use hash translation table

[Eric Sunshine]

On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> If the hash we're using is 32 bytes in size, attempting to insert a
> 20-byte object name won't work.  Since these are synthesized objects
> that are almost all zeros, look them up in a translation table.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> @@ -1007,12 +1007,13 @@ test_expect_success SHA1 'validate object ID for a known tree' '
>  test_expect_success 'put invalid objects into the index' '
>         rm -f .git/index &&
> -       [...]
> +       suffix=$(echo $ZERO_OID | sed -e "s/^.//") &&

What's this "suffix" thing for? I don't see it used anywhere.

> +       cat >badobjects <<-EOF &&
> +       100644 blob $(test_oid 001)     dir/file1
> +       100644 blob $(test_oid 002)     dir/file2
> +       100644 blob $(test_oid 003)     dir/file3
> +       100644 blob $(test_oid 004)     dir/file4
> +       100644 blob $(test_oid 005)     dir/file5

So, test_oid() knows about these keys because the patch 1/11 loaded
them via test_oid_cache(). Are the keys in oid-info/hash-info and
oid-info/oid going to be needed by multiple scripts? If so, I'm
wondering if it would make more sense to have test-lib-functions.sh
load them instead of expecting each script to do so. Another
possibility is to have a test_oid_init() function in
test-lib-functions.sh which both loads that low-level information and
initializes the hash algorithm.

>         EOF
>         git update-index --index-info <badobjects
>  '

Re: [PATCH v2 03/11] t0000: update tests for SHA-256

[Eric Sunshine]

On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> Test t0000 tests the "basics of the basics" and as such, checks that we
> have various fixed hard-coded object IDs.  The tests relying on these
> assertions have been marked with the SHA1 prerequisite, as they will
> obviously not function in their current form with SHA-256.
>
> Use the test_oid helper to update these assertions and provide values
> for both SHA-1 and SHA-256.
> [...]
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> @@ -868,8 +870,9 @@ test_expect_success 'writing tree out with git write-tree' '
> +test_expect_success 'validate object ID of a known tree' '
> +echo $tree &&

Debugging gunk?

> +       test "$tree" = $(test_oid simpletree)

If test_oid() fails to find key "simpletree", this expression will be
invalid. Therefore, it probably would be a good idea to quote the
$(test_oid ...) invocation.

> @@ -911,17 +914,18 @@ test_expect_success 'showing stage with git ls-files --stage' '
> +test_expect_success 'validate git ls-files output for a known tree' '
> +       cat >expected <<-EOF &&
> +       100644 $(test_oid path0f) 0     path0
> +       120000 $(test_oid path0s) 0     path0sym
> +       100644 $(test_oid path2f) 0     path2/file2
> +       120000 $(test_oid path2s) 0     path2/file2sym
> +       100644 $(test_oid path3f) 0     path3/file3
> +       120000 $(test_oid path3s) 0     path3/file3sym
> +       100644 $(test_oid subp3f) 0     path3/subp3/file3
> +       120000 $(test_oid subp3s) 0     path3/subp3/file3sym
>         EOF
> +       cat expected &&

Debugging gunk?

>         test_cmp expected current
>  '

Re: [PATCH v2 04/11] t0002: abstract away SHA-1 specific constants

[Eric Sunshine]

On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> Adjust the test so that it computes variables for object IDs instead of
> using hard-coded hashes.

Nit: s/hashes/hash values/

> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
> @@ -92,11 +92,12 @@ test_expect_success 'enter_repo non-strict mode' '
>         ) &&
> +       head=$(git -C enter_repo rev-parse HEAD) &&
>         [...]
> +       cat >expected <<-EOF &&
> +       $head   HEAD
> +       $head   refs/heads/master
> +       $head   refs/tags/foo
>         EOF
>         test_cmp expected actual
>  '

Okay, but...

> @@ -107,20 +108,20 @@ test_expect_success 'enter_repo linked checkout' '
> -       [...]
> +       cat >expected <<-EOF &&
> +       $head   HEAD
> +       $head   refs/heads/master
> +       $head   refs/tags/foo
>         EOF
>         test_cmp expected actual
>  '

This is relying upon 'head' set inside an earlier test, which seems
fragile. More robust would be to compute 'head' anew within each test
which needs it (including the other new test added by this patch).

Re: [PATCH v2 05/11] t0027: make hash size independent

[Eric Sunshine]

On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> We transform various object IDs into all-zero object IDs for comparison.
> Adjust the length as well so that this works for all hash algorithms.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> @@ -15,8 +15,9 @@ compare_ws_file () {
> -       tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp" &&
> +       tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp+" &&

My immediate thought upon reading this was whether "+" is valid in
Windows filenames. Apparently, it is, but perhaps (if you re-roll) it
would make sense to use a character less likely to cause brain
hiccups; for instance, "exp0'.

>         tr '\015\000abcdef0123456789' QN00000000000000000 <"$3" >"$act" &&
> +       sed -e "s/0000+/$ZERO_OID/" "$exp+" >"$exp" &&

Re: [PATCH v2 01/11] t: add tool to translate hash-related values

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 8110 bytes --]

On Sun, Aug 19, 2018 at 03:40:22PM -0400, Eric Sunshine wrote:
> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > diff --git a/t/oid-info/oid b/t/oid-info/oid
> > @@ -0,0 +1,29 @@
> > +# All zeros or Fs missing one or two hex segments.
> > +zero_1         sha1:000000000000000000000000000000000000000
> > +zero_2         sha256:000000000000000000000000000000000000000000000000000000000000000
> > +zero_2         sha1:00000000000000000000000000000000000000
> > +zero_2         sha256:00000000000000000000000000000000000000000000000000000000000000
> 
> Too many zero_2's. I guess the first one should be zero_1.

Ah, yes, you're right.

> > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> > @@ -821,6 +821,35 @@ test_expect_success 'tests clean up even on failures' "
> > +test_oid_cache hash-info oid
> > +
> > +test_expect_success 'test_oid_info provides sane info by default' '
> 
> Is "test_oid_info" in the test title outdated? I don't see anything by
> that name. Same question regarding the other new tests.

Probably.

> > +       test_oid zero &&
> > +       test_oid zero >actual &&
> 
> If the lookup "test_oid zero" fails, it's going to fail whether
> redirected or not, so the first invocation can be dropped.

Right.  I think that might have been a debugging statement.  Will fix.

> > +       grep "00*" actual &&
> 
> Do you want to anchor this regex? ^00*$

I could.  That would probably be a bit more robust.

> > +       test "$(test_oid hexsz)" -eq $(wc -c <actual) &&
> > +       test $(( $(test_oid rawsz) * 2)) -eq "$(test_oid hexsz)"
> > +'
> 
> If $(test_oid rawsz) fails to find "rawsz" and returns nothing, this
> expression will be "*2", which will error out in a
> less-than-meaningful way. Perhaps it would make more sense to dump the
> results of the two test_oid() to file and use test_cmp()?
> 
> Similar comment regarding all the other "test ... -eq ..." expressions
> here and below: I wonder if it would be better to dump them to files
> and compare the files.

I think what I'd like to do instead is store in a variable and make
test_oid return unsuccessfully if the value doesn't exist.  I think
that's better for writing tests overall and will accomplish the same
goal.

> > +test_expect_success 'test_oid_info can look up data for SHA-1' '
> > +       test_detect_hash sha1 &&
> > +       test_oid zero >actual &&
> > +       grep "00*" actual &&
> > +       test $(wc -c <actual) -eq 40 &&
> > +       test "$(test_oid rawsz)" -eq 20 &&
> > +       test "$(test_oid hexsz)" -eq 40
> > +'
> > +
> > +test_expect_success 'test_oid_info can look up data for SHA-256' '
> > +       test_when_finished "test_detect_hash" &&
> 
> Should the previous test also do this test_when_finished() to protect
> against someone coming along and re-ordering them some day? Or someone
> inserting a test between the two?

Yeah, that sounds like a more robust solution.

> > +       test_detect_hash sha256 &&
> > +       test_oid zero >actual &&
> > +       grep "00*" actual &&
> > +       test $(wc -c <actual) -eq 64 &&
> > +       test "$(test_oid rawsz)" -eq 32 &&
> > +       test "$(test_oid hexsz)" -eq 64
> > +'
> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> > @@ -1147,3 +1147,39 @@ depacketize () {
> > +test_detect_hash () {
> > +       if test -n "$1"
> > +       then
> > +               test_hash_algo="$1"
> > +       else
> > +               test_hash_algo='sha1'
> > +       fi
> > +}
> 
> Mmph. Outside of t0000, do you expect other callers which will need to
> set the hash algorithm to an explicit value? If not, this sort of
> polymorphic behavior adds extra, perhaps unnecessary, complexity. Even
> if you do expect a few other callers, a dedicated test_set_hash()
> function might be clearer since test_detect_hash() has a very specific
> meaning.

I'm not anticipating one.  I can split it out into a separate function.

> > +test_oid_cache () {
> > +       test -n "$test_hash_algo" || test_detect_hash
> > +       if test -n "$1"
> > +       then
> > +               while test -n "$1"
> > +               do
> > +                       test_oid_cache <"$TEST_DIRECTORY/oid-info/$1"
> 
> What is the benefit of placing test-specific OID info in some
> dedicated directory? I suppose the idea of lumping all these OID files
> together in a single oid-info/ directory is that it makes it easier to
> see at a glance what needs to be changed next time a new hash
> algorithm is selected. However, that potential long-term benefit comes
> at the cost of divorcing test-specific information from the tests
> themselves. I imagine (for myself, at least) that it would be easier
> to grok a test if its OID's were specified and cached directly in the
> test script itself (via test_oid_cache <<here-doc). I could even
> imagine a test script invoking test_oid_cache<<here-doc before each
> test which has unique OID's in order to localize OID information to
> the test which needs it, or perhaps even within a test.

Putting them in a separate directory makes it possible to do something
like this:

   (cd t && ./t0002-* --verbose)

and then use shell editing to change the command line.  If we put them
in the same directory as the tests, we make developers' lives a bit
harder.

You mentioned the desire to experiment with additional hash algorithms
as a hope for this series.  I don't know if that's still something
desirable to have, now that we've decided on SHA-256, but I felt it
would make it easier to find all the places that need updating.

Since you seem to have a strong preference about keeping them in the
test script, I'm happy to make that change unless other people have
strong feelings one way or the other.

> So, I'm having trouble understanding the benefit of the current
> approach over merely loading OID's via here-docs in the test scripts
> themselves. Perhaps the commit message could say something about why
> the current approach was taken.

I can do that.  The idea is that we have lots of common uses of certain
values that will need to be loaded, and it's better to load some of
those values from a file.  I felt it would be ugly to have to write out
the full "$TEST_DIRECTORY..." piece every time.

> > +                       shift
> > +               done
> > +               return $?
> 
> Why, specifically, return $? here, as opposed to simply returning?

A simple return is probably better.  I think I wasn't clear on whether
POSIX required a bare "return" to return $? and may not have been online
at that time to look.  I remember being very clear that I didn't want to
return 0 unconditionally, though.

> Mmph. This polymorphic, recursive behavior in which test_oid_cache()
> can load data from a list of files or from its own standard input adds
> complexity. One alternative would be to have a separate
> test_oid_cache_file() function. However, I'm wondering why such
> functionality needs to be built in, in the first place, as opposed to
> clients merely doing the redirect themselves (test_oid_cache
> <whatever). I see that you want to support specifying multiple files
> for a single invocation, but is that likely to come up (aside from
> t0000)?

I expect that we are going to have a lot of uses of the hash-info and
oid files.  A separate command should be fine.

> > +       while read _tag _rest
> > +       do
> > +               case $_tag in \#*)
> > +                       continue;;
> > +               esac
> 
> This handles "# comment" lines, but what about blank lines?

Good catch.

> > +               _k="${_rest%:*}"
> > +               _v="${_rest#*:}"
> 
> Should this be doing any sort of validation, such as complaining if _k
> or _v is empty? Or if _k contains weird characters?

_v could be validly empty in some cases.  _k is probably worth checking.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

Re: [PATCH v2 03/11] t0000: update tests for SHA-256

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 711 bytes --]

On Sun, Aug 19, 2018 at 04:01:05PM -0400, Eric Sunshine wrote:
> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
> > diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> > @@ -868,8 +870,9 @@ test_expect_success 'writing tree out with git write-tree' '
> > +test_expect_success 'validate object ID of a known tree' '
> > +echo $tree &&
> 
> Debugging gunk?

Yup.  Will fix both.

> > +       test "$tree" = $(test_oid simpletree)
> 
> If test_oid() fails to find key "simpletree", this expression will be
> invalid. Therefore, it probably would be a good idea to quote the
> $(test_oid ...) invocation.

Good idea.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

Re: [PATCH v2 05/11] t0027: make hash size independent'

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:
> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > We transform various object IDs into all-zero object IDs for comparison.
> > Adjust the length as well so that this works for all hash algorithms.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> > diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> > @@ -15,8 +15,9 @@ compare_ws_file () {
> > -       tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp" &&
> > +       tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp+" &&
> 
> My immediate thought upon reading this was whether "+" is valid in
> Windows filenames. Apparently, it is, but perhaps (if you re-roll) it
> would make sense to use a character less likely to cause brain
> hiccups; for instance, "exp0'.

The reason I picked that is that we use it quite a bit in the Makefile,
so it seemed like an obvious choice for a temporary file name.  If you
feel strongly I can pick something else, but I thought it would be
reasonably intuitive for other developers.  Maybe I was wrong, though.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

Re: [PATCH v2 05/11] t0027: make hash size independent'

[Eric Sunshine]

On Sun, Aug 19, 2018 at 5:57 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:
> > On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
> > > -       tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp" &&
> > > +       tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp+" &&
> >
> > My immediate thought upon reading this was whether "+" is valid in
> > Windows filenames. Apparently, it is, but perhaps (if you re-roll) it
> > would make sense to use a character less likely to cause brain
> > hiccups; for instance, "exp0'.
>
> The reason I picked that is that we use it quite a bit in the Makefile,
> so it seemed like an obvious choice for a temporary file name.  If you
> feel strongly I can pick something else, but I thought it would be
> reasonably intuitive for other developers.  Maybe I was wrong, though.

I don't feel strongly about it. My brain tripped over it probably
because it's not an idiom in Git tests. In fact, I see just one place
where "+" has been used like this, in t6026.

Re: [PATCH v2 01/11] t: add tool to translate hash-related values

[Eric Sunshine]

On Sun, Aug 19, 2018 at 5:50 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Sun, Aug 19, 2018 at 03:40:22PM -0400, Eric Sunshine wrote:
> > On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
> > <sandals@crustytoothpaste.net> wrote:
> > > +       test "$(test_oid hexsz)" -eq $(wc -c <actual) &&
> > > +       test $(( $(test_oid rawsz) * 2)) -eq "$(test_oid hexsz)"
> > > +'
> >
> > If $(test_oid rawsz) fails to find "rawsz" and returns nothing, this
> > expression will be "*2", which will error out in a
> > less-than-meaningful way. Perhaps it would make more sense to dump the
> > results of the two test_oid() to file and use test_cmp()?
>
> I think what I'd like to do instead is store in a variable and make
> test_oid return unsuccessfully if the value doesn't exist.  I think
> that's better for writing tests overall and will accomplish the same
> goal.

My knee-jerk reaction is that could get clunky in practice, but
perhaps I'm not seeing the full picture.

An alternative would be to return a distinguished error value.

> > > + test_oid_cache <"$TEST_DIRECTORY/oid-info/$1"
> >
> > What is the benefit of placing test-specific OID info in some
> > dedicated directory? I suppose the idea of lumping all these OID files
> > together in a single oid-info/ directory is that it makes it easier to
> > see at a glance what needs to be changed next time a new hash
> > algorithm is selected. However, that potential long-term benefit comes
> > at the cost of divorcing test-specific information from the tests
> > themselves. I imagine (for myself, at least) that it would be easier
> > to grok a test if its OID's were specified and cached directly in the
> > test script itself (via test_oid_cache <<here-doc). I could even
> > imagine a test script invoking test_oid_cache<<here-doc before each
> > test which has unique OID's in order to localize OID information to
> > the test which needs it, or perhaps even within a test.
>
> Putting them in a separate directory makes it possible to do something
> like this:
>
>    (cd t && ./t0002-* --verbose)
>
> and then use shell editing to change the command line.  If we put them
> in the same directory as the tests, we make developers' lives a bit
> harder.

Perhaps I'm missing something obvious, but I'm not following what
you're trying to convey.

Okay, thinking upon this further, I guess you mean people who type
your example directly rather than using "./t0002-*.sh" or something.

> You mentioned the desire to experiment with additional hash algorithms
> as a hope for this series.  I don't know if that's still something
> desirable to have, now that we've decided on SHA-256, but I felt it
> would make it easier to find all the places that need updating.

I'm still open to the idea of supporting experimentation with other
hash algorithms and I see how lumping all the OID tables into a single
directory can make it easy to locate everything that will require
adjustment for a new algorithm. But, this information can also be
gleaned via a simple grep for "test_oid_cache", so I'm not sure the
lumped-directory approach buys much.

Also, my gut feeling is that such experimentation will be very rare,
whereas the addition of new tests which have some sort of
OID-dependency will likely be a more frequent occurrence. And, the
locality of an OID-table plopped down right next to the test which
requires it seems a win (in my mind).

> > So, I'm having trouble understanding the benefit of the current
> > approach over merely loading OID's via here-docs in the test scripts
> > themselves. Perhaps the commit message could say something about why
> > the current approach was taken.
>
> I can do that.  The idea is that we have lots of common uses of certain
> values that will need to be loaded, and it's better to load some of
> those values from a file.  I felt it would be ugly to have to write out
> the full "$TEST_DIRECTORY..." piece every time.

I do favor the simplicity of the caller deciding whether to use
"test_oid_cache <file" or "test_oid_cache <<here-doc"; it provides
extra flexibility over a function which loads the OID tables from a
fixed path.

However, I'm sympathetic to the ugliness each caller having to specify
"$TEST_DIRECTORY/...". In my review of 2/11, I suggested the idea of a
test_oid_init() function which could load those common OID tables for
scripts which need them, thus hiding that ugliness. Another idea which
has some appeal is to define an $OIDDB variable (or some such name)
with value "$TEST_DIRECTORY/oid-info", which lets callers use:

    test_oid_cache <"$OIDDB/bloop"

which isn't so bad.

> > Why, specifically, return $? here, as opposed to simply returning?
>
> A simple return is probably better.  I think I wasn't clear on whether
> POSIX required a bare "return" to return $? and may not have been online
> at that time to look.  I remember being very clear that I didn't want to
> return 0 unconditionally, though.

I asked because, as far as I can see, this _is_ returning 0
unconditionally since $? will always be 0 by the time it gets to the
'return' (since it's not making any effort to break from the 'while'
loop with a meaningful value upon failure.

Fixing this function to return a meaningful value (success or failure)
might be sensible since that would allow it to be used directly in a
test rather than only outside.

Re: [PATCH v2 01/11] t: add tool to translate hash-related values

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2879 bytes --]

On Sun, Aug 19, 2018 at 07:06:14PM -0400, Eric Sunshine wrote:
> On Sun, Aug 19, 2018 at 5:50 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > I think what I'd like to do instead is store in a variable and make
> > test_oid return unsuccessfully if the value doesn't exist.  I think
> > that's better for writing tests overall and will accomplish the same
> > goal.
> 
> My knee-jerk reaction is that could get clunky in practice, but
> perhaps I'm not seeing the full picture.
> 
> An alternative would be to return a distinguished error value.

We already do it in several other places in the testsuite when we want
to check the exit status of a command substitution, so I think it will
end up being fine.  I'd like to try it and see.

I think that we need to fix the return code either way, though.

> > Putting them in a separate directory makes it possible to do something
> > like this:
> >
> >    (cd t && ./t0002-* --verbose)
> >
> > and then use shell editing to change the command line.  If we put them
> > in the same directory as the tests, we make developers' lives a bit
> > harder.
> 
> Perhaps I'm missing something obvious, but I'm not following what
> you're trying to convey.
> 
> Okay, thinking upon this further, I guess you mean people who type
> your example directly rather than using "./t0002-*.sh" or something.

Right.  It also makes completion nicer (in Vim at least).

> I'm still open to the idea of supporting experimentation with other
> hash algorithms and I see how lumping all the OID tables into a single
> directory can make it easy to locate everything that will require
> adjustment for a new algorithm. But, this information can also be
> gleaned via a simple grep for "test_oid_cache", so I'm not sure the
> lumped-directory approach buys much.
> 
> Also, my gut feeling is that such experimentation will be very rare,
> whereas the addition of new tests which have some sort of
> OID-dependency will likely be a more frequent occurrence. And, the
> locality of an OID-table plopped down right next to the test which
> requires it seems a win (in my mind).

Okay.  I'll do that, and we'll see how it works.

> However, I'm sympathetic to the ugliness each caller having to specify
> "$TEST_DIRECTORY/...". In my review of 2/11, I suggested the idea of a
> test_oid_init() function which could load those common OID tables for
> scripts which need them, thus hiding that ugliness. Another idea which
> has some appeal is to define an $OIDDB variable (or some such name)
> with value "$TEST_DIRECTORY/oid-info", which lets callers use:
> 
>     test_oid_cache <"$OIDDB/bloop"
> 
> which isn't so bad.

I'll stuff in a test_oid_init function.  I think I like that approach
the best.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

Re: [PATCH v2 05/11] t0027: make hash size independent'

[Torsten Bögershausen]

On 20.08.18 00:10, Eric Sunshine wrote:
> On Sun, Aug 19, 2018 at 5:57 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>> On Sun, Aug 19, 2018 at 04:10:21PM -0400, Eric Sunshine wrote:
>>> On Sun, Aug 19, 2018 at 1:54 PM brian m. carlson
>>>> -       tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp" &&
>>>> +       tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp+" &&
>>>
>>> My immediate thought upon reading this was whether "+" is valid in
>>> Windows filenames. Apparently, it is, but perhaps (if you re-roll) it
>>> would make sense to use a character less likely to cause brain
>>> hiccups; for instance, "exp0'.
>>
>> The reason I picked that is that we use it quite a bit in the Makefile,
>> so it seemed like an obvious choice for a temporary file name.  If you
>> feel strongly I can pick something else, but I thought it would be
>> reasonably intuitive for other developers.  Maybe I was wrong, though.
> 
> I don't feel strongly about it. My brain tripped over it probably
> because it's not an idiom in Git tests. In fact, I see just one place
> where "+" has been used like this, in t6026.
> 

Probably "tmp" is a better name than "exp+" 
(Why the '+' ? Is it better that the non-'+' ?)

Anyway,
If we re-order a little bit, can we use a simple '|' instead ?

tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" |
	sed -e "s/0000+/$ZERO_OID/"               >"$exp" &&
tr '\015\000abcdef0123456789' QN00000000000000000 <"$3" >"$act" &&