{"thread":{"id":"56311","subject":"[PATCH] t6300: don't run cat-file on non-existent object","startedAt":"2021-08-17T11:49:30Z","lastAt":"2021-08-21T01:36:55Z","messageCount":11,"participants":["Đoàn Trần Công Danh","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"432944","messageId":"bcbde2e7364865ac16702447b863b8a725670428.1629200841.git.congdanhqx@gmail.com","threadId":"56311","inReplyTo":null,"subject":"[PATCH] t6300: don't run cat-file on non-existent object","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-17T11:48:53Z","receivedAt":"2021-08-17T11:49:30Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"In t6300, some tests are guarded behind some prerequisites.\nThus, objects created by those tests ain't available if those\nprerequisites is unsatistified.  Attempting to run \"cat-file\"\non those objects will run into failure.\n\nIn fact, running t6300 in an environment without gpg(1),\nwe'll see those warnings:\n\n\tfatal: Not a valid object name refs/tags/signed-empty\n\tfatal: Not a valid object name refs/tags/signed-short\n\tfatal: Not a valid object name refs/tags/signed-long\n\nLet's put those commands into the real tests, in order to:\n\n* skip their execution if prerequisites aren't satistified.\n* check their exit status code\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/t6300-for-each-ref.sh | 27 ++++++++++++++++-----------\n 1 file changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 9e0214076b..65fbed2bef 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -59,18 +59,23 @@ test_atom() {\n \t# Automatically test \"contents:size\" atom after testing \"contents\"\n \tif test \"$2\" = \"contents\"\n \tthen\n-\t\tcase $(git cat-file -t \"$ref\") in\n-\t\ttag)\n-\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n-\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n-\t\ttree | blob)\n-\t\t\texpect='' ;;\n-\t\tcommit)\n-\t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n-\t\tesac\n-\t\t# Leave $expect unquoted to lose possible leading whitespaces\n-\t\techo $expect >expected\n+\t\texpect=$(printf '%s' \"$3\" | wc -c)\n \t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n+\t\t\ttype=$(git cat-file -t \"$ref\") &&\n+\t\t\tcase $type in\n+\t\t\ttag)\n+\t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n+\t\t\t\tgit cat-file tag $ref >out &&\n+\t\t\t\texpect=$(<out tail -n +6 | wc -c) ;;\n+\t\t\ttree | blob)\n+\t\t\t\texpect=\"\" ;;\n+\t\t\tcommit)\n+\t\t\t\t: \"use the calculated expect\" ;;\n+\t\t\t*)\n+\t\t\t\tBUG \"unknown object type\" ;;\n+\t\t\tesac &&\n+\t\t\t# Leave $expect unquoted to lose possible leading whitespaces\n+\t\t\techo $expect >expected &&\n \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n \t\t\ttest_cmp expected actual\n \t\t'\n-- \n2.33.0\n\n"},{"id":"433032","messageId":"nycvar.QRO.7.76.6.2108172339080.55@tvgsbejvaqbjf.bet","threadId":"56311","inReplyTo":"bcbde2e7364865ac16702447b863b8a725670428.1629200841.git.congdanhqx@gmail.com","subject":"Re: [PATCH] t6300: don't run cat-file on non-existent object","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-17T21:44:35Z","receivedAt":"2021-08-17T21:44:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Danh,\n\nOn Tue, 17 Aug 2021, Đoàn Trần Công Danh wrote:\n\n> In t6300, some tests are guarded behind some prerequisites.\n> Thus, objects created by those tests ain't available if those\n> prerequisites is unsatistified.  Attempting to run \"cat-file\"\n> on those objects will run into failure.\n>\n> In fact, running t6300 in an environment without gpg(1),\n> we'll see those warnings:\n>\n> \tfatal: Not a valid object name refs/tags/signed-empty\n> \tfatal: Not a valid object name refs/tags/signed-short\n> \tfatal: Not a valid object name refs/tags/signed-long\n>\n> Let's put those commands into the real tests, in order to:\n>\n> * skip their execution if prerequisites aren't satistified.\n> * check their exit status code\n>\n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n\nMakes sense.\n\n> ---\n>  t/t6300-for-each-ref.sh | 27 ++++++++++++++++-----------\n>  1 file changed, 16 insertions(+), 11 deletions(-)\n>\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 9e0214076b..65fbed2bef 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -59,18 +59,23 @@ test_atom() {\n>  \t# Automatically test \"contents:size\" atom after testing \"contents\"\n>  \tif test \"$2\" = \"contents\"\n>  \tthen\n> -\t\tcase $(git cat-file -t \"$ref\") in\n> -\t\ttag)\n> -\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n> -\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n\nHere, we pipe the output of `cat-file` to `tail` and then `wc`. But below:\n\n> -\t\ttree | blob)\n> -\t\t\texpect='' ;;\n> -\t\tcommit)\n> -\t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n> -\t\tesac\n> -\t\t# Leave $expect unquoted to lose possible leading whitespaces\n> -\t\techo $expect >expected\n> +\t\texpect=$(printf '%s' \"$3\" | wc -c)\n>  \t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n> +\t\t\ttype=$(git cat-file -t \"$ref\") &&\n> +\t\t\tcase $type in\n> +\t\t\ttag)\n> +\t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n> +\t\t\t\tgit cat-file tag $ref >out &&\n> +\t\t\t\texpect=$(<out tail -n +6 | wc -c) ;;\n\n... we break the _first_ pipe apart, redirecting into `out` instead. I am\nnot sure that this patch should change that as it does, I would think that\na regular code move (with re-indentation) would be preferable.\n\nBesides, while it is legal and works, I don't think we ever start with the\nredirection. Read: it should probably be `tail -n +6 <out` instead.\n\n> +\t\t\ttree | blob)\n> +\t\t\t\texpect=\"\" ;;\n> +\t\t\tcommit)\n> +\t\t\t\t: \"use the calculated expect\" ;;\n\nThis necessarily has to be different from the original code (i.e. the code\ncould not have been moved verbatim) because it uses `$3`, which at this\npoint has a different value.\n\nMy suggestion: mention this in the commit message, other reviewers or\nfuture readers might stumble over this otherwise.\n\n> +\t\t\t*)\n> +\t\t\t\tBUG \"unknown object type\" ;;\n\nThis one is new. Do we need it?\n\nThanks,\nDscho\n\n> +\t\t\tesac &&\n> +\t\t\t# Leave $expect unquoted to lose possible leading whitespaces\n> +\t\t\techo $expect >expected &&\n>  \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n>  \t\t\ttest_cmp expected actual\n>  \t\t'\n> --\n> 2.33.0\n>\n>\n"},{"id":"433042","messageId":"cover.1629263759.git.congdanhqx@gmail.com","threadId":"56311","inReplyTo":"bcbde2e7364865ac16702447b863b8a725670428.1629200841.git.congdanhqx@gmail.com","subject":"[PATCH v2 0/2] t6300: clear warning when running without gpg","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-18T05:19:25Z","receivedAt":"2021-08-18T05:19:39Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"\nRunning t6300 in an environment without gpg(1),\nwe'll see those warnings:\n\n\tfatal: Not a valid object name refs/tags/signed-empty\n\tfatal: Not a valid object name refs/tags/signed-short\n\tfatal: Not a valid object name refs/tags/signed-long\n\nBecause, those objects will be created only when GPG is satistified.\nThis series try to clean those errors.\n\nChange from v1:\n* Make 1/2 as near pure-code-move; and\n* Use 2/2 as a code change to preserve status code for cat-file\n* Mention reasons that 1/2 couldn't be pure-code-move.\n\nĐoàn Trần Công Danh (2):\n  t6300: don't run cat-file on non-existent object\n  t6300: check for cat-file exit status code\n\n t/t6300-for-each-ref.sh | 29 ++++++++++++++++++-----------\n 1 file changed, 18 insertions(+), 11 deletions(-)\n\nRange-diff against v1:\n1:  6d36f3a8df ! 1:  b813d6f2ad t6300: don't run cat-file on non-existent object\n    @@ Commit message\n         * skip their execution if prerequisites aren't satistified.\n         * check their exit status code\n     \n    +    The expected value for objects with type: commit needs to be\n    +    computed outside the test because we can't relies on \"$3\" there.\n    +    Furthermore, to prevent the accidental usage of that computed\n    +    expected value, BUG out on unknown object's type.\n    +\n         Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n     \n      ## t/t6300-for-each-ref.sh ##\n    @@ t/t6300-for-each-ref.sh: test_atom() {\n     -\t\tesac\n     -\t\t# Leave $expect unquoted to lose possible leading whitespaces\n     -\t\techo $expect >expected\n    ++\t\t# for commit leg, $3 is changed there\n     +\t\texpect=$(printf '%s' \"$3\" | wc -c)\n      \t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n     +\t\t\ttype=$(git cat-file -t \"$ref\") &&\n     +\t\t\tcase $type in\n     +\t\t\ttag)\n     +\t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n    -+\t\t\t\tgit cat-file tag $ref >out &&\n    -+\t\t\t\texpect=$(<out tail -n +6 | wc -c) ;;\n    ++\t\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n     +\t\t\ttree | blob)\n     +\t\t\t\texpect=\"\" ;;\n     +\t\t\tcommit)\n-:  ---------- > 2:  68ee769121 t6300: check for cat-file exit status code\n-- \n2.33.0.rc1\n\n"},{"id":"433043","messageId":"b813d6f2ad96d79a8904ef8b255d4b73ea6567d2.1629263759.git.congdanhqx@gmail.com","threadId":"56311","inReplyTo":"cover.1629263759.git.congdanhqx@gmail.com","subject":"[PATCH v2 1/2] t6300: don't run cat-file on non-existent object","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-18T05:19:26Z","receivedAt":"2021-08-18T05:19:40Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"In t6300, some tests are guarded behind some prerequisites.\nThus, objects created by those tests ain't available if those\nprerequisites is unsatistified.  Attempting to run \"cat-file\"\non those objects will run into failure.\n\nIn fact, running t6300 in an environment without gpg(1),\nwe'll see those warnings:\n\n\tfatal: Not a valid object name refs/tags/signed-empty\n\tfatal: Not a valid object name refs/tags/signed-short\n\tfatal: Not a valid object name refs/tags/signed-long\n\nLet's put those commands into the real tests, in order to:\n\n* skip their execution if prerequisites aren't satistified.\n* check their exit status code\n\nThe expected value for objects with type: commit needs to be\ncomputed outside the test because we can't relies on \"$3\" there.\nFurthermore, to prevent the accidental usage of that computed\nexpected value, BUG out on unknown object's type.\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/t6300-for-each-ref.sh | 27 ++++++++++++++++-----------\n 1 file changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 0d2e062f79..93126341b3 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -59,18 +59,23 @@ test_atom() {\n \t# Automatically test \"contents:size\" atom after testing \"contents\"\n \tif test \"$2\" = \"contents\"\n \tthen\n-\t\tcase $(git cat-file -t \"$ref\") in\n-\t\ttag)\n-\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n-\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n-\t\ttree | blob)\n-\t\t\texpect='' ;;\n-\t\tcommit)\n-\t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n-\t\tesac\n-\t\t# Leave $expect unquoted to lose possible leading whitespaces\n-\t\techo $expect >expected\n+\t\t# for commit leg, $3 is changed there\n+\t\texpect=$(printf '%s' \"$3\" | wc -c)\n \t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n+\t\t\ttype=$(git cat-file -t \"$ref\") &&\n+\t\t\tcase $type in\n+\t\t\ttag)\n+\t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n+\t\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n+\t\t\ttree | blob)\n+\t\t\t\texpect=\"\" ;;\n+\t\t\tcommit)\n+\t\t\t\t: \"use the calculated expect\" ;;\n+\t\t\t*)\n+\t\t\t\tBUG \"unknown object type\" ;;\n+\t\t\tesac &&\n+\t\t\t# Leave $expect unquoted to lose possible leading whitespaces\n+\t\t\techo $expect >expected &&\n \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n \t\t\ttest_cmp expected actual\n \t\t'\n-- \n2.33.0.rc1\n\n"},{"id":"433044","messageId":"68ee769121195eb61bb51fd6a27d22a8dddb13b6.1629263759.git.congdanhqx@gmail.com","threadId":"56311","inReplyTo":"cover.1629263759.git.congdanhqx@gmail.com","subject":"[PATCH v2 2/2] t6300: check for cat-file exit status code","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-18T05:19:27Z","receivedAt":"2021-08-18T05:19:41Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"In test_atom(), we're piping the output of cat-file to tail(1),\nthus, losing its exit status.\n\nLet's use a temporary file to preserve git exit status code.\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/t6300-for-each-ref.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 93126341b3..cc0f5b6627 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -66,7 +66,9 @@ test_atom() {\n \t\t\tcase $type in\n \t\t\ttag)\n \t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n-\t\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n+\t\t\t\tgit cat-file tag $ref >out &&\n+\t\t\t\texpect=$(tail -n +6 <out | wc -c) &&\n+\t\t\t\trm -f out ;;\n \t\t\ttree | blob)\n \t\t\t\texpect=\"\" ;;\n \t\t\tcommit)\n-- \n2.33.0.rc1\n\n"},{"id":"433061","messageId":"nycvar.QRO.7.76.6.2108181231400.55@tvgsbejvaqbjf.bet","threadId":"56311","inReplyTo":"cover.1629263759.git.congdanhqx@gmail.com","subject":"Re: [PATCH v2 0/2] t6300: clear warning when running without gpg","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-08-18T10:33:12Z","receivedAt":"2021-08-18T10:34:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Danh,\n\nOn Wed, 18 Aug 2021, Đoàn Trần Công Danh wrote:\n\n>\n> Running t6300 in an environment without gpg(1),\n> we'll see those warnings:\n>\n> \tfatal: Not a valid object name refs/tags/signed-empty\n> \tfatal: Not a valid object name refs/tags/signed-short\n> \tfatal: Not a valid object name refs/tags/signed-long\n>\n> Because, those objects will be created only when GPG is satistified.\n> This series try to clean those errors.\n>\n> Change from v1:\n> * Make 1/2 as near pure-code-move; and\n> * Use 2/2 as a code change to preserve status code for cat-file\n> * Mention reasons that 1/2 couldn't be pure-code-move.\n\nThank you for accommodating my concerns so quickly. The code was still so\npresent in my mind that I did not have to go back to remind myself, but it\nwas sufficient to look through the range-diff.\n\nThis version happily gets my `Reviewed-by:`.\n\nThanks,\nDscho\n\n>\n> Đoàn Trần Công Danh (2):\n>   t6300: don't run cat-file on non-existent object\n>   t6300: check for cat-file exit status code\n>\n>  t/t6300-for-each-ref.sh | 29 ++++++++++++++++++-----------\n>  1 file changed, 18 insertions(+), 11 deletions(-)\n>\n> Range-diff against v1:\n> 1:  6d36f3a8df ! 1:  b813d6f2ad t6300: don't run cat-file on non-existent object\n>     @@ Commit message\n>          * skip their execution if prerequisites aren't satistified.\n>          * check their exit status code\n>\n>     +    The expected value for objects with type: commit needs to be\n>     +    computed outside the test because we can't relies on \"$3\" there.\n>     +    Furthermore, to prevent the accidental usage of that computed\n>     +    expected value, BUG out on unknown object's type.\n>     +\n>          Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n>\n>       ## t/t6300-for-each-ref.sh ##\n>     @@ t/t6300-for-each-ref.sh: test_atom() {\n>      -\t\tesac\n>      -\t\t# Leave $expect unquoted to lose possible leading whitespaces\n>      -\t\techo $expect >expected\n>     ++\t\t# for commit leg, $3 is changed there\n>      +\t\texpect=$(printf '%s' \"$3\" | wc -c)\n>       \t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n>      +\t\t\ttype=$(git cat-file -t \"$ref\") &&\n>      +\t\t\tcase $type in\n>      +\t\t\ttag)\n>      +\t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n>     -+\t\t\t\tgit cat-file tag $ref >out &&\n>     -+\t\t\t\texpect=$(<out tail -n +6 | wc -c) ;;\n>     ++\t\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n>      +\t\t\ttree | blob)\n>      +\t\t\t\texpect=\"\" ;;\n>      +\t\t\tcommit)\n> -:  ---------- > 2:  68ee769121 t6300: check for cat-file exit status code\n> --\n> 2.33.0.rc1\n>\n>\n"},{"id":"433165","messageId":"xmqqo89tyyu2.fsf@gitster.g","threadId":"56311","inReplyTo":"b813d6f2ad96d79a8904ef8b255d4b73ea6567d2.1629263759.git.congdanhqx@gmail.com","subject":"Re: [PATCH v2 1/2] t6300: don't run cat-file on non-existent object","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-19T20:16:37Z","receivedAt":"2021-08-19T20:16:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> In t6300, some tests are guarded behind some prerequisites.\n> Thus, objects created by those tests ain't available if those\n> prerequisites is unsatistified.  Attempting to run \"cat-file\"\n\nis -> are.\n\n> on those objects will run into failure.\n>\n> In fact, running t6300 in an environment without gpg(1),\n> we'll see those warnings:\n>\n> \tfatal: Not a valid object name refs/tags/signed-empty\n> \tfatal: Not a valid object name refs/tags/signed-short\n> \tfatal: Not a valid object name refs/tags/signed-long\n>\n> Let's put those commands into the real tests, in order to:\n>\n> * skip their execution if prerequisites aren't satistified.\n> * check their exit status code\n>\n> The expected value for objects with type: commit needs to be\n> computed outside the test because we can't relies on \"$3\" there.\n\nrelies -> rely\n"},{"id":"433166","messageId":"xmqqim01yyox.fsf@gitster.g","threadId":"56311","inReplyTo":"68ee769121195eb61bb51fd6a27d22a8dddb13b6.1629263759.git.congdanhqx@gmail.com","subject":"Re: [PATCH v2 2/2] t6300: check for cat-file exit status code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-19T20:19:42Z","receivedAt":"2021-08-19T20:19:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:\n\n> In test_atom(), we're piping the output of cat-file to tail(1),\n> thus, losing its exit status.\n>\n> Let's use a temporary file to preserve git exit status code.\n>\n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> ---\n>  t/t6300-for-each-ref.sh | 4 +++-\n>  1 file changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\n> index 93126341b3..cc0f5b6627 100755\n> --- a/t/t6300-for-each-ref.sh\n> +++ b/t/t6300-for-each-ref.sh\n> @@ -66,7 +66,9 @@ test_atom() {\n>  \t\t\tcase $type in\n>  \t\t\ttag)\n>  \t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n> -\t\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n> +\t\t\t\tgit cat-file tag $ref >out &&\n> +\t\t\t\texpect=$(tail -n +6 <out | wc -c) &&\n\nIt is not wrong per-se, but do we need a redirect '<' here?  \"tail\"\ntakes filename(s) on the command line, but is there a reason to feed\nthe contents from the standard input?\n\n> +\t\t\t\trm -f out ;;\n>  \t\t\ttree | blob)\n>  \t\t\t\texpect=\"\" ;;\n>  \t\t\tcommit)\n"},{"id":"433290","messageId":"cover.1629509530.git.congdanhqx@gmail.com","threadId":"56311","inReplyTo":"bcbde2e7364865ac16702447b863b8a725670428.1629200841.git.congdanhqx@gmail.com","subject":"[PATCH v3 0/2] t6300: clear warning when running without gpg","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-21T01:36:32Z","receivedAt":"2021-08-21T01:36:52Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Running t6300 in an environment without gpg(1),\nwe'll see those warnings:\n\n\tfatal: Not a valid object name refs/tags/signed-empty\n\tfatal: Not a valid object name refs/tags/signed-short\n\tfatal: Not a valid object name refs/tags/signed-long\n\nBecause, those objects will be created only when GPG is satistified.\nThis series try to clean those errors.\n\nChange in v3 from v2:\n* Fix grammar in 1/2 commit's message\n* Let tail open input file instead of using shell redirection.\n\nChange in v2 from v1:\n* Make 1/2 as near pure-code-move; and\n* Use 2/2 as a code change to preserve status code for cat-file\n* Mention reasons that 1/2 couldn't be pure-code-move.\n\n\nĐoàn Trần Công Danh (2):\n  t6300: don't run cat-file on non-existent object\n  t6300: check for cat-file exit status code\n\n t/t6300-for-each-ref.sh | 29 ++++++++++++++++++-----------\n 1 file changed, 18 insertions(+), 11 deletions(-)\n\nRange-diff against v2:\n1:  b813d6f2ad ! 1:  b1b9771913 t6300: don't run cat-file on non-existent object\n    @@ Commit message\n     \n         In t6300, some tests are guarded behind some prerequisites.\n         Thus, objects created by those tests ain't available if those\n    -    prerequisites is unsatistified.  Attempting to run \"cat-file\"\n    +    prerequisites are unsatistified.  Attempting to run \"cat-file\"\n         on those objects will run into failure.\n     \n         In fact, running t6300 in an environment without gpg(1),\n    @@ Commit message\n         * check their exit status code\n     \n         The expected value for objects with type: commit needs to be\n    -    computed outside the test because we can't relies on \"$3\" there.\n    +    computed outside the test because we can't rely on \"$3\" there.\n         Furthermore, to prevent the accidental usage of that computed\n         expected value, BUG out on unknown object's type.\n     \n2:  68ee769121 ! 2:  83d532528b t6300: check for cat-file exit status code\n    @@ t/t6300-for-each-ref.sh: test_atom() {\n      \t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n     -\t\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n     +\t\t\t\tgit cat-file tag $ref >out &&\n    -+\t\t\t\texpect=$(tail -n +6 <out | wc -c) &&\n    ++\t\t\t\texpect=$(tail -n +6 out | wc -c) &&\n     +\t\t\t\trm -f out ;;\n      \t\t\ttree | blob)\n      \t\t\t\texpect=\"\" ;;\n-- \n2.33.0.254.g68ee769121\n\n"},{"id":"433291","messageId":"b1b9771913f310829ff142eeb1262a4e8f7e70cb.1629509531.git.congdanhqx@gmail.com","threadId":"56311","inReplyTo":"cover.1629509530.git.congdanhqx@gmail.com","subject":"[PATCH v3 1/2] t6300: don't run cat-file on non-existent object","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-21T01:36:33Z","receivedAt":"2021-08-21T01:36:54Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"In t6300, some tests are guarded behind some prerequisites.\nThus, objects created by those tests ain't available if those\nprerequisites are unsatistified.  Attempting to run \"cat-file\"\non those objects will run into failure.\n\nIn fact, running t6300 in an environment without gpg(1),\nwe'll see those warnings:\n\n\tfatal: Not a valid object name refs/tags/signed-empty\n\tfatal: Not a valid object name refs/tags/signed-short\n\tfatal: Not a valid object name refs/tags/signed-long\n\nLet's put those commands into the real tests, in order to:\n\n* skip their execution if prerequisites aren't satistified.\n* check their exit status code\n\nThe expected value for objects with type: commit needs to be\ncomputed outside the test because we can't rely on \"$3\" there.\nFurthermore, to prevent the accidental usage of that computed\nexpected value, BUG out on unknown object's type.\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/t6300-for-each-ref.sh | 27 ++++++++++++++++-----------\n 1 file changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 0d2e062f79..93126341b3 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -59,18 +59,23 @@ test_atom() {\n \t# Automatically test \"contents:size\" atom after testing \"contents\"\n \tif test \"$2\" = \"contents\"\n \tthen\n-\t\tcase $(git cat-file -t \"$ref\") in\n-\t\ttag)\n-\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n-\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n-\t\ttree | blob)\n-\t\t\texpect='' ;;\n-\t\tcommit)\n-\t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n-\t\tesac\n-\t\t# Leave $expect unquoted to lose possible leading whitespaces\n-\t\techo $expect >expected\n+\t\t# for commit leg, $3 is changed there\n+\t\texpect=$(printf '%s' \"$3\" | wc -c)\n \t\ttest_expect_${4:-success} $PREREQ \"basic atom: $1 contents:size\" '\n+\t\t\ttype=$(git cat-file -t \"$ref\") &&\n+\t\t\tcase $type in\n+\t\t\ttag)\n+\t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n+\t\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n+\t\t\ttree | blob)\n+\t\t\t\texpect=\"\" ;;\n+\t\t\tcommit)\n+\t\t\t\t: \"use the calculated expect\" ;;\n+\t\t\t*)\n+\t\t\t\tBUG \"unknown object type\" ;;\n+\t\t\tesac &&\n+\t\t\t# Leave $expect unquoted to lose possible leading whitespaces\n+\t\t\techo $expect >expected &&\n \t\t\tgit for-each-ref --format=\"%(contents:size)\" \"$ref\" >actual &&\n \t\t\ttest_cmp expected actual\n \t\t'\n-- \n2.33.0.254.g68ee769121\n\n"},{"id":"433292","messageId":"83d532528bce7cb475833a504a244aa945fe048d.1629509531.git.congdanhqx@gmail.com","threadId":"56311","inReplyTo":"cover.1629509530.git.congdanhqx@gmail.com","subject":"[PATCH v3 2/2] t6300: check for cat-file exit status code","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-08-21T01:36:34Z","receivedAt":"2021-08-21T01:36:55Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"In test_atom(), we're piping the output of cat-file to tail(1),\nthus, losing its exit status.\n\nLet's use a temporary file to preserve git exit status code.\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n\n Junio wrote:\n\n > It is not wrong per-se, but do we need a redirect '<' here?  \"tail\"\n > takes filename(s) on the command line, but is there a reason to feed\n > the contents from the standard input?\n\n Well, no reason. In v1, I replaced the left hand side of pipe with <out,\n then in v2, I moved it to the end of command.\n\n Changed to not use shell redirection.\n\n t/t6300-for-each-ref.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 93126341b3..80679d5e12 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -66,7 +66,9 @@ test_atom() {\n \t\t\tcase $type in\n \t\t\ttag)\n \t\t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n-\t\t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n+\t\t\t\tgit cat-file tag $ref >out &&\n+\t\t\t\texpect=$(tail -n +6 out | wc -c) &&\n+\t\t\t\trm -f out ;;\n \t\t\ttree | blob)\n \t\t\t\texpect=\"\" ;;\n \t\t\tcommit)\n-- \n2.33.0.254.g68ee769121\n\n"}]}