{"thread":{"id":"46412","subject":"[PATCH] t8008: rely on rev-parse'd HEAD instead of sha1 value","startedAt":"2017-07-18T00:20:28Z","lastAt":"2017-07-26T20:32:39Z","messageCount":6,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"324693","messageId":"20170718002020.14309-1-sbeller@google.com","threadId":"46412","inReplyTo":null,"subject":"[PATCH] t8008: rely on rev-parse'd HEAD instead of sha1 value","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-18T00:20:20Z","receivedAt":"2017-07-18T00:20:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Remove hard coded sha1 values, obtain the values using 'git rev-parse HEAD'\nwhich should be future proof regardless of the hash function used.\n\nSigned-off-by: Stefan Beller \n---\n t/t8008-blame-formats.sh | 28 +++++++++++++++-------------\n 1 file changed, 15 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh\nindex 92c8e792d1..49cac4b9af 100755\n--- a/t/t8008-blame-formats.sh\n+++ b/t/t8008-blame-formats.sh\n@@ -12,22 +12,25 @@ test_expect_success 'setup' '\n \techo c >>file &&\n \techo d >>file &&\n \ttest_tick &&\n-\tgit commit -a -m two\n+\tgit commit -a -m two &&\n+\tID1=$(git rev-parse HEAD^) &&\n+\tshortID1=\"^$(git rev-parse HEAD^ |cut -c 1-7)\" &&\n+\tID2=$(git rev-parse HEAD) &&\n+\tshortID2=\"$(git rev-parse HEAD |cut -c 1-8)\"\n '\n \n-cat >expect <<'EOF'\n-^baf5e0b (A U Thor 2005-04-07 15:13:13 -0700 1) a\n-8825379d (A U Thor 2005-04-07 15:14:13 -0700 2) b\n-8825379d (A U Thor 2005-04-07 15:14:13 -0700 3) c\n-8825379d (A U Thor 2005-04-07 15:14:13 -0700 4) d\n+cat >expect <actual &&\n \ttest_cmp expect actual\n '\n \n-ID1=baf5e0b3869e0b2b2beb395a3720c7b51eac94fc\n-COMMIT1='author A U Thor\n+COMMIT1=\"author A U Thor\n author-mail \n author-time 1112911993\n author-tz -0700\n@@ -37,9 +40,8 @@ committer-time 1112911993\n committer-tz -0700\n summary one\n boundary\n-filename file'\n-ID2=8825379dfb8a1267b58e8e5bcf69eec838f685ec\n-COMMIT2='author A U Thor\n+filename file\"\n+COMMIT2=\"author A U Thor\n author-mail \n author-time 1112912053\n author-tz -0700\n@@ -48,8 +50,8 @@ committer-mail \n committer-time 1112912053\n committer-tz -0700\n summary two\n-previous baf5e0b3869e0b2b2beb395a3720c7b51eac94fc file\n-filename file'\n+previous $ID1 file\n+filename file\"\n \n cat >expect < writes:\n\n> Remove hard coded sha1 values, obtain the values using 'git rev-parse HEAD'\n> which should be future proof regardless of the hash function used.\n\nDon't hardcoded lengths of the hashes defeat this future-proofing\neffort, though? It shouldn't be too hard to do the equivalent of\nthe auto computation of abbreviation in this script, which would be\ntrue future-proofing, I guess.\n\n> Signed-off-by: Stefan Beller \n> ---\n> t/t8008-blame-formats.sh | 28 +++++++++++++++-------------\n> 1 file changed, 15 insertions(+), 13 deletions(-)\n>\n> diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh\n> index 92c8e792d1..49cac4b9af 100755\n> --- a/t/t8008-blame-formats.sh\n> +++ b/t/t8008-blame-formats.sh\n> @@ -12,22 +12,25 @@ test_expect_success 'setup' '\n> \techo c >>file &&\n> \techo d >>file &&\n> \ttest_tick &&\n> -\tgit commit -a -m two\n> +\tgit commit -a -m two &&\n> +\tID1=$(git rev-parse HEAD^) &&\n> +\tshortID1=\"^$(git rev-parse HEAD^ |cut -c 1-7)\" &&\n> +\tID2=$(git rev-parse HEAD) &&\n> +\tshortID2=\"$(git rev-parse HEAD |cut -c 1-8)\"\n> '\n> \n> -cat >expect <<'EOF'\n> -^baf5e0b (A U Thor 2005-04-07 15:13:13 -0700 1) a\n> -8825379d (A U Thor 2005-04-07 15:14:13 -0700 2) b\n> -8825379d (A U Thor 2005-04-07 15:14:13 -0700 3) c\n> -8825379d (A U Thor 2005-04-07 15:14:13 -0700 4) d\n> +cat >expect < +$shortID1 (A U Thor 2005-04-07 15:13:13 -0700 1) a\n> +$shortID2 (A U Thor 2005-04-07 15:14:13 -0700 2) b\n> +$shortID2 (A U Thor 2005-04-07 15:14:13 -0700 3) c\n> +$shortID2 (A U Thor 2005-04-07 15:14:13 -0700 4) d\n> EOF\n> test_expect_success 'normal blame output' '\n> \tgit blame file >actual &&\n> \ttest_cmp expect actual\n> '\n> \n> -ID1=baf5e0b3869e0b2b2beb395a3720c7b51eac94fc\n> -COMMIT1='author A U Thor\n> +COMMIT1=\"author A U Thor\n> author-mail \n> author-time 1112911993\n> author-tz -0700\n> @@ -37,9 +40,8 @@ committer-time 1112911993\n> committer-tz -0700\n> summary one\n> boundary\n> -filename file'\n> -ID2=8825379dfb8a1267b58e8e5bcf69eec838f685ec\n> -COMMIT2='author A U Thor\n> +filename file\"\n> +COMMIT2=\"author A U Thor\n> author-mail \n> author-time 1112912053\n> author-tz -0700\n> @@ -48,8 +50,8 @@ committer-mail \n> committer-time 1112912053\n> committer-tz -0700\n> summary two\n> -previous baf5e0b3869e0b2b2beb395a3720c7b51eac94fc file\n> -filename file'\n> +previous $ID1 file\n> +filename file\"\n> \n> cat >expect < $ID1 1 1 1\n"},{"id":"324729","messageId":"CAGZ79kbTQ=eSoDCRFTt_B3kjDbWZDkcPwc3fMvhd8dEKPWr-8A@mail.gmail.com","threadId":"46412","inReplyTo":"xmqqwp75o8qi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t8008: rely on rev-parse'd HEAD instead of sha1 value","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-18T19:22:35Z","receivedAt":"2017-07-18T19:22:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Jul 18, 2017 at 12:17 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> Remove hard coded sha1 values, obtain the values using 'git rev-parse HEAD'\n>> which should be future proof regardless of the hash function used.\n>\n> Don't hardcoded lengths of the hashes defeat this future-proofing\n> effort, though? It shouldn't be too hard to do the equivalent of\n> the auto computation of abbreviation in this script, which would be\n> true future-proofing, I guess.\n\nIt depends on the definition of future proofing.\nMy definition here only included the change of the hash function,\nnot the change of display length in git-blame for a small artificial repo\nwith 2 commits . These seem to be unrelated, so in case we'd change\nthe length of the abbreviated displayed hash, we'd still want to have\na test to tell us?\n"},{"id":"324731","messageId":"xmqqk235o7rm.fsf@gitster.mtv.corp.google.com","threadId":"46412","inReplyTo":"CAGZ79kbTQ=eSoDCRFTt_B3kjDbWZDkcPwc3fMvhd8dEKPWr-8A@mail.gmail.com","subject":"Re: [PATCH] t8008: rely on rev-parse'd HEAD instead of sha1 value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-18T19:38:21Z","receivedAt":"2017-07-18T19:38:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Tue, Jul 18, 2017 at 12:17 PM, Junio C Hamano wrote:\n>> Stefan Beller writes:\n>>\n>>> Remove hard coded sha1 values, obtain the values using 'git rev-parse HEAD'\n>>> which should be future proof regardless of the hash function used.\n>>\n>> Don't hardcoded lengths of the hashes defeat this future-proofing\n>> effort, though? It shouldn't be too hard to do the equivalent of\n>> the auto computation of abbreviation in this script, which would be\n>> true future-proofing, I guess.\n>\n> It depends on the definition of future proofing.\n> My definition here only included the change of the hash function,\n> not the change of display length in git-blame for a small artificial repo\n> with 2 commits . These seem to be unrelated, so in case we'd change\n> the length of the abbreviated displayed hash, we'd still want to have\n> a test to tell us?\n\nThe thing is that depending on how these 2 commits hash and share\nprefixes, the length needed to disambiguate changes.\n"},{"id":"325141","messageId":"20170726190631.560-1-sbeller@google.com","threadId":"46412","inReplyTo":"xmqqk235o7rm.fsf@gitster.mtv.corp.google.com","subject":"[PATCHv2] t8008: rely on rev-parse'd HEAD instead of sha1 value","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-26T19:06:31Z","receivedAt":"2017-07-26T19:06:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Remove hard coded sha1 values, obtain the values using\n'git rev-parse HEAD' which should be future proof regardless\nof the hash function used.\n\nAdditionally future-proof the test by hard coding the\nabbreviation length of the hash.\n\nSigned-off-by: Stefan Beller \n---\n\n> Don't hardcoded lengths of the hashes defeat this future-proofing\n> effort, though? It shouldn't be too hard to do the equivalent of\n> the auto computation of abbreviation in this script, which would be\n> true future-proofing, I guess.\n\nAdded --abbrev=n to also hard code hash abbreviation. At first\nI had the impression of a off-by-one-error, but after reading the\nman page for both blame and rev-parse, I realize that blames\nabbrev notion is different than rev-parse precisely for the\ncaret that may occur to indicate out-of-range.\n\n t/t8008-blame-formats.sh | 30 ++++++++++++++++--------------\n 1 file changed, 16 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh\nindex 92c8e792d1..ae4b579d24 100755\n--- a/t/t8008-blame-formats.sh\n+++ b/t/t8008-blame-formats.sh\n@@ -12,22 +12,25 @@ test_expect_success 'setup' '\n \techo c >>file &&\n \techo d >>file &&\n \ttest_tick &&\n-\tgit commit -a -m two\n+\tgit commit -a -m two &&\n+\tID1=$(git rev-parse HEAD^) &&\n+\tshortID1=\"^$(git rev-parse HEAD^ |cut -c 1-17)\" &&\n+\tID2=$(git rev-parse HEAD) &&\n+\tshortID2=\"$(git rev-parse HEAD |cut -c 1-18)\"\n '\n \n-cat >expect <<'EOF'\n-^baf5e0b (A U Thor 2005-04-07 15:13:13 -0700 1) a\n-8825379d (A U Thor 2005-04-07 15:14:13 -0700 2) b\n-8825379d (A U Thor 2005-04-07 15:14:13 -0700 3) c\n-8825379d (A U Thor 2005-04-07 15:14:13 -0700 4) d\n+cat >expect <actual &&\n+\tgit blame --abbrev=17 file >actual &&\n \ttest_cmp expect actual\n '\n \n-ID1=baf5e0b3869e0b2b2beb395a3720c7b51eac94fc\n-COMMIT1='author A U Thor\n+COMMIT1=\"author A U Thor\n author-mail \n author-time 1112911993\n author-tz -0700\n@@ -37,9 +40,8 @@ committer-time 1112911993\n committer-tz -0700\n summary one\n boundary\n-filename file'\n-ID2=8825379dfb8a1267b58e8e5bcf69eec838f685ec\n-COMMIT2='author A U Thor\n+filename file\"\n+COMMIT2=\"author A U Thor\n author-mail \n author-time 1112912053\n author-tz -0700\n@@ -48,8 +50,8 @@ committer-mail \n committer-time 1112912053\n committer-tz -0700\n summary two\n-previous baf5e0b3869e0b2b2beb395a3720c7b51eac94fc file\n-filename file'\n+previous $ID1 file\n+filename file\"\n \n cat >expect < writes:\n\n> Remove hard coded sha1 values, obtain the values using\n> 'git rev-parse HEAD' which should be future proof regardless\n> of the hash function used.\n>\n> Additionally future-proof the test by hard coding the\n> abbreviation length of the hash.\n>\n> Signed-off-by: Stefan Beller \n> ---\n>\n>> Don't hardcoded lengths of the hashes defeat this future-proofing\n>> effort, though? It shouldn't be too hard to do the equivalent of\n>> the auto computation of abbreviation in this script, which would be\n>> true future-proofing, I guess.\n>\n> Added --abbrev=n to also hard code hash abbreviation. At first\n> I had the impression of a off-by-one-error, but after reading the\n> man page for both blame and rev-parse, I realize that blames\n> abbrev notion is different than rev-parse precisely for the\n> caret that may occur to indicate out-of-range.\n\nOK, and 17 hexdigits ought to be sufficient for a sample repository\nwe create for the test---we shouldn't be picking a hash that would\ncause hash prefix collisions with such a small number of objects and\nreasonably long prefix anyway ;-)\n\nThanks, will queue.\n\n>\n> t/t8008-blame-formats.sh | 30 ++++++++++++++++--------------\n> 1 file changed, 16 insertions(+), 14 deletions(-)\n>\n> diff --git a/t/t8008-blame-formats.sh b/t/t8008-blame-formats.sh\n> index 92c8e792d1..ae4b579d24 100755\n> --- a/t/t8008-blame-formats.sh\n> +++ b/t/t8008-blame-formats.sh\n> @@ -12,22 +12,25 @@ test_expect_success 'setup' '\n> \techo c >>file &&\n> \techo d >>file &&\n> \ttest_tick &&\n> -\tgit commit -a -m two\n> +\tgit commit -a -m two &&\n> +\tID1=$(git rev-parse HEAD^) &&\n> +\tshortID1=\"^$(git rev-parse HEAD^ |cut -c 1-17)\" &&\n> +\tID2=$(git rev-parse HEAD) &&\n> +\tshortID2=\"$(git rev-parse HEAD |cut -c 1-18)\"\n> '\n> \n> -cat >expect <<'EOF'\n> -^baf5e0b (A U Thor 2005-04-07 15:13:13 -0700 1) a\n> -8825379d (A U Thor 2005-04-07 15:14:13 -0700 2) b\n> -8825379d (A U Thor 2005-04-07 15:14:13 -0700 3) c\n> -8825379d (A U Thor 2005-04-07 15:14:13 -0700 4) d\n> +cat >expect < +$shortID1 (A U Thor 2005-04-07 15:13:13 -0700 1) a\n> +$shortID2 (A U Thor 2005-04-07 15:14:13 -0700 2) b\n> +$shortID2 (A U Thor 2005-04-07 15:14:13 -0700 3) c\n> +$shortID2 (A U Thor 2005-04-07 15:14:13 -0700 4) d\n> EOF\n> test_expect_success 'normal blame output' '\n> -\tgit blame file >actual &&\n> +\tgit blame --abbrev=17 file >actual &&\n> \ttest_cmp expect actual\n> '\n> \n> -ID1=baf5e0b3869e0b2b2beb395a3720c7b51eac94fc\n> -COMMIT1='author A U Thor\n> +COMMIT1=\"author A U Thor\n> author-mail \n> author-time 1112911993\n> author-tz -0700\n> @@ -37,9 +40,8 @@ committer-time 1112911993\n> committer-tz -0700\n> summary one\n> boundary\n> -filename file'\n> -ID2=8825379dfb8a1267b58e8e5bcf69eec838f685ec\n> -COMMIT2='author A U Thor\n> +filename file\"\n> +COMMIT2=\"author A U Thor\n> author-mail \n> author-time 1112912053\n> author-tz -0700\n> @@ -48,8 +50,8 @@ committer-mail \n> committer-time 1112912053\n> committer-tz -0700\n> summary two\n> -previous baf5e0b3869e0b2b2beb395a3720c7b51eac94fc file\n> -filename file'\n> +previous $ID1 file\n> +filename file\"\n> \n> cat >expect < $ID1 1 1 1\n"}]}