{"thread":{"id":"23588","subject":"[PATCH 0/2] request-pull: protect against OPTIONS_KEEPDASHDASH from environment","startedAt":"2010-04-24T12:10:10Z","lastAt":"2010-04-26T00:12:56Z","messageCount":6,"participants":["Jonathan Nieder","Thomas Rast","Miklos Vajna"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"140266","messageId":"20100424120437.GA12237@progeny.tock","threadId":"23588","inReplyTo":null,"subject":"[PATCH 0/2] request-pull: protect against OPTIONS_KEEPDASHDASH from environment","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T12:10:10Z","receivedAt":"2010-04-24T12:10:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nAt last, the mysterious t7400 test failure has been solved, thanks to\nGerrit Pape.  It was caused by an assumption in git that a certain\nvariable not in its namespace would not be set.\n\nHere’s a problem of the same kind, though harder to trip.  I can’t\nimagine why anyone would be exporting the OPTIONS_KEEPDASHDASH\nvariable, but it is not in git’s namespace so it seems better not\nto make any assumptions.\n\nJonathan Nieder (2):\n  tests for request-pull\n  request-pull: protect against OPTIONS_KEEPDASHDASH from environment\n\n contrib/git-resurrect.sh |    1 +\n git-request-pull.sh      |    1 +\n t/t5150-request-pull.sh  |  228 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 230 insertions(+), 0 deletions(-)\n create mode 100644 t/t5150-request-pull.sh\n"},{"id":"140267","messageId":"20100424121151.GA12312@progeny.tock","threadId":"23588","inReplyTo":"20100424120437.GA12237@progeny.tock","subject":"[PATCH 1/2] tests for request-pull","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T12:11:51Z","receivedAt":"2010-04-24T12:11:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Test that request-pull handles failure to push cleanly, writes\npull requests that produce the correct effect when followed, and\nuses a predictable format.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n t/t5150-request-pull.sh |  214 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 214 insertions(+), 0 deletions(-)\n create mode 100644 t/t5150-request-pull.sh\n\ndiff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\nnew file mode 100644\nindex 0000000..e012a36\n--- /dev/null\n+++ b/t/t5150-request-pull.sh\n@@ -0,0 +1,214 @@\n+#!/bin/sh\n+\n+test_description='Test workflows involving pull request.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\n+\tgit init --bare upstream.git &&\n+\tgit init --bare downstream.git &&\n+\tgit clone upstream.git upstream-private &&\n+\tgit clone downstream.git local &&\n+\n+\ttrash_url=\"file://$TRASH_DIRECTORY\" &&\n+\tdownstream_url=\"$trash_url/downstream.git/\" &&\n+\tupstream_url=\"$trash_url/upstream.git/\" &&\n+\n+\t(\n+\t\tcd upstream-private &&\n+\t\tcat <<-\\EOT >mnemonic.txt &&\n+\t\tThirtey days hath November,\n+\t\tAprile, June, and September:\n+\t\tEOT\n+\t\tgit add mnemonic.txt &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"\\\"Thirty days\\\", a reminder of month lengths\" &&\n+\t\tgit tag -m \"version 1\" -a initial &&\n+\t\tgit push --tags origin master\n+\t) &&\n+\t(\n+\t\tcd local &&\n+\t\tgit remote add upstream \"$trash_url/upstream.git\" &&\n+\t\tgit fetch upstream &&\n+\t\tgit pull upstream master &&\n+\t\tcat <<-\\EOT >>mnemonic.txt &&\n+\t\tOf twyecescore-eightt is but eine,\n+\t\tAnd all the remnante be thrycescore-eine.\n+\t\tO’course Leap yare comes an’pynes,\n+\t\tEv’rie foure yares, gote it ryghth.\n+\t\tAn’twyecescore-eight is but twyecescore-nyne.\n+\t\tEOT\n+\t\tgit add mnemonic.txt &&\n+\t\ttest_tick &&\n+\t\tgit commit -m \"More detail\" &&\n+\t\tgit tag -m \"version 2\" -a full &&\n+\t\tgit checkout -b simplify HEAD^ &&\n+\t\tmv mnemonic.txt mnemonic.standard &&\n+\t\tcat <<-\\EOT >mnemonic.clarified &&\n+\t\tThirty days has September,\n+\t\tAll the rest I can’t remember.\n+\t\tEOT\n+\t\tgit add -N mnemonic.standard mnemonic.clarified &&\n+\t\tgit commit -a -m \"Adapt to use modern, simpler English\n+\n+But keep the old version, too, in case some people prefer it.\" &&\n+\t\tgit checkout master\n+\t)\n+\n+'\n+\n+test_expect_success 'setup: two scripts for reading pull requests' '\n+\n+\tdownstream_url_for_sed=$(\n+\t\tprintf \"%s\\n\" \"$downstream_url\" |\n+\t\tsed -e '\\''s/\\\\/\\\\\\\\/g'\\'' -e '\\''s/[[/.*^$]/\\\\&/g'\\''\n+\t) &&\n+\n+\tcat <<-\\EOT >read-request.sed &&\n+\t#!/bin/sed -nf\n+\t/ in the git repository at:$/! d\n+\tn\n+\t/^$/! q\n+\tn\n+\ts/^[ \t]*\\(.*\\) \\([^ ]*\\)/please pull\\\n+\t\\1\\\n+\t\\2/p\n+\tq\n+\tEOT\n+\n+\tcat <<-EOT >fuzz.sed\n+\t#!/bin/sed -nf\n+\ts/$_x40/OBJECT_NAME/g\n+\ts/A U Thor/AUTHOR/g\n+\ts/        [^ ].*/        SUBJECT/g\n+\ts/$downstream_url_for_sed/URL/g\n+\ts/for-upstream/BRANCH/g\n+\ts/mnemonic.txt/FILENAME/g\n+\t/^ FILENAME | *[0-9]* [-+]*\\$/ b diffstat\n+\t/^AUTHOR ([0-9]*):\\$/ b shortlog\n+\tp\n+\tb\n+\t: diffstat\n+\tn\n+\t/ [0-9]* files changed/ {\n+\t\ta\\\n+\tDIFFSTAT\n+\t\tb\n+\t}\n+\tb diffstat\n+\t: shortlog\n+\t/^        [a-zA-Z]/ n\n+\t/^[a-zA-Z]* ([0-9]*):\\$/ n\n+\t/^\\$/ N\n+\t/^\\n[a-zA-Z]* ([0-9]*):\\$/! {\n+\t\ta\\\n+\tSHORTLOG\n+\t\tD\n+\t}\n+\tn\n+\tb shortlog\n+\tEOT\n+\n+'\n+\n+test_expect_success 'pull request when forgot to push' '\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\ttest_must_fail git request-pull initial \"$downstream_url\" \\\n+\t\t\t2>../err\n+\t) &&\n+\tgrep \"No branch of.*is at:\\$\" err &&\n+\tgrep \"Are you sure you pushed\" err\n+\n+'\n+\n+test_expect_success 'pull request after push' '\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 master:for-upstream &&\n+\t\tgit request-pull initial origin >../request\n+\t) &&\n+\tsed -nf read-request.sed <request >digest &&\n+\tcat digest &&\n+\t{\n+\t\tread task &&\n+\t\tread repository &&\n+\t\tread branch\n+\t} <digest &&\n+\t(\n+\t\tcd upstream-private &&\n+\t\tgit checkout initial &&\n+\t\tgit pull --ff-only \"$repository\" \"$branch\"\n+\t) &&\n+\ttest \"$branch\" = for-upstream &&\n+\ttest_cmp local/mnemonic.txt upstream-private/mnemonic.txt\n+\n+'\n+\n+test_expect_success 'request names an appropriate branch' '\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 --tags origin master simplify &&\n+\t\tgit push origin master:for-upstream &&\n+\t\tgit request-pull initial \"$downstream_url\" >../request\n+\t) &&\n+\tsed -nf read-request.sed <request >digest &&\n+\tcat digest &&\n+\t{\n+\t\tread task &&\n+\t\tread repository &&\n+\t\tread branch\n+\t} <digest &&\n+\t{\n+\t\ttest \"$branch\" = master ||\n+\t\ttest \"$branch\" = for-upstream\n+\t}\n+\n+'\n+\n+test_expect_success 'pull request format' '\n+\n+\trm -fr downstream.git &&\n+\tgit init --bare downstream.git &&\n+\tcat <<-\\EOT >expect &&\n+\tThe following changes since commit OBJECT_NAME:\n+\t  AUTHOR (1):\n+\t        SUBJECT\n+\n+\tare available in the git repository at:\n+\n+\t  URL BRANCH\n+\n+\tSHORTLOG\n+\n+\tDIFFSTAT\n+\tEOT\n+\t(\n+\t\tcd local &&\n+\t\tgit checkout initial &&\n+\t\tgit merge --ff-only master &&\n+\t\tgit push origin master:for-upstream &&\n+\t\tgit request-pull initial \"$downstream_url\" >../request\n+\t) &&\n+\t<request sed -nf fuzz.sed >request.fuzzy &&\n+\ttest_cmp expect request.fuzzy\n+\n+'\n+\n+test_done\n-- \n1.7.1.rc1\n"},{"id":"140268","messageId":"20100424121537.GB12312@progeny.tock","threadId":"23588","inReplyTo":"20100424120437.GA12237@progeny.tock","subject":"[PATCH 2/2] request-pull: protect against OPTIONS_KEEPDASHDASH from environment","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T12:15:37Z","receivedAt":"2010-04-24T12:15:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Like most git commands, request-pull supports a -- delimiter to allow\ncallers to pass arguments that would otherwise be treated as an option\nafterwards.  The internal OPTIONS_KEEPDASHDASH variable is passed\nempty to git-sh-setup to indicate that request-pull itself does not\ncare about the position of the -- delimiter.  But if the user has\nthat variable in her environment, request-pull will see the “--” and\nfail.\n\nEmpty it explicitly to guard against this.  While at it, make the\ncorresponding fix to git-resurrect, too (all other scripts in git.git\nalready protect themselves).\n\nCc: Thomas Rast <trast@student.ethz.ch>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nForgot to mention: this series is against maint.  For master, the test\nneeds a fixup to pass with v1.7.1-rc0~144^2 (request-pull: avoid\nmentioning that the start point is a single commit, 2010-01-29), which\nI’ll send as a reply.\n\nThanks for reading.  As always, thoughts and simplifying suggestions\nare welcome.\n\n contrib/git-resurrect.sh |    1 +\n git-request-pull.sh      |    1 +\n t/t5150-request-pull.sh  |   14 ++++++++++++++\n 3 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh\nindex c364dda..a4ed4c3 100755\n--- a/contrib/git-resurrect.sh\n+++ b/contrib/git-resurrect.sh\n@@ -9,6 +9,7 @@ other/Merge <other> into <name> (respectively) commit subjects, which\n is rather slow but allows you to resurrect other people's topic\n branches.\"\n \n+OPTIONS_KEEPDASHDASH=\n OPTIONS_SPEC=\"\\\n git resurrect $USAGE\n --\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex 630cedd..b0a0311 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -8,6 +8,7 @@ USAGE='<start> <url> [<end>]'\n LONG_USAGE='Summarizes the changes between two commits to the standard output,\n and includes the given URL in the generated summary.'\n SUBDIRECTORY_OK='Yes'\n+OPTIONS_KEEPDASHDASH=\n OPTIONS_SPEC='git request-pull [options] start url [end]\n --\n p    show patch text as well\ndiff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\nindex e012a36..4c7f48a 100644\n--- a/t/t5150-request-pull.sh\n+++ b/t/t5150-request-pull.sh\n@@ -211,4 +211,18 @@ test_expect_success 'pull request format' '\n \n '\n \n+test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '\n+\n+\t(\n+\t\tcd local &&\n+\t\tOPTIONS_KEEPDASHDASH=Yes &&\n+\t\texport OPTIONS_KEEPDASHDASH &&\n+\t\tgit checkout initial &&\n+\t\tgit merge --ff-only master &&\n+\t\tgit push origin master:for-upstream &&\n+\t\tgit request-pull -- initial \"$downstream_url\" >../request\n+\t)\n+\n+'\n+\n test_done\n-- \n1.7.1.rc1\n"},{"id":"140269","messageId":"20100424122952.GA14196@progeny.tock","threadId":"23588","inReplyTo":"20100424121151.GA12312@progeny.tock","subject":"[PATCH] adapt request-pull tests for new pull request format","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T12:29:52Z","receivedAt":"2010-04-24T12:29:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"10eb0007 (request-pull: avoid mentioning that the start point is a\nsingle commit, 2010-01-29), changed the pull request format, so the\ntest needs some changes to still pass:\n\n - tolerate a missing blank line between “in the git repository at:”\n   and the name of repository and branch\n\n - recognize subject and date in the new request format\n\n - update the expected request template to match the new format\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n t/t5150-request-pull.sh |   10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\nindex 4c7f48a..b31b828 100644\n--- a/t/t5150-request-pull.sh\n+++ b/t/t5150-request-pull.sh\n@@ -69,8 +69,7 @@ test_expect_success 'setup: two scripts for reading pull requests' '\n \t#!/bin/sed -nf\n \t/ in the git repository at:$/! d\n \tn\n-\t/^$/! q\n-\tn\n+\t/^$/ n\n \ts/^[ \t]*\\(.*\\) \\([^ ]*\\)/please pull\\\n \t\\1\\\n \t\\2/p\n@@ -81,7 +80,9 @@ test_expect_success 'setup: two scripts for reading pull requests' '\n \t#!/bin/sed -nf\n \ts/$_x40/OBJECT_NAME/g\n \ts/A U Thor/AUTHOR/g\n+\ts/[-0-9]\\{10\\} [:0-9]\\{8\\} [-+][0-9]\\{4\\}/DATE/g\n \ts/        [^ ].*/        SUBJECT/g\n+\ts/  [^ ].* (DATE)/  SUBJECT (DATE)/g\n \ts/$downstream_url_for_sed/URL/g\n \ts/for-upstream/BRANCH/g\n \ts/mnemonic.txt/FILENAME/g\n@@ -188,11 +189,10 @@ test_expect_success 'pull request format' '\n \tgit init --bare downstream.git &&\n \tcat <<-\\EOT >expect &&\n \tThe following changes since commit OBJECT_NAME:\n-\t  AUTHOR (1):\n-\t        SUBJECT\n \n-\tare available in the git repository at:\n+\t  SUBJECT (DATE)\n \n+\tare available in the git repository at:\n \t  URL BRANCH\n \n \tSHORTLOG\n-- \n1.7.1.rc1\n"},{"id":"140336","messageId":"201004251434.29296.trast@student.ethz.ch","threadId":"23588","inReplyTo":"20100424121537.GB12312@progeny.tock","subject":"Re: [PATCH 2/2] request-pull: protect against OPTIONS_KEEPDASHDASH from environment","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-04-25T12:34:28Z","receivedAt":"2010-04-25T12:34:28Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jonathan Nieder wrote:\n> Empty it explicitly to guard against this.  While at it, make the\n> corresponding fix to git-resurrect, too (all other scripts in git.git\n> already protect themselves).\n\nAck.  Thanks for thinking of contrib :-)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"140373","messageId":"20100426001256.GJ19595@genesis.frugalware.org","threadId":"23588","inReplyTo":"20100424120437.GA12237@progeny.tock","subject":"Re: [PATCH 0/2] request-pull: protect against OPTIONS_KEEPDASHDASH from environment","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2010-04-26T00:12:56Z","receivedAt":"2010-04-26T00:12:56Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sat, Apr 24, 2010 at 07:10:10AM -0500, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> At last, the mysterious t7400 test failure has been solved, thanks to\n> Gerrit Pape.  It was caused by an assumption in git that a certain\n> variable not in its namespace would not be set.\n> \n> Here???s a problem of the same kind, though harder to trip.  I can???t\n> imagine why anyone would be exporting the OPTIONS_KEEPDASHDASH\n> variable, but it is not in git???s namespace so it seems better not\n> to make any assumptions.\n\nAck - I really just did cosmetics on request-pull's output, nothing\nmore. :)\n"}]}