{"thread":{"id":"41886","subject":"[PATCH v4] git-send-pack: fix --all option when used with directory","startedAt":"2016-03-31T13:55:09Z","lastAt":"2016-04-01T10:44:41Z","messageCount":6,"participants":["stanislav@assembla.com","Junio C Hamano","Eric Sunshine","Jeff King","Stanislav Kolotinskiy"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"282305","messageId":"1459432509-12934-1-git-send-email-stanislav@assembla.com","threadId":"41886","inReplyTo":null,"subject":"[PATCH v4] git-send-pack: fix --all option when used with directory","fromName":"","fromEmail":"stanislav@assembla.com","sentAt":"2016-03-31T13:55:09Z","receivedAt":"2016-03-31T13:55:09Z","isPatch":true,"sender":{"key":"stanislav@assembla.com","avatar":"https://gravatar.com/avatar/dca9d54f97b9c9bb1c41538bcc64a933d063c1f8fe3cac0936a4d4b09e4c45ff?d=mp&s=160"},"body":"From: Stanislav Kolotinskiy \n\nWhen using git send-pack with --all option\nand a target repository specification ([:]),\nusage message is being displayed instead of performing\nthe actual transmission.\n\nThe reason for this issue is that destination and refspecs are being set\nin the same conditional and are populated from argv. When a target\nrepository is passed, refspecs is being populated as well with its value.\nThis makes the check for refspecs not being NULL to always return true,\nwhich, in conjunction with the check for --all or --mirror options,\nis always true as well and returns usage message instead of proceeding.\n\nThis ensures that send-pack will stop execution only when --all\nor --mirror switch is used in conjunction with any refspecs passed.\n\nSigned-off-by: Stanislav Kolotinskiy \n---\n builtin/send-pack.c | 2 +-\n t/t5400-send-pack.sh | 12 ++++++++++++\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex f6e5d64..19f0577 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -225,7 +225,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t * --all and --mirror are incompatible; neither makes sense\n \t * with any refspecs.\n \t */\n-\tif ((refspecs && (send_all || args.send_mirror)) ||\n+\tif ((nr_refspecs > 0 && (send_all || args.send_mirror)) ||\n \t (send_all && args.send_mirror))\n \t\tusage_with_options(send_pack_usage, options);\n\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 04cea97..305ca7a 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -128,6 +128,18 @@ test_expect_success 'denyNonFastforwards trumps --force' '\n \ttest \"$victim_orig\" = \"$victim_head\"\n '\n\n+test_expect_success 'send-pack --all sends all branches' '\n+\t# make sure we have at least 2 branches with different\n+\t# values, just to be thorough\n+\tgit branch other-branch HEAD^ &&\n+\n+\tgit init --bare all.git &&\n+\tgit send-pack --all all.git &&\n+\tgit for-each-ref refs/heads >expect &&\n+\tgit -C all.git for-each-ref refs/heads >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'push --all excludes remote-tracking hierarchy' '\n \tmkdir parent &&\n \t(\n--\n2.8.0\n"},{"id":"282385","messageId":"xmqq1t6qmlxk.fsf@gitster.mtv.corp.google.com","threadId":"41886","inReplyTo":"1459432509-12934-1-git-send-email-stanislav@assembla.com","subject":"Re: [PATCH v4] git-send-pack: fix --all option when used with directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-31T20:28:39Z","receivedAt":"2016-03-31T20:28:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"stanislav@assembla.com writes:\n\n> From: Stanislav Kolotinskiy \n>\n> When using git send-pack with --all option\n> and a target repository specification ([:]),\n> usage message is being displayed instead of performing\n> the actual transmission.\n>\n> The reason for this issue is that destination and refspecs are being set\n> in the same conditional and are populated from argv. When a target\n> repository is passed, refspecs is being populated as well with its value.\n> This makes the check for refspecs not being NULL to always return true,\n> which, in conjunction with the check for --all or --mirror options,\n> is always true as well and returns usage message instead of proceeding.\n>\n> This ensures that send-pack will stop execution only when --all\n> or --mirror switch is used in conjunction with any refspecs passed.\n>\n> Signed-off-by: Stanislav Kolotinskiy \n> ---\n\nThanks, will queue.\n\n> builtin/send-pack.c | 2 +-\n> t/t5400-send-pack.sh | 12 ++++++++++++\n> 2 files changed, 13 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/send-pack.c b/builtin/send-pack.c\n> index f6e5d64..19f0577 100644\n> --- a/builtin/send-pack.c\n> +++ b/builtin/send-pack.c\n> @@ -225,7 +225,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n> \t * --all and --mirror are incompatible; neither makes sense\n> \t * with any refspecs.\n> \t */\n> -\tif ((refspecs && (send_all || args.send_mirror)) ||\n> +\tif ((nr_refspecs > 0 && (send_all || args.send_mirror)) ||\n> \t (send_all && args.send_mirror))\n> \t\tusage_with_options(send_pack_usage, options);\n>\n> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\n> index 04cea97..305ca7a 100755\n> --- a/t/t5400-send-pack.sh\n> +++ b/t/t5400-send-pack.sh\n> @@ -128,6 +128,18 @@ test_expect_success 'denyNonFastforwards trumps --force' '\n> \ttest \"$victim_orig\" = \"$victim_head\"\n> '\n>\n> +test_expect_success 'send-pack --all sends all branches' '\n> +\t# make sure we have at least 2 branches with different\n> +\t# values, just to be thorough\n> +\tgit branch other-branch HEAD^ &&\n> +\n> +\tgit init --bare all.git &&\n> +\tgit send-pack --all all.git &&\n> +\tgit for-each-ref refs/heads >expect &&\n> +\tgit -C all.git for-each-ref refs/heads >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_expect_success 'push --all excludes remote-tracking hierarchy' '\n> \tmkdir parent &&\n> \t(\n> --\n> 2.8.0\n"},{"id":"282399","messageId":"xmqqoa9ul30c.fsf@gitster.mtv.corp.google.com","threadId":"41886","inReplyTo":"xmqq1t6qmlxk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4] git-send-pack: fix --all option when used with directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-31T22:02:43Z","receivedAt":"2016-03-31T22:02:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> stanislav@assembla.com writes:\n>\n>> From: Stanislav Kolotinskiy \n>>\n>> When using git send-pack with --all option\n>> and a target repository specification ([:]),\n>> usage message is being displayed instead of performing\n>> the actual transmission.\n>>\n>> The reason for this issue is that destination and refspecs are being set\n>> in the same conditional and are populated from argv. When a target\n>> repository is passed, refspecs is being populated as well with its value.\n>> This makes the check for refspecs not being NULL to always return true,\n>> which, in conjunction with the check for --all or --mirror options,\n>> is always true as well and returns usage message instead of proceeding.\n>>\n>> This ensures that send-pack will stop execution only when --all\n>> or --mirror switch is used in conjunction with any refspecs passed.\n>>\n>> Signed-off-by: Stanislav Kolotinskiy \n>> ---\n>\n> Thanks, will queue.\n\nBy the way, for some reason it was unusually painful to find the\nexact breakage by bisecting between maint-2.4 and maint-2.6. It\nsomehow ended up on fingering random places like v2.6.0 itself.\n\nThe true culprit is 068c77a5 (builtin/send-pack.c: use parse_options\nAPI, 2015-08-19). I didn't dug deep enough to tell if we recently\nbroke \"git bisect\" or if there are something wrong in the shape of\nmy history.\n"},{"id":"282407","messageId":"CAPig+cS-Yk7Ey8_+waG7HqG-9BijrQNrtP+0szcgbmVycv_qvQ@mail.gmail.com","threadId":"41886","inReplyTo":"xmqqoa9ul30c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4] git-send-pack: fix --all option when used with directory","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-31T22:55:22Z","receivedAt":"2016-03-31T22:55:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Mar 31, 2016 at 6:02 PM, Junio C Hamano wrote:\n> By the way, for some reason it was unusually painful to find the\n> exact breakage by bisecting between maint-2.4 and maint-2.6. It\n> somehow ended up on fingering random places like v2.6.0 itself.\n>\n> The true culprit is 068c77a5 (builtin/send-pack.c: use parse_options\n> API, 2015-08-19). I didn't dug deep enough to tell if we recently\n> broke \"git bisect\" or if there are something wrong in the shape of\n> my history.\n\nI had a similar experience a couple months ago where I was bisecting\nto find a fix (rather than breakage), and each time I ran bisect, it\nseemed (randomly) to arrive at a different commit, often a merge,\nrather than the real fix (which I eventually located by manually\nexamining commits near the commits bisect identified). At the time, I\nthought I was doing something wrong (and perhaps I was), but your\ndescriptions sounds eerily similar to that experience.\n\nUnfortunately, I no longer recall precisely for what I was searching\n(other than that someone reported a Git bug which I recalled having\nbeen already fixed, and wanted to use bisect to respond with the exact\ncommit which fixed the problem), and the bisect \"run\" script was\nthrowaway, so I can't consult that for further details.\n"},{"id":"282411","messageId":"20160331232623.GA6120@sigill.intra.peff.net","threadId":"41886","inReplyTo":"xmqqoa9ul30c.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4] git-send-pack: fix --all option when used with directory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-31T23:26:23Z","receivedAt":"2016-03-31T23:26:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 31, 2016 at 03:02:43PM -0700, Junio C Hamano wrote:\n\n> By the way, for some reason it was unusually painful to find the\n> exact breakage by bisecting between maint-2.4 and maint-2.6. It\n> somehow ended up on fingering random places like v2.6.0 itself.\n> \n> The true culprit is 068c77a5 (builtin/send-pack.c: use parse_options\n> API, 2015-08-19). I didn't dug deep enough to tell if we recently\n> broke \"git bisect\" or if there are something wrong in the shape of\n> my history.\n\nWeird. I bisected this when v1 was published, and didn't have any\ntrouble. I guess it's possible I just got lucky in where I landed,\nthough, if I started at different endpoints than you.\n\n-Peff\n"},{"id":"282442","messageId":"56FE5119.5000502@assembla.com","threadId":"41886","inReplyTo":"xmqq1t6qmlxk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4] git-send-pack: fix --all option when used with directory","fromName":"Stanislav Kolotinskiy","fromEmail":"stanislav@assembla.com","sentAt":"2016-04-01T10:44:41Z","receivedAt":"2016-04-01T10:44:41Z","isPatch":true,"sender":{"key":"stanislav@assembla.com","avatar":"https://gravatar.com/avatar/dca9d54f97b9c9bb1c41538bcc64a933d063c1f8fe3cac0936a4d4b09e4c45ff?d=mp&s=160"},"body":"On 31/03/16 23:28, Junio C Hamano wrote:\n> Thanks, will queue. \n\nThanks a lot!\n\n--\nRegards,\nStanislav\n"}]}