{"thread":{"id":"48455","subject":"[PATCH 1/2] t5516-fetch-push: fix 'push with dry-run' test","startedAt":"2018-05-10T14:02:08Z","lastAt":"2018-05-11T03:32:44Z","messageCount":3,"participants":["SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"347227","messageId":"20180510140154.30381-1-szeder.dev@gmail.com","threadId":"48455","inReplyTo":null,"subject":"[PATCH 1/2] t5516-fetch-push: fix 'push with dry-run' test","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-05-10T14:01:53Z","receivedAt":"2018-05-10T14:02:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"In a while-at-it cleanup replacing a 'cd dir && <...> && cd ..' with a\nsubshell, commit 28391a80a9 (receive-pack: allow deletion of corrupt\nrefs, 2007-11-29) also moved the assignment of the $old_commit\nvariable to that subshell.  This variable, however, is used outside of\nthat subshell as a parameter of check_push_result(), to check that a\nref still points to the commit where it is supposed to.  With the\nvariable remaining unset outside the subshell check_push_result()\ndoesn't perform that check at all.\n\nUse 'git -C <dir> cmd...', so we don't need to change directory, and\nthus don't need the subshell either when setting $old_commit.\n\nFurthermore, change check_push_result() to require at least three\nparameters (the repo, the oid, and at least one ref), so it will catch\nsimilar issues earlier should they ever arise.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t5516-fetch-push.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 82239138d5..832b79ad40 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -94,6 +94,9 @@ mk_child() {\n }\n \n check_push_result () {\n+\ttest $# -ge 3 ||\n+\terror \"bug in the test script: check_push_result requires at least 3 parameters\"\n+\n \trepo_name=\"$1\"\n \tshift\n \n@@ -553,10 +556,7 @@ test_expect_success 'branch.*.pushremote config order is irrelevant' '\n test_expect_success 'push with dry-run' '\n \n \tmk_test testrepo heads/master &&\n-\t(\n-\t\tcd testrepo &&\n-\t\told_commit=$(git show-ref -s --verify refs/heads/master)\n-\t) &&\n+\told_commit=$(git -C testrepo show-ref -s --verify refs/heads/master) &&\n \tgit push --dry-run testrepo : &&\n \tcheck_push_result testrepo $old_commit heads/master\n '\n-- \n2.17.0.756.gcf614c5aff\n\n"},{"id":"347228","messageId":"20180510140154.30381-2-szeder.dev@gmail.com","threadId":"48455","inReplyTo":"20180510140154.30381-1-szeder.dev@gmail.com","subject":"[PATCH 2/2] t5516-fetch-push: fix broken &&-chain","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-05-10T14:01:54Z","receivedAt":"2018-05-10T14:02:10Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"b2dc968e60 (t5516: refactor oddball tests, 2008-11-07) accidentaly\nbroke the &&-chain in the test 'push does not update local refs on\nfailure', but since it was in a subshell, chain-lint couldn't notice\nit.\n\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n t/t5516-fetch-push.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 832b79ad40..3e8940eee5 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -612,7 +612,7 @@ test_expect_success 'push does not update local refs on failure' '\n \tchmod +x testrepo/.git/hooks/pre-receive &&\n \t(\n \t\tcd child &&\n-\t\tgit pull .. master\n+\t\tgit pull .. master &&\n \t\ttest_must_fail git push &&\n \t\ttest $(git rev-parse master) != \\\n \t\t\t$(git rev-parse remotes/origin/master)\n-- \n2.17.0.756.gcf614c5aff\n\n"},{"id":"347330","messageId":"xmqqmux6hnop.fsf@gitster-ct.c.googlers.com","threadId":"48455","inReplyTo":"20180510140154.30381-1-szeder.dev@gmail.com","subject":"Re: [PATCH 1/2] t5516-fetch-push: fix 'push with dry-run' test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-11T03:32:38Z","receivedAt":"2018-05-11T03:32:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> In a while-at-it cleanup replacing a 'cd dir && <...> && cd ..' with a\n> subshell, commit 28391a80a9 (receive-pack: allow deletion of corrupt\n> refs, 2007-11-29) also moved the assignment of the $old_commit\n> variable to that subshell.  This variable, however, is used outside of\n> that subshell as a parameter of check_push_result(), to check that a\n> ref still points to the commit where it is supposed to.  With the\n> variable remaining unset outside the subshell check_push_result()\n> doesn't perform that check at all.\n\nSigh/Blush.  Thanks for finding an old screw-up.\n\n>\n> Use 'git -C <dir> cmd...', so we don't need to change directory, and\n> thus don't need the subshell either when setting $old_commit.\n>\n> Furthermore, change check_push_result() to require at least three\n> parameters (the repo, the oid, and at least one ref), so it will catch\n> similar issues earlier should they ever arise.\n>\n> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> ---\n>  t/t5516-fetch-push.sh | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index 82239138d5..832b79ad40 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -94,6 +94,9 @@ mk_child() {\n>  }\n>  \n>  check_push_result () {\n> +\ttest $# -ge 3 ||\n> +\terror \"bug in the test script: check_push_result requires at least 3 parameters\"\n> +\n>  \trepo_name=\"$1\"\n>  \tshift\n>  \n> @@ -553,10 +556,7 @@ test_expect_success 'branch.*.pushremote config order is irrelevant' '\n>  test_expect_success 'push with dry-run' '\n>  \n>  \tmk_test testrepo heads/master &&\n> -\t(\n> -\t\tcd testrepo &&\n> -\t\told_commit=$(git show-ref -s --verify refs/heads/master)\n> -\t) &&\n> +\told_commit=$(git -C testrepo show-ref -s --verify refs/heads/master) &&\n>  \tgit push --dry-run testrepo : &&\n>  \tcheck_push_result testrepo $old_commit heads/master\n>  '\n"}]}