{"thread":{"id":"51187","subject":"[PATCH 0/2] request-pull: warn if the remote object is not the same as the local one","startedAt":"2019-05-28T10:15:49Z","lastAt":"2019-05-28T10:15:53Z","messageCount":3,"participants":["Paolo Bonzini"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"376286","messageId":"20190528101543.16094-1-bonzini@gnu.org","threadId":"51187","inReplyTo":null,"subject":"[PATCH 0/2] request-pull: warn if the remote object is not the same as the local one","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2019-05-28T10:15:41Z","receivedAt":"2019-05-28T10:15:49Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nIn some cases, git request-pull might be invoked with remote and\nlocal objects that differ even though they point to the same commit.\nFor example, the remote object might be a lightweight tag\nvs. an annotated tag on the local side, or the user might have\nreworded the tag locally and forgotten to push it.\n\nWhen this happens git-request-pull will not warn, because it only\nchecks that \"git ls-remote\" returns an SHA1 that matches the local\ncommit.  Patch 2 of this series makes git-request-pull remember the tag\nobject's SHA1 while processing the \"git ls-remote\" output, so that it\ncan be matched against the local object.\n    \nPaolo Bonzini (2):\n  request-pull: quote metacharacters in local ref\n  request-pull: warn if the remote object is not the same as the local one\n\n git-request-pull.sh     | 46 ++++++++++++++++++++++-------------\n t/t5150-request-pull.sh | 53 +++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 82 insertions(+), 17 deletions(-)\n\n-- \n2.21.0\n\n"},{"id":"376287","messageId":"20190528101543.16094-2-bonzini@gnu.org","threadId":"51187","inReplyTo":"20190528101543.16094-1-bonzini@gnu.org","subject":"[PATCH 1/2] request-pull: quote regex metacharacters in local ref","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2019-05-28T10:15:42Z","receivedAt":"2019-05-28T10:15:51Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nThe local part of the third argument of git-request-pull is used in\na regular expression without quoting it.  Use qr{} and \\Q\\E to ensure\nthat e.g. a period in a tag name does not match any character on the\nremote side.\n\nSigned-off-by: Paolo Bonzini <pbonzini@redhat.com>\n---\n git-request-pull.sh     |  5 ++---\n t/t5150-request-pull.sh | 18 ++++++++++++++++++\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex 13c172bd94..0d128be7fd 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -83,19 +83,18 @@ die \"fatal: No commits in common between $base and $head\"\n # Otherwise find a random ref that matches $headrev.\n find_matching_ref='\n \tmy ($head,$headrev) = (@ARGV);\n+\tmy $pattern = qr{/\\Q$head\\E$};\n \tmy ($found);\n \n \twhile (<STDIN>) {\n \t\tchomp;\n \t\tmy ($sha1, $ref, $deref) = /^(\\S+)\\s+([^^]+)(\\S*)$/;\n-\t\tmy ($pattern);\n \t\tnext unless ($sha1 eq $headrev);\n \n-\t\t$pattern=\"/$head\\$\";\n \t\tif ($ref eq $head) {\n \t\t\t$found = $ref;\n \t\t}\n-\t\tif ($ref =~ /$pattern/) {\n+\t\tif ($ref =~ $pattern) {\n \t\t\t$found = $ref;\n \t\t}\n \t\tif ($sha1 eq $head) {\ndiff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\nindex fca001eb9b..c1a821a549 100755\n--- a/t/t5150-request-pull.sh\n+++ b/t/t5150-request-pull.sh\n@@ -246,4 +246,22 @@ test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '\n \n '\n \n+test_expect_success 'request-pull quotes regex metacharacters properly' '\n+\n+\trm -fr downstream.git &&\n+\tgit init --bare downstream.git &&\n+\t(\n+\t\tcd local &&\n+\t\tgit checkout initial &&\n+\t\tgit merge --ff-only master &&\n+\t\tgit tag -mrelease v2.0 &&\n+\t\tgit push origin refs/tags/v2.0:refs/tags/v2-0 &&\n+\t\ttest_must_fail git request-pull initial \"$downstream_url\" tags/v2.0 \\\n+\t\t\t2>../err\n+\t) &&\n+\tgrep \"No match for commit .*\" err &&\n+\tgrep \"Are you sure you pushed\" err\n+\n+'\n+\n test_done\n-- \n2.21.0\n\n\n"},{"id":"376288","messageId":"20190528101543.16094-3-bonzini@gnu.org","threadId":"51187","inReplyTo":"20190528101543.16094-1-bonzini@gnu.org","subject":"[PATCH 2/2] request-pull: warn if the remote object is not the same as the local one","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2019-05-28T10:15:43Z","receivedAt":"2019-05-28T10:15:53Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"From: Paolo Bonzini <pbonzini@redhat.com>\n\nIn some cases, git request-pull might be invoked with remote and\nlocal objects that differ even though they point to the same commit.\nFor example, the remote object might be a lightweight tag\nvs. an annotated tag on the local side; or the user might have\nreworded the tag locally and forgotten to push it.\n\nWhen this happens git-request-pull will not warn, because it only\nchecks that \"git ls-remote\" returns an SHA1 that matches the local\ncommit (known as $headrev in the script).  This patch makes\ngit-request-pull retrieve the tag object SHA1 while processing\nthe \"git ls-remote\" output, so that it can be matched against the\nlocal object.\n\nSigned-off-by: Paolo Bonzini <pbonzini@redhat.com>\n---\n git-request-pull.sh     | 43 +++++++++++++++++++++++++++--------------\n t/t5150-request-pull.sh | 35 +++++++++++++++++++++++++++++++++\n 2 files changed, 63 insertions(+), 15 deletions(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex 0d128be7fd..2d0e44656c 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -65,6 +65,8 @@ test -z \"$head\" && die \"fatal: Not a valid revision: $local\"\n headrev=$(git rev-parse --verify --quiet \"$head\"^0)\n test -z \"$headrev\" && die \"fatal: Ambiguous revision: $local\"\n \n+local_sha1=$(git rev-parse --verify --quiet \"$head\")\n+\n # Was it a branch with a description?\n branch_name=${head#refs/heads/}\n if test \"z$branch_name\" = \"z$headref\" ||\n@@ -77,42 +79,53 @@ merge_base=$(git merge-base $baserev $headrev) ||\n die \"fatal: No commits in common between $base and $head\"\n \n # $head is the refname from the command line.\n-# If a ref with the same name as $head exists at the remote\n-# and their values match, use that.\n-#\n-# Otherwise find a random ref that matches $headrev.\n+# Find a ref with the same name as $head that exists at the remote\n+# and points to the same commit as the local object.\n find_matching_ref='\n \tmy ($head,$headrev) = (@ARGV);\n \tmy $pattern = qr{/\\Q$head\\E$};\n-\tmy ($found);\n+\tmy ($remote_sha1, $found);\n \n \twhile (<STDIN>) {\n \t\tchomp;\n \t\tmy ($sha1, $ref, $deref) = /^(\\S+)\\s+([^^]+)(\\S*)$/;\n-\t\tnext unless ($sha1 eq $headrev);\n \n-\t\tif ($ref eq $head) {\n-\t\t\t$found = $ref;\n-\t\t}\n-\t\tif ($ref =~ $pattern) {\n-\t\t\t$found = $ref;\n-\t\t}\n \t\tif ($sha1 eq $head) {\n-\t\t\t$found = $sha1;\n+\t\t\t$found = $remote_sha1 = $sha1;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tif ($ref eq $head || $ref =~ $pattern) {\n+\t\t\tif ($deref eq \"\") {\n+\t\t\t\t# Remember the matching object on the remote side\n+\t\t\t\t$remote_sha1 = $sha1;\n+\t\t\t}\n+\t\t\tif ($sha1 eq $headrev) {\n+\t\t\t\t$found = $ref;\n+\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n \t}\n \tif ($found) {\n-\t\tprint \"$found\\n\";\n+\t\t$remote_sha1 = $headrev if ! defined $remote_sha1;\n+\t\tprint \"$remote_sha1 $found\\n\";\n \t}\n '\n \n-ref=$(git ls-remote \"$url\" | @@PERL@@ -e \"$find_matching_ref\" \"${remote:-HEAD}\" \"$headrev\")\n+set fnord $(git ls-remote \"$url\" | @@PERL@@ -e \"$find_matching_ref\" \"${remote:-HEAD}\" \"$headrev\")\n+remote_sha1=$2\n+ref=$3\n \n if test -z \"$ref\"\n then\n \techo \"warn: No match for commit $headrev found at $url\" >&2\n \techo \"warn: Are you sure you pushed '${remote:-HEAD}' there?\" >&2\n \tstatus=1\n+elif test \"$local_sha1\" != \"$remote_sha1\"\n+then\n+\techo \"warn: $head found at $url but points to a different object\" >&2\n+\techo \"warn: Are you sure you pushed '${remote:-HEAD}' there?\" >&2\n+\tstatus=1\n fi\n \n # Special case: turn \"for_linus\" to \"tags/for_linus\" when it is correct\ndiff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\nindex c1a821a549..852dcd913f 100755\n--- a/t/t5150-request-pull.sh\n+++ b/t/t5150-request-pull.sh\n@@ -264,4 +264,39 @@ test_expect_success 'request-pull quotes regex metacharacters properly' '\n \n '\n \n+test_expect_success 'pull request with mismatched object' '\n+\n+\trm -fr downstream.git &&\n+\tgit init --bare downstream.git &&\n+\t(\n+\t\tcd local &&\n+\t\tgit checkout initial &&\n+\t\tgit merge --ff-only master &&\n+\t\tgit push origin HEAD:refs/tags/full &&\n+\t\ttest_must_fail git request-pull initial \"$downstream_url\" tags/full \\\n+\t\t\t2>../err\n+\t) &&\n+\tgrep \"points to a different object\" err &&\n+\tgrep \"Are you sure you pushed\" err\n+\n+'\n+\n+test_expect_success 'pull request with stale object' '\n+\n+\trm -fr downstream.git &&\n+\tgit init --bare downstream.git &&\n+\t(\n+\t\tcd local &&\n+\t\tgit checkout initial &&\n+\t\tgit merge --ff-only master &&\n+\t\tgit push origin refs/tags/full &&\n+\t\tgit tag -f -m\"Thirty-one days\" full &&\n+\t\ttest_must_fail git request-pull initial \"$downstream_url\" tags/full \\\n+\t\t\t2>../err\n+\t) &&\n+\tgrep \"points to a different object\" err &&\n+\tgrep \"Are you sure you pushed\" err\n+\n+'\n+\n test_done\n-- \n2.21.0\n\n"}]}