[PATCH 00/10] Hash-independent tests (part 1)

[PATCH 00/10] Hash-independent tests (part 1)

[brian m. carlson]

This is a series to make our tests hash-independent.  Many tests have
hard-coded SHA-1 values in them, and it would be valuable to express
these items in a hash-independent way for our hash transitions.

The approach in this series relies on only three components for hash
independence: git rev-parse, git hash-object, and EMPTY_BLOB and
EMPTY_TREE.  Because many of our shell scripts and test components
already rely on the first two, this seems like a safe assumption.

For the same reason, this series avoids modifying tests that test these
components or their expected SHA-1 values.  I expect that when we add
another hash function, we'll copy these tests to expose both SHA-1 and
NewHash versions.

Many of our tests use heredocs for defining expected values.  My
approach has been to interpolate values into the heredocs, as that
produces the best readability in my view.

These tests have been tested using my "short BLAKE2b" series (branch
blake2b-test-hash) and have also been tested based off master.

Comments on any aspect of this series are welcome, but opinions on the
approach or style are especially so.

brian m. carlson (10):
  t1011: abstract away SHA-1-specific constants
  t1304: abstract away SHA-1-specific constants
  t1300: abstract away SHA-1-specific constants
  t1405: sort reflog entries in a hash-independent way
  t1411: abstract away SHA-1-specific constants
  t1507: abstract away SHA-1-specific constants
  t2020: abstract away SHA-1 specific constants
  t2101: modernize test style
  t2101: abstract away SHA-1-specific constants
  t2107: abstract away SHA-1-specific constants

 t/t1011-read-tree-sparse-checkout.sh |  7 ++-
 t/t1300-repo-config.sh               |  6 +-
 t/t1304-default-acl.sh               |  2 +-
 t/t1405-main-ref-store.sh            |  4 +-
 t/t1411-reflog-show.sh               | 21 ++++---
 t/t1507-rev-parse-upstream.sh        |  5 +-
 t/t2020-checkout-detach.sh           | 40 +++++++-----
 t/t2101-update-index-reupdate.sh     | 91 +++++++++++++++-------------
 t/t2107-update-index-basic.sh        |  6 +-
 9 files changed, 100 insertions(+), 82 deletions(-)

[PATCH 01/10] t1011: abstract away SHA-1-specific constants

[brian m. carlson]

Adjust the test so that it computes the expected hash value dynamically
instead of relying on a hard-coded hash.  Hoist some code earlier in the
test to make this possible.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1011-read-tree-sparse-checkout.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/t1011-read-tree-sparse-checkout.sh b/t/t1011-read-tree-sparse-checkout.sh
index c167f606ca..0c6f48f302 100755
--- a/t/t1011-read-tree-sparse-checkout.sh
+++ b/t/t1011-read-tree-sparse-checkout.sh
@@ -15,8 +15,11 @@ test_description='sparse checkout tests
 . "$TEST_DIRECTORY"/lib-read-tree.sh
 
 test_expect_success 'setup' '
+	test_commit init &&
+	echo modified >>init.t &&
+
 	cat >expected <<-EOF &&
-	100644 77f0ba1734ed79d12881f81b36ee134de6a3327b 0	init.t
+	100644 $(git hash-object init.t) 0	init.t
 	100644 $EMPTY_BLOB 0	sub/added
 	100644 $EMPTY_BLOB 0	sub/addedtoo
 	100644 $EMPTY_BLOB 0	subsub/added
@@ -28,8 +31,6 @@ test_expect_success 'setup' '
 	H subsub/added
 	EOF
 
-	test_commit init &&
-	echo modified >>init.t &&
 	mkdir sub subsub &&
 	touch sub/added sub/addedtoo subsub/added &&
 	git add init.t sub/added sub/addedtoo subsub/added &&

[PATCH 02/10] t1304: abstract away SHA-1-specific constants

[brian m. carlson]

Adjust the test so that it uses the $EMPTY_BLOB value instead of
hard-coding the hash.

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

diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index f5422f1d33..335d3f3211 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -54,7 +54,7 @@ test_expect_success SETFACL 'Setup test repo' '
 
 test_expect_success SETFACL 'Objects creation does not break ACLs with restrictive umask' '
 	# SHA1 for empty blob
-	check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
+	check_perms_and_acl .git/objects/$(echo $EMPTY_BLOB | sed -e "s,^\(..\),\1/,")
 '
 
 test_expect_success SETFACL 'git gc does not break ACLs with restrictive umask' '

[PATCH 03/10] t1300: abstract away SHA-1-specific constants

[brian m. carlson]

Adjust the test so that it uses the computed blob value instead of
hard-coding a hash.

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

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 4f8e6f5fde..dc7e6c2e77 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1587,10 +1587,10 @@ test_expect_success '--show-origin stdin with file include' '
 '
 
 test_expect_success !MINGW '--show-origin blob' '
-	cat >expect <<-\EOF &&
-		blob:a9d9f9e555b5c6f07cbe09d3f06fe3df11e09c08	user.custom=true
-	EOF
 	blob=$(git hash-object -w "$CUSTOM_CONFIG_FILE") &&
+	cat >expect <<-EOF &&
+		blob:$blob	user.custom=true
+	EOF
 	git config --blob=$blob --show-origin --list >output &&
 	test_cmp expect output
 '

[PATCH 04/10] t1405: sort reflog entries in a hash-independent way

[brian m. carlson]

The test enumerates reflog entries in an arbitrary order and then sorts
them.  For SHA-1, this produces results that happen to sort in
alphabetical order, but for other hash algorithms they sort differently.
Ensure we sort the reflog entries in a hash-independent way by sorting
on the ref name instead of the object ID.  Remove an assumption about
the length of a hash by using cut with the delimiter and field options
instead of the character range option.

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 e8115df5ba..a1e243a05c 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -45,7 +45,7 @@ test_expect_success 'rename_refs(master, new-master)' '
 '
 
 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
@@ -71,7 +71,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 -k2 | cut -c 42- >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	refs/heads/master 0x0

Re: [PATCH 04/10] t1405: sort reflog entries in a hash-independent way

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> The test enumerates reflog entries in an arbitrary order and then sorts
> them.  For SHA-1, this produces results that happen to sort in
> alphabetical order, but for other hash algorithms they sort differently.
> Ensure we sort the reflog entries in a hash-independent way by sorting
> on the ref name instead of the object ID.

Makes sense.

> Remove an assumption about the length of a hash by using cut with
> the delimiter and field options instead of the character range
> option.

I thought you used your truncated blake hash to develop and verify
these changes, but I'd imagine that the "42" thing were hard to
spot.

Thanks.


> 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 e8115df5ba..a1e243a05c 100755
> --- a/t/t1405-main-ref-store.sh
> +++ b/t/t1405-main-ref-store.sh
> @@ -45,7 +45,7 @@ test_expect_success 'rename_refs(master, new-master)' '
>  '
>  
>  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
> @@ -71,7 +71,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 -k2 | cut -c 42- >actual &&
>  	cat >expected <<-\EOF &&
>  	HEAD 0x1
>  	refs/heads/master 0x0

Re: [PATCH 04/10] t1405: sort reflog entries in a hash-independent way

[brian m. carlson]

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

On Mon, Mar 26, 2018 at 03:18:20PM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > The test enumerates reflog entries in an arbitrary order and then sorts
> > them.  For SHA-1, this produces results that happen to sort in
> > alphabetical order, but for other hash algorithms they sort differently.
> > Ensure we sort the reflog entries in a hash-independent way by sorting
> > on the ref name instead of the object ID.
> 
> Makes sense.
> 
> > Remove an assumption about the length of a hash by using cut with
> > the delimiter and field options instead of the character range
> > option.
> 
> I thought you used your truncated blake hash to develop and verify
> these changes, but I'd imagine that the "42" thing were hard to
> spot.

Yeah, I had to do those by hand, but it looks like I did miss one.  I'll
reroll with that change.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

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

[PATCH 05/10] t1411: abstract away SHA-1-specific constants

[brian m. carlson]

Adjust the test so that it uses a variable consisting of the current
HEAD instead of a hard-coded hash.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1411-reflog-show.sh | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 6ac7734d79..596907758d 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -10,6 +10,7 @@ test_expect_success 'setup' '
 	git commit -m one
 '
 
+commit=$(git rev-parse --short HEAD)
 cat >expect <<'EOF'
 Reflog: HEAD@{0} (C O Mitter <committer@example.com>)
 Reflog message: commit (initial): one
@@ -20,8 +21,8 @@ test_expect_success 'log -g shows reflog headers' '
 	test_cmp expect actual
 '
 
-cat >expect <<'EOF'
-e46513e HEAD@{0}: commit (initial): one
+cat >expect <<EOF
+$commit HEAD@{0}: commit (initial): one
 EOF
 test_expect_success 'oneline reflog format' '
 	git log -g -1 --oneline >actual &&
@@ -33,8 +34,8 @@ test_expect_success 'reflog default format' '
 	test_cmp expect actual
 '
 
-cat >expect <<'EOF'
-commit e46513e
+cat >expect <<EOF
+commit $commit
 Reflog: HEAD@{0} (C O Mitter <committer@example.com>)
 Reflog message: commit (initial): one
 Author: A U Thor <author@example.com>
@@ -56,8 +57,8 @@ test_expect_success 'using @{now} syntax shows reflog date (multiline)' '
 	test_cmp expect actual
 '
 
-cat >expect <<'EOF'
-e46513e HEAD@{Thu Apr 7 15:13:13 2005 -0700}: commit (initial): one
+cat >expect <<EOF
+$commit HEAD@{Thu Apr 7 15:13:13 2005 -0700}: commit (initial): one
 EOF
 test_expect_success 'using @{now} syntax shows reflog date (oneline)' '
 	git log -g -1 --oneline HEAD@{now} >actual &&
@@ -82,8 +83,8 @@ test_expect_success 'using --date= shows reflog date (multiline)' '
 	test_cmp expect actual
 '
 
-cat >expect <<'EOF'
-e46513e HEAD@{Thu Apr 7 15:13:13 2005 -0700}: commit (initial): one
+cat >expect <<EOF
+$commit HEAD@{Thu Apr 7 15:13:13 2005 -0700}: commit (initial): one
 EOF
 test_expect_success 'using --date= shows reflog date (oneline)' '
 	git log -g -1 --oneline --date=default >actual &&
@@ -109,8 +110,8 @@ test_expect_success 'log.date does not invoke "--date" magic (multiline)' '
 	test_cmp expect actual
 '
 
-cat >expect <<'EOF'
-e46513e HEAD@{0}: commit (initial): one
+cat >expect <<EOF
+$commit HEAD@{0}: commit (initial): one
 EOF
 test_expect_success 'log.date does not invoke "--date" magic (oneline)' '
 	test_config log.date raw &&

[PATCH 06/10] t1507: abstract away SHA-1-specific constants

[brian m. carlson]

Adjust the test so that it uses a variable consisting of the current
HEAD instead of a hard-coded hash.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1507-rev-parse-upstream.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 2ce68cc277..93c77eac45 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -209,8 +209,9 @@ test_expect_success '@{u} works when tracking a local branch' '
 	test refs/heads/master = "$(full_name @{u})"
 '
 
+commit=$(git rev-parse HEAD)
 cat >expect <<EOF
-commit 8f489d01d0cc65c3b0f09504ec50b5ed02a70bd5
+commit $commit
 Reflog: master@{0} (C O Mitter <committer@example.com>)
 Reflog message: branch: Created from HEAD
 Author: A U Thor <author@example.com>
@@ -224,7 +225,7 @@ test_expect_success 'log -g other@{u}' '
 '
 
 cat >expect <<EOF
-commit 8f489d01d0cc65c3b0f09504ec50b5ed02a70bd5
+commit $commit
 Reflog: master@{Thu Apr 7 15:17:13 2005 -0700} (C O Mitter <committer@example.com>)
 Reflog message: branch: Created from HEAD
 Author: A U Thor <author@example.com>

[PATCH 07/10] t2020: abstract away SHA-1 specific constants

[brian m. carlson]

Adjust the test so that it uses variables for the revisions we're
checking out instead of hard-coded hashes.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t2020-checkout-detach.sh | 40 +++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index bb4f2e0c63..1fa670625c 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -189,8 +189,12 @@ test_expect_success 'no advice given for explicit detached head state' '
 # Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (new format)
 test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not asked to' "
 
+	commit=$(git rev-parse --short=12 master^) &&
+	commit2=$(git rev-parse --short=12 master~2) &&
+	commit3=$(git rev-parse --short=12 master~3) &&
+
 	# The first detach operation is more chatty than the following ones.
-	cat >1st_detach <<-'EOF' &&
+	cat >1st_detach <<-EOF &&
 	Note: checking out 'HEAD^'.
 
 	You are in 'detached HEAD' state. You can look around, make experimental
@@ -202,18 +206,18 @@ test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not as
 
 	  git checkout -b <new-branch-name>
 
-	HEAD is now at 7c7cd714e262 three
+	HEAD is now at \$commit three
 	EOF
 
 	# The remaining ones just show info about previous and current HEADs.
-	cat >2nd_detach <<-'EOF' &&
-	Previous HEAD position was 7c7cd714e262 three
-	HEAD is now at 139b20d8e6c5 two
+	cat >2nd_detach <<-EOF &&
+	Previous HEAD position was \$commit three
+	HEAD is now at \$commit2 two
 	EOF
 
-	cat >3rd_detach <<-'EOF' &&
-	Previous HEAD position was 139b20d8e6c5 two
-	HEAD is now at d79ce1670bdc one
+	cat >3rd_detach <<-EOF &&
+	Previous HEAD position was \$commit2 two
+	HEAD is now at \$commit3 one
 	EOF
 
 	reset &&
@@ -261,8 +265,12 @@ test_expect_success 'describe_detached_head prints no SHA-1 ellipsis when not as
 # Detached HEAD tests for GIT_PRINT_SHA1_ELLIPSIS (old format)
 test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked to' "
 
+	commit=$(git rev-parse --short=12 master^) &&
+	commit2=$(git rev-parse --short=12 master~2) &&
+	commit3=$(git rev-parse --short=12 master~3) &&
+
 	# The first detach operation is more chatty than the following ones.
-	cat >1st_detach <<-'EOF' &&
+	cat >1st_detach <<-EOF &&
 	Note: checking out 'HEAD^'.
 
 	You are in 'detached HEAD' state. You can look around, make experimental
@@ -274,18 +282,18 @@ test_expect_success 'describe_detached_head does print SHA-1 ellipsis when asked
 
 	  git checkout -b <new-branch-name>
 
-	HEAD is now at 7c7cd714e262... three
+	HEAD is now at \$commit... three
 	EOF
 
 	# The remaining ones just show info about previous and current HEADs.
-	cat >2nd_detach <<-'EOF' &&
-	Previous HEAD position was 7c7cd714e262... three
-	HEAD is now at 139b20d8e6c5... two
+	cat >2nd_detach <<-EOF &&
+	Previous HEAD position was \$commit... three
+	HEAD is now at \$commit2... two
 	EOF
 
-	cat >3rd_detach <<-'EOF' &&
-	Previous HEAD position was 139b20d8e6c5... two
-	HEAD is now at d79ce1670bdc... one
+	cat >3rd_detach <<-EOF &&
+	Previous HEAD position was \$commit2... two
+	HEAD is now at \$commit3... one
 	EOF
 
 	reset &&

[PATCH 08/10] t2101: modernize test style

[brian m. carlson]

Most of our tests start with the opening quote of the test body on the
same line as the test_expect_success call.  Additionally, our tests are
usually indented with a single tab.  Update this test to be the same as
most others, which will make it easier to use inline heredocs in the
future.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t2101-update-index-reupdate.sh | 52 ++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/t/t2101-update-index-reupdate.sh b/t/t2101-update-index-reupdate.sh
index c8bce8c2e4..168733a3c7 100755
--- a/t/t2101-update-index-reupdate.sh
+++ b/t/t2101-update-index-reupdate.sh
@@ -12,15 +12,16 @@ cat > expected <<\EOF
 100644 3b18e512dba79e4c8300dd08aeb37f8e728b8dad 0	file1
 100644 9db8893856a8a02eaa73470054b7c1c5a7c82e47 0	file2
 EOF
-test_expect_success 'update-index --add' \
-	'echo hello world >file1 &&
-	 echo goodbye people >file2 &&
-	 git update-index --add file1 file2 &&
-	 git ls-files -s >current &&
-	 cmp current expected'
+test_expect_success 'update-index --add' '
+	echo hello world >file1 &&
+	echo goodbye people >file2 &&
+	git update-index --add file1 file2 &&
+	git ls-files -s >current &&
+	cmp current expected
+'
 
-test_expect_success 'update-index --again' \
-	'rm -f file1 &&
+test_expect_success 'update-index --again' '
+	rm -f file1 &&
 	echo hello everybody >file2 &&
 	if git update-index --again
 	then
@@ -29,16 +30,18 @@ test_expect_success 'update-index --again' \
 	else
 		echo happy - failed as expected
 	fi &&
-	 git ls-files -s >current &&
-	 cmp current expected'
+	git ls-files -s >current &&
+	cmp current expected
+'
 
 cat > expected <<\EOF
 100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
 EOF
-test_expect_success 'update-index --remove --again' \
-	'git update-index --remove --again &&
-	 git ls-files -s >current &&
-	 cmp current expected'
+test_expect_success 'update-index --remove --again' '
+	git update-index --remove --again &&
+	git ls-files -s >current &&
+	cmp current expected
+'
 
 test_expect_success 'first commit' 'git commit -m initial'
 
@@ -46,8 +49,8 @@ cat > expected <<\EOF
 100644 53ab446c3f4e42ce9bb728a0ccb283a101be4979 0	dir1/file3
 100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
 EOF
-test_expect_success 'update-index again' \
-	'mkdir -p dir1 &&
+test_expect_success 'update-index again' '
+	mkdir -p dir1 &&
 	echo hello world >dir1/file3 &&
 	echo goodbye people >file2 &&
 	git update-index --add file2 dir1/file3 &&
@@ -55,30 +58,33 @@ test_expect_success 'update-index again' \
 	echo happy >dir1/file3 &&
 	git update-index --again &&
 	git ls-files -s >current &&
-	cmp current expected'
+	cmp current expected
+'
 
 cat > expected <<\EOF
 100644 d7fb3f695f06c759dbf3ab00046e7cc2da22d10f 0	dir1/file3
 100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
 EOF
-test_expect_success 'update-index --update from subdir' \
-	'echo not so happy >file2 &&
+test_expect_success 'update-index --update from subdir' '
+	echo not so happy >file2 &&
 	(cd dir1 &&
 	cat ../file2 >file3 &&
 	git update-index --again
 	) &&
 	git ls-files -s >current &&
-	cmp current expected'
+	cmp current expected
+'
 
 cat > expected <<\EOF
 100644 594fb5bb1759d90998e2bf2a38261ae8e243c760 0	dir1/file3
 100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
 EOF
-test_expect_success 'update-index --update with pathspec' \
-	'echo very happy >file2 &&
+test_expect_success 'update-index --update with pathspec' '
+	echo very happy >file2 &&
 	cat file2 >dir1/file3 &&
 	git update-index --again dir1/ &&
 	git ls-files -s >current &&
-	cmp current expected'
+	cmp current expected
+'
 
 test_done

[PATCH 09/10] t2101: abstract away SHA-1-specific constants

[brian m. carlson]

Adjust the test so that it uses variables and command substitution for
blobs instead of hard-coded hashes.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t2101-update-index-reupdate.sh | 41 ++++++++++++++++----------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/t/t2101-update-index-reupdate.sh b/t/t2101-update-index-reupdate.sh
index 168733a3c7..685ec45639 100755
--- a/t/t2101-update-index-reupdate.sh
+++ b/t/t2101-update-index-reupdate.sh
@@ -8,15 +8,15 @@ test_description='git update-index --again test.
 
 . ./test-lib.sh
 
-cat > expected <<\EOF
-100644 3b18e512dba79e4c8300dd08aeb37f8e728b8dad 0	file1
-100644 9db8893856a8a02eaa73470054b7c1c5a7c82e47 0	file2
-EOF
 test_expect_success 'update-index --add' '
 	echo hello world >file1 &&
 	echo goodbye people >file2 &&
 	git update-index --add file1 file2 &&
 	git ls-files -s >current &&
+	cat >expected <<-EOF &&
+	100644 $(git hash-object file1) 0	file1
+	100644 $(git hash-object file2) 0	file2
+	EOF
 	cmp current expected
 '
 
@@ -34,21 +34,17 @@ test_expect_success 'update-index --again' '
 	cmp current expected
 '
 
-cat > expected <<\EOF
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
-EOF
 test_expect_success 'update-index --remove --again' '
 	git update-index --remove --again &&
 	git ls-files -s >current &&
+	cat >expected <<-EOF &&
+	100644 $(git hash-object file2) 0	file2
+	EOF
 	cmp current expected
 '
 
 test_expect_success 'first commit' 'git commit -m initial'
 
-cat > expected <<\EOF
-100644 53ab446c3f4e42ce9bb728a0ccb283a101be4979 0	dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
-EOF
 test_expect_success 'update-index again' '
 	mkdir -p dir1 &&
 	echo hello world >dir1/file3 &&
@@ -58,13 +54,14 @@ test_expect_success 'update-index again' '
 	echo happy >dir1/file3 &&
 	git update-index --again &&
 	git ls-files -s >current &&
+	cat >expected <<-EOF &&
+	100644 $(git hash-object dir1/file3) 0	dir1/file3
+	100644 $(git hash-object file2) 0	file2
+	EOF
 	cmp current expected
 '
 
-cat > expected <<\EOF
-100644 d7fb3f695f06c759dbf3ab00046e7cc2da22d10f 0	dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
-EOF
+file2=$(git hash-object file2)
 test_expect_success 'update-index --update from subdir' '
 	echo not so happy >file2 &&
 	(cd dir1 &&
@@ -72,18 +69,22 @@ test_expect_success 'update-index --update from subdir' '
 	git update-index --again
 	) &&
 	git ls-files -s >current &&
-	cmp current expected
+	cat >expected <<-EOF &&
+	100644 $(git hash-object dir1/file3) 0	dir1/file3
+	100644 $file2 0	file2
+	EOF
+	test_cmp current expected
 '
 
-cat > expected <<\EOF
-100644 594fb5bb1759d90998e2bf2a38261ae8e243c760 0	dir1/file3
-100644 0f1ae1422c2bf43f117d3dbd715c988a9ed2103f 0	file2
-EOF
 test_expect_success 'update-index --update with pathspec' '
 	echo very happy >file2 &&
 	cat file2 >dir1/file3 &&
 	git update-index --again dir1/ &&
 	git ls-files -s >current &&
+	cat >expected <<-EOF &&
+	100644 $(git hash-object dir1/file3) 0	dir1/file3
+	100644 $file2 0	file2
+	EOF
 	cmp current expected
 '
 

[PATCH 10/10] t2107: abstract away SHA-1-specific constants

[brian m. carlson]

Use the $EMPTY_BLOB variable instead of hard-coding a hash.

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

diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 32ac6e09bd..1db7e6a1ab 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -85,9 +85,9 @@ test_expect_success '--chmod=+x and chmod=-x in the same argument list' '
 	>B &&
 	git add A B &&
 	git update-index --chmod=+x A --chmod=-x B &&
-	cat >expect <<-\EOF &&
-	100755 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	A
-	100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0	B
+	cat >expect <<-EOF &&
+	100755 $EMPTY_BLOB 0	A
+	100644 $EMPTY_BLOB 0	B
 	EOF
 	git ls-files --stage A B >actual &&
 	test_cmp expect actual

Re: [PATCH 00/10] Hash-independent tests (part 1)

[Eric Sunshine]

On Sun, Mar 25, 2018 at 3:20 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> This is a series to make our tests hash-independent.  Many tests have
> hard-coded SHA-1 values in them, and it would be valuable to express
> these items in a hash-independent way for our hash transitions.
>
> The approach in this series relies on only three components for hash
> independence: git rev-parse, git hash-object, and EMPTY_BLOB and
> EMPTY_TREE.  Because many of our shell scripts and test components
> already rely on the first two, this seems like a safe assumption.
>
> For the same reason, this series avoids modifying tests that test these
> components or their expected SHA-1 values.  I expect that when we add
> another hash function, we'll copy these tests to expose both SHA-1 and
> NewHash versions.

What's the plan for oddball cases such as 66ae9a57b8 (t3404: rebase
-i: demonstrate short SHA-1 collision, 2013-08-23) which depend
implicitly upon SHA-1 without actually hardcoding any hashes? The test
added by 66ae9a57b8, for instance, won't start failing in the face of
NewHash, but it also won't be testing anything meaningful.

Should such tests be dropped altogether? Should they be marked with a
'SHA1' predicate or be annotated with a comment as being
SHA-1-specific? Something else?

Re: [PATCH 00/10] Hash-independent tests (part 1)

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> What's the plan for oddball cases such as 66ae9a57b8 (t3404: rebase
> -i: demonstrate short SHA-1 collision, 2013-08-23) which depend
> implicitly upon SHA-1 without actually hardcoding any hashes? The test
> added by 66ae9a57b8, for instance, won't start failing in the face of
> NewHash, but it also won't be testing anything meaningful.
>
> Should such tests be dropped altogether? Should they be marked with a
> 'SHA1' predicate or be annotated with a comment as being
> SHA-1-specific? Something else?

Ideally, the existing one be annotated with prereq SHA1, and also
duplicated with a tweak to cause the same kind of (half-)collision
under the NewHash and be annotated with prereq NewHash.

It's a different matter how feasible it is to attain such an ideal,
though.  t1512 was fun to write, but it was quite a lot of work to
come up with bunch of blobs, trees and commits whose object names
share the common prefix 0{10}.

Re: [PATCH 00/10] Hash-independent tests (part 1)

[Johannes Schindelin]

Hi Junio,

On Sun, 25 Mar 2018, Junio C Hamano wrote:

> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > What's the plan for oddball cases such as 66ae9a57b8 (t3404: rebase
> > -i: demonstrate short SHA-1 collision, 2013-08-23) which depend
> > implicitly upon SHA-1 without actually hardcoding any hashes? The test
> > added by 66ae9a57b8, for instance, won't start failing in the face of
> > NewHash, but it also won't be testing anything meaningful.
> >
> > Should such tests be dropped altogether? Should they be marked with a
> > 'SHA1' predicate or be annotated with a comment as being
> > SHA-1-specific? Something else?
> 
> Ideally, the existing one be annotated with prereq SHA1, and also
> duplicated with a tweak to cause the same kind of (half-)collision
> under the NewHash and be annotated with prereq NewHash.

That's a good idea. I wonder whether we want to be a bit more specific,
though, so that we have something grep'able for the future? Something like
SHA1_SHORT_COLLISION or some such?

> It's a different matter how feasible it is to attain such an ideal,
> though.  t1512 was fun to write, but it was quite a lot of work to
> come up with bunch of blobs, trees and commits whose object names
> share the common prefix 0{10}.

Did you write a helper to brute-force those? If so, we might want to have
such a helper in t/helper/ to generate/re-generate those (i.e. it could be
asked to generate a blob whose ID starts with five NUL bytes, and it would
have hard-coded values for already-generated ones). Even if you did not
write such a helper, we might want to have such a helper. That would take
the responsibility away from the shell script.

Ciao,
Dscho

Re: [PATCH 00/10] Hash-independent tests (part 1)

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Ideally, the existing one be annotated with prereq SHA1, and also
>> duplicated with a tweak to cause the same kind of (half-)collision
>> under the NewHash and be annotated with prereq NewHash.
>
> That's a good idea. I wonder whether we want to be a bit more specific,
> though, so that we have something grep'able for the future? Something like
> SHA1_SHORT_COLLISION or some such?

Sorry, you lost me.  

What I meant was that a test, for example, that expects the object
name for an empty blob begins with e69de29 is valid ONLY when Git is
using SHA-1 to name objects, so such a test should be run only when
Git is using SHA-1 and no other hash.  All tests in t1512 that
expects the sequence of events in it would yield blobs and trees
whose names have many leading "0"s are the same way.

I think it would do to have a single prerequisite to cover all such
tests: "Run this piece of test only when Git is using SHA-1 hash".
I do not think you need a set of prerequisites to say "Run this with
SHA-1 because we are testing X" where X may be "disambiguation",
"unique-abbrev", "loose-refs", or whatever.

Re: [PATCH 00/10] Hash-independent tests (part 1)

[Johannes Schindelin]

Hi Junio,

On Wed, 28 Mar 2018, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Ideally, the existing one be annotated with prereq SHA1, and also
> >> duplicated with a tweak to cause the same kind of (half-)collision
> >> under the NewHash and be annotated with prereq NewHash.
> >
> > That's a good idea. I wonder whether we want to be a bit more specific,
> > though, so that we have something grep'able for the future? Something like
> > SHA1_SHORT_COLLISION or some such?
> 
> Sorry, you lost me.  
> 
> What I meant was that a test, for example, that expects the object
> name for an empty blob begins with e69de29 is valid ONLY when Git is
> using SHA-1 to name objects, so such a test should be run only when
> Git is using SHA-1 and no other hash.  All tests in t1512 that
> expects the sequence of events in it would yield blobs and trees
> whose names have many leading "0"s are the same way.
> 
> I think it would do to have a single prerequisite to cover all such
> tests: "Run this piece of test only when Git is using SHA-1 hash".
> I do not think you need a set of prerequisites to say "Run this with
> SHA-1 because we are testing X" where X may be "disambiguation",
> "unique-abbrev", "loose-refs", or whatever.

I meant for the test case to be annotated such that one could find easily
which test cases rely on specially-crafted objects to cause identical hash
prefixes.

But I guess it is not worth the effort (because you'll only find out which
tests miss that annotation when you try to port the test suite to a new
hash).

Ciao,
Dscho

Re: [PATCH 00/10] Hash-independent tests (part 1)

[brian m. carlson]

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

On Sun, Mar 25, 2018 at 10:10:21PM -0400, Eric Sunshine wrote:
> What's the plan for oddball cases such as 66ae9a57b8 (t3404: rebase
> -i: demonstrate short SHA-1 collision, 2013-08-23) which depend
> implicitly upon SHA-1 without actually hardcoding any hashes? The test
> added by 66ae9a57b8, for instance, won't start failing in the face of
> NewHash, but it also won't be testing anything meaningful.
> 
> Should such tests be dropped altogether? Should they be marked with a
> 'SHA1' predicate or be annotated with a comment as being
> SHA-1-specific? Something else?

My plan for these was to treat them the same way as for git rev-parse
and git hash-object.  Basically, for the moment, I had planned to ignore
them, although I like the idea for a prerequisite to get the full
testsuite passing in the interim.

Ultimately, we could use some sort of lookup table or a test helper to
translate them so that we get functionally equivalent results.  t1512 is
another great example of this same kind of test.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

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

Re: [PATCH 00/10] Hash-independent tests (part 1)

[Johannes Schindelin]

Hi Brian,

On Sun, 25 Mar 2018, brian m. carlson wrote:

> This is a series to make our tests hash-independent.  Many tests have
> hard-coded SHA-1 values in them, and it would be valuable to express
> these items in a hash-independent way for our hash transitions.
> 
> The approach in this series relies on only three components for hash
> independence: git rev-parse, git hash-object, and EMPTY_BLOB and
> EMPTY_TREE.  Because many of our shell scripts and test components
> already rely on the first two, this seems like a safe assumption.
> 
> For the same reason, this series avoids modifying tests that test these
> components or their expected SHA-1 values.  I expect that when we add
> another hash function, we'll copy these tests to expose both SHA-1 and
> NewHash versions.
> 
> Many of our tests use heredocs for defining expected values.  My
> approach has been to interpolate values into the heredocs, as that
> produces the best readability in my view.
> 
> These tests have been tested using my "short BLAKE2b" series (branch
> blake2b-test-hash) and have also been tested based off master.
> 
> Comments on any aspect of this series are welcome, but opinions on the
> approach or style are especially so.

Thank you for this patch series!

I reviewed all 10 patches, and while I cannot say anything about whether
they miss any spot, they all look sensible and correct.

Thanks,
Dscho