{"thread":{"id":"22526","subject":"[PATCH] fix an error message in git-push so it goes to stderr","startedAt":"2010-02-05T00:41:40Z","lastAt":"2010-02-05T20:30:45Z","messageCount":14,"participants":["Larry D'Anna","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"133649","messageId":"20100205004140.GA2841@cthulhu","threadId":"22526","inReplyTo":null,"subject":"[PATCH] fix an error message in git-push so it goes to stderr","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T00:41:40Z","receivedAt":"2010-02-05T00:41:40Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"Having it go to standard output interferes with git-push --porcelain.\n---\n builtin-push.c | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-push.c b/builtin-push.c\nindex 5633f0a..0a27072 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -124,9 +124,9 @@ static int push_with_options(struct transport *transport, int flags)\n \t\treturn 0;\n \n \tif (nonfastforward && advice_push_nonfastforward) {\n-\t\tprintf(\"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n-\t\t \"Merge the remote changes before pushing again. See the 'Note about\\n\"\n-\t\t \"fast-forwards' section of 'git push --help' for details.\\n\");\n+\t\tfprintf(stderr, \"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n+\t\t\t\t\"Merge the remote changes before pushing again. See the 'Note about\\n\"\n+\t\t\t\t\"fast-forwards' section of 'git push --help' for details.\\n\");\n \t}\n \n \treturn 1;\n-- \n1.7.0.rc1.33.g07cf0f.dirty\n"},{"id":"133683","messageId":"20100205150638.GB14116@coredump.intra.peff.net","threadId":"22526","inReplyTo":"20100205004140.GA2841@cthulhu","subject":"Re: [PATCH] fix an error message in git-push so it goes to stderr","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-05T15:06:38Z","receivedAt":"2010-02-05T15:06:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 04, 2010 at 07:41:40PM -0500, Larry D'Anna wrote:\n\n> Having it go to standard output interferes with git-push --porcelain.\n> ---\n> builtin-push.c | 6 +++---\n> 1 files changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/builtin-push.c b/builtin-push.c\n> index 5633f0a..0a27072 100644\n> --- a/builtin-push.c\n> +++ b/builtin-push.c\n> @@ -124,9 +124,9 @@ static int push_with_options(struct transport *transport, int flags)\n> \t\treturn 0;\n> \n> \tif (nonfastforward && advice_push_nonfastforward) {\n> -\t\tprintf(\"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n> -\t\t \"Merge the remote changes before pushing again. See the 'Note about\\n\"\n> -\t\t \"fast-forwards' section of 'git push --help' for details.\\n\");\n> +\t\tfprintf(stderr, \"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n> +\t\t\t\t\"Merge the remote changes before pushing again. See the 'Note about\\n\"\n> +\t\t\t\t\"fast-forwards' section of 'git push --help' for details.\\n\");\n\nI agree that stderr is a more sensible place for such a message to go,\nbut shouldn't the porcelain output format just suppress it entirely? The\nwhole point of it is to be machine readable, and this text is\n\n a. formatted for humans\n\n b. totally redundant with the machine-readable information presented\n earlier\n\n-Peff\n"},{"id":"133703","messageId":"1265398462-17316-1-git-send-email-larry@elder-gods.org","threadId":"22526","inReplyTo":"20100205150638.GB14116@coredump.intra.peff.net","subject":"[PATCH 1/3] fix an error message in git-push so it goes to stderr","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T19:34:20Z","receivedAt":"2010-02-05T19:34:20Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"Having it go to standard output interferes with git-push --porcelain.\n\nSigned-off-by: Larry D'Anna \n---\n builtin-push.c | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-push.c b/builtin-push.c\nindex 5633f0a..0a27072 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -124,9 +124,9 @@ static int push_with_options(struct transport *transport, int flags)\n \t\treturn 0;\n \n \tif (nonfastforward && advice_push_nonfastforward) {\n-\t\tprintf(\"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n-\t\t \"Merge the remote changes before pushing again. See the 'Note about\\n\"\n-\t\t \"fast-forwards' section of 'git push --help' for details.\\n\");\n+\t\tfprintf(stderr, \"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n+\t\t\t\t\"Merge the remote changes before pushing again. See the 'Note about\\n\"\n+\t\t\t\t\"fast-forwards' section of 'git push --help' for details.\\n\");\n \t}\n \n \treturn 1;\n-- \n1.7.0.rc1.33.g07cf0f.dirty\n"},{"id":"133702","messageId":"1265398462-17316-2-git-send-email-larry@elder-gods.org","threadId":"22526","inReplyTo":"20100205150638.GB14116@coredump.intra.peff.net","subject":"[PATCH 2/3] silence human readable info messages going to stderr from git push --porcelain","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T19:34:21Z","receivedAt":"2010-02-05T19:34:21Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"These messages are redundant information to a script that's calling git-push.\n\nSigned-off-by: Larry D'Anna \n---\n builtin-push.c | 4 ++--\n transport.c | 6 +++---\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-push.c b/builtin-push.c\nindex 0a27072..3fa4516 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -111,7 +111,7 @@ static int push_with_options(struct transport *transport, int flags)\n \tif (thin)\n \t\ttransport_set_option(transport, TRANS_OPT_THIN, \"yes\");\n \n-\tif (flags & TRANSPORT_PUSH_VERBOSE)\n+\tif (flags & TRANSPORT_PUSH_VERBOSE && !(flags & TRANSPORT_PUSH_PORCELAIN))\n \t\tfprintf(stderr, \"Pushing to %s\\n\", transport->url);\n \terr = transport_push(transport, refspec_nr, refspec, flags,\n \t\t\t &nonfastforward);\n@@ -123,7 +123,7 @@ static int push_with_options(struct transport *transport, int flags)\n \tif (!err)\n \t\treturn 0;\n \n-\tif (nonfastforward && advice_push_nonfastforward) {\n+\tif (!(flags & TRANSPORT_PUSH_PORCELAIN) && nonfastforward && advice_push_nonfastforward) {\n \t\tfprintf(stderr, \"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n \t\t\t\t\"Merge the remote changes before pushing again. See the 'Note about\\n\"\n \t\t\t\t\"fast-forwards' section of 'git push --help' for details.\\n\");\ndiff --git a/transport.c b/transport.c\nindex 3846aac..f707c7b 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -674,7 +674,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)\n \n static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)\n {\n-\tif (!count)\n+\tif (!count && !porcelain)\n \t\tfprintf(stderr, \"To %s\\n\", dest);\n \n \tswitch(ref->status) {\n@@ -1067,10 +1067,10 @@ int transport_push(struct transport *transport,\n \t\tif (!(flags & TRANSPORT_PUSH_DRY_RUN)) {\n \t\t\tstruct ref *ref;\n \t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n-\t\t\t\tupdate_tracking_ref(transport->remote, ref, verbose);\n+\t\t\t\tupdate_tracking_ref(transport->remote, ref, verbose && !porcelain);\n \t\t}\n \n-\t\tif (!quiet && !ret && !refs_pushed(remote_refs))\n+\t\tif (!quiet && !porcelain && !ret && !refs_pushed(remote_refs))\n \t\t\tfprintf(stderr, \"Everything up-to-date\\n\");\n \t\treturn ret;\n \t}\n-- \n1.7.0.rc1.33.g07cf0f.dirty\n"},{"id":"133701","messageId":"1265398462-17316-3-git-send-email-larry@elder-gods.org","threadId":"22526","inReplyTo":"20100205150638.GB14116@coredump.intra.peff.net","subject":"[PATCH 3/3] make git push --dry-run --porcelain exit with status 0 even if updates will be rejected","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T19:34:22Z","receivedAt":"2010-02-05T19:34:22Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"The script calling git push --dry-run --porcelain can see clearly from the\noutput that the updates will be rejected. However, it will probably need to\ndistinguish this condition from the push failing for other reasons, such as the\nremote not being reachable.\n\nSigned-off-by: Larry D'Anna \n---\n builtin-send-pack.c | 5 +++++\n send-pack.h | 1 +\n transport.c | 11 +++++++++--\n 3 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 76c7206..dfd7470 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -478,6 +478,11 @@ int send_pack(struct send_pack_args *args,\n \t\treturn ret;\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tswitch (ref->status) {\n+\t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n+\t\tcase REF_STATUS_REJECT_NODELETE:\n+\t\t\tif (args->porcelain && args->dry_run)\n+\t\t\t\tbreak;\n+\t\t\treturn -1;\n \t\tcase REF_STATUS_NONE:\n \t\tcase REF_STATUS_UPTODATE:\n \t\tcase REF_STATUS_OK:\ndiff --git a/send-pack.h b/send-pack.h\nindex 28141ac..60b4ba6 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -4,6 +4,7 @@\n struct send_pack_args {\n \tunsigned verbose:1,\n \t\tquiet:1,\n+\t\tporcelain:1,\n \t\tsend_mirror:1,\n \t\tforce_update:1,\n \t\tuse_thin_pack:1,\ndiff --git a/transport.c b/transport.c\nindex f707c7b..e61d288 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -558,10 +558,16 @@ static int fetch_refs_via_pack(struct transport *transport,\n \treturn (refs ? 0 : -1);\n }\n \n-static int push_had_errors(struct ref *ref)\n+static int push_had_errors(struct ref *ref, int flags)\n {\n \tfor (; ref; ref = ref->next) {\n \t\tswitch (ref->status) {\n+\t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n+\t\tcase REF_STATUS_REJECT_NODELETE:\n+\t\t\tif (flags & TRANSPORT_PUSH_DRY_RUN && flags & TRANSPORT_PUSH_PORCELAIN)\n+\t\t\t\tbreak;\n+\t\t\telse\n+\t\t\t\treturn 1;\n \t\tcase REF_STATUS_NONE:\n \t\tcase REF_STATUS_UPTODATE:\n \t\tcase REF_STATUS_OK:\n@@ -791,6 +797,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \targs.verbose = !!(flags & TRANSPORT_PUSH_VERBOSE);\n \targs.quiet = !!(flags & TRANSPORT_PUSH_QUIET);\n \targs.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);\n+\targs.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);\n \n \tret = send_pack(&args, data->fd, data->conn, remote_refs,\n \t\t\t&data->extra_have);\n@@ -1052,7 +1059,7 @@ int transport_push(struct transport *transport,\n \t\t\tflags & TRANSPORT_PUSH_FORCE);\n \n \t\tret = transport->push_refs(transport, remote_refs, flags);\n-\t\terr = push_had_errors(remote_refs);\n+\t\terr = push_had_errors(remote_refs, flags);\n \n \t\tret |= err;\n \n-- \n1.7.0.rc1.33.g07cf0f.dirty\n"},{"id":"133704","messageId":"20100205193950.GA18108@cthulhu","threadId":"22526","inReplyTo":"20100205150638.GB14116@coredump.intra.peff.net","subject":"Re: [PATCH] fix an error message in git-push so it goes to stderr","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T19:39:50Z","receivedAt":"2010-02-05T19:39:50Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"* Jeff King (peff@peff.net) [100205 10:06]:\n> On Thu, Feb 04, 2010 at 07:41:40PM -0500, Larry D'Anna wrote:\n> \n> > Having it go to standard output interferes with git-push --porcelain.\n> > ---\n> > builtin-push.c | 6 +++---\n> > 1 files changed, 3 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/builtin-push.c b/builtin-push.c\n> > index 5633f0a..0a27072 100644\n> > --- a/builtin-push.c\n> > +++ b/builtin-push.c\n> > @@ -124,9 +124,9 @@ static int push_with_options(struct transport *transport, int flags)\n> > \t\treturn 0;\n> > \n> > \tif (nonfastforward && advice_push_nonfastforward) {\n> > -\t\tprintf(\"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n> > -\t\t \"Merge the remote changes before pushing again. See the 'Note about\\n\"\n> > -\t\t \"fast-forwards' section of 'git push --help' for details.\\n\");\n> > +\t\tfprintf(stderr, \"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n> > +\t\t\t\t\"Merge the remote changes before pushing again. See the 'Note about\\n\"\n> > +\t\t\t\t\"fast-forwards' section of 'git push --help' for details.\\n\");\n> \n> I agree that stderr is a more sensible place for such a message to go,\n> but shouldn't the porcelain output format just suppress it entirely? \n\nI think you're right. There are some other messages that are similar that\nshould probably also be suppressed.\n\nAlso it seems to me that git push --dry-run --porcelain should exit successfully\neven if it knows some refs will be rejected. The calling script can see just\nfine for itself that they will be rejected, and it probably still wants to know\nwhether or not the dry-run succeeded, which has nothing to do with whether or\nnot the same push would succeed as a not-dry-run. \n\n --larry\n"},{"id":"133708","messageId":"20100205194824.GD24474@coredump.intra.peff.net","threadId":"22526","inReplyTo":"20100205193950.GA18108@cthulhu","subject":"Re: [PATCH] fix an error message in git-push so it goes to stderr","fromName":"Jeff King","fromEmail":"jrk@wrek.org","sentAt":"2010-02-05T19:48:24Z","receivedAt":"2010-02-05T19:48:24Z","isPatch":true,"sender":{"key":"jrk@wrek.org","avatar":null},"body":"On Fri, Feb 05, 2010 at 02:39:50PM -0500, Larry D'Anna wrote:\n\n> Also it seems to me that git push --dry-run --porcelain should exit successfully\n> even if it knows some refs will be rejected. The calling script can see just\n> fine for itself that they will be rejected, and it probably still wants to know\n> whether or not the dry-run succeeded, which has nothing to do with whether or\n> not the same push would succeed as a not-dry-run.\n\nI think that is OK, but only if \"git push --dry-run\" still exits with an\nerror case, since people may be using it for \"will this push work?\" and\nnot simply \"did an error occur?\".\n\n-Peff\n"},{"id":"133706","messageId":"20100205195004.GA21772@cthulhu","threadId":"22526","inReplyTo":"20100205194824.GD24474@coredump.intra.peff.net","subject":"Re: [PATCH] fix an error message in git-push so it goes to stderr","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T19:50:04Z","receivedAt":"2010-02-05T19:50:04Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"* Jeff King (jrk@wrek.org) [100205 14:48]:\n> On Fri, Feb 05, 2010 at 02:39:50PM -0500, Larry D'Anna wrote:\n> \n> > Also it seems to me that git push --dry-run --porcelain should exit successfully\n> > even if it knows some refs will be rejected. The calling script can see just\n> > fine for itself that they will be rejected, and it probably still wants to know\n> > whether or not the dry-run succeeded, which has nothing to do with whether or\n> > not the same push would succeed as a not-dry-run.\n> \n> I think that is OK, but only if \"git push --dry-run\" still exits with an\n> error case, since people may be using it for \"will this push work?\" and\n> not simply \"did an error occur?\".\n\nYup. That's exactly what the patch I just posted does.\n\n --larry\n"},{"id":"133707","messageId":"20100205195039.GA25201@coredump.intra.peff.net","threadId":"22526","inReplyTo":"20100205194824.GD24474@coredump.intra.peff.net","subject":"Re: [PATCH] fix an error message in git-push so it goes to stderr","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-05T19:50:39Z","receivedAt":"2010-02-05T19:50:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 05, 2010 at 02:48:24PM -0500, Jeff King wrote:\n\n> From: Jeff King \n\nArgh, stupid email configuration failure. Please address any followups\nto my usual peff@peff.net.\n\n-Peff\n"},{"id":"133709","messageId":"20100205195644.GE24474@coredump.intra.peff.net","threadId":"22526","inReplyTo":"1265398462-17316-3-git-send-email-larry@elder-gods.org","subject":"Re: [PATCH 3/3] make git push --dry-run --porcelain exit with status 0 even if updates will be rejected","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-05T19:56:44Z","receivedAt":"2010-02-05T19:56:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 05, 2010 at 02:34:22PM -0500, Larry D'Anna wrote:\n\n> diff --git a/builtin-send-pack.c b/builtin-send-pack.c\n> index 76c7206..dfd7470 100644\n> --- a/builtin-send-pack.c\n> +++ b/builtin-send-pack.c\n> @@ -478,6 +478,11 @@ int send_pack(struct send_pack_args *args,\n> \t\treturn ret;\n> \tfor (ref = remote_refs; ref; ref = ref->next) {\n> \t\tswitch (ref->status) {\n> +\t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n> +\t\tcase REF_STATUS_REJECT_NODELETE:\n> +\t\t\tif (args->porcelain && args->dry_run)\n> +\t\t\t\tbreak;\n> +\t\t\treturn -1;\n> \t\tcase REF_STATUS_NONE:\n> \t\tcase REF_STATUS_UPTODATE:\n> \t\tcase REF_STATUS_OK:\n\nWhy just these two status flags? Based on your reasoning elsewhere, I\nwould assume the logic should be:\n\n - if we had some transport-related error, return failure\n\n - if not, then return success, as any ref's failure is already\n indicated in the porcelain output\n\nSo shouldn't it just be:\n\n if (args->porcelain && args->dry_run)\n return 0;\n\nafter we check for transport errors but before the loop that you are\nmodifying.\n\n> -static int push_had_errors(struct ref *ref)\n> +static int push_had_errors(struct ref *ref, int flags)\n> {\n> \tfor (; ref; ref = ref->next) {\n> \t\tswitch (ref->status) {\n> +\t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n> +\t\tcase REF_STATUS_REJECT_NODELETE:\n> +\t\t\tif (flags & TRANSPORT_PUSH_DRY_RUN && flags & TRANSPORT_PUSH_PORCELAIN)\n> +\t\t\t\tbreak;\n> +\t\t\telse\n> +\t\t\t\treturn 1;\n\nDitto here.\n\n-Peff\n"},{"id":"133711","messageId":"20100205200524.GA24027@cthulhu","threadId":"22526","inReplyTo":"20100205195644.GE24474@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] make git push --dry-run --porcelain exit with status 0 even if updates will be rejected","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T20:05:24Z","receivedAt":"2010-02-05T20:05:24Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"* Jeff King (peff@peff.net) [100205 14:56]:\n> On Fri, Feb 05, 2010 at 02:34:22PM -0500, Larry D'Anna wrote:\n> \n> > diff --git a/builtin-send-pack.c b/builtin-send-pack.c\n> > index 76c7206..dfd7470 100644\n> > --- a/builtin-send-pack.c\n> > +++ b/builtin-send-pack.c\n> > @@ -478,6 +478,11 @@ int send_pack(struct send_pack_args *args,\n> > \t\treturn ret;\n> > \tfor (ref = remote_refs; ref; ref = ref->next) {\n> > \t\tswitch (ref->status) {\n> > +\t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n> > +\t\tcase REF_STATUS_REJECT_NODELETE:\n> > +\t\t\tif (args->porcelain && args->dry_run)\n> > +\t\t\t\tbreak;\n> > +\t\t\treturn -1;\n> > \t\tcase REF_STATUS_NONE:\n> > \t\tcase REF_STATUS_UPTODATE:\n> > \t\tcase REF_STATUS_OK:\n> \n> Why just these two status flags? Based on your reasoning elsewhere, I\n> would assume the logic should be:\n> \n> - if we had some transport-related error, return failure\n> \n> - if not, then return success, as any ref's failure is already\n> indicated in the porcelain output\n> \n> So shouldn't it just be:\n> \n> if (args->porcelain && args->dry_run)\n> return 0;\n> \n> after we check for transport errors but before the loop that you are\n> modifying.\n\nI don't know what the deal is with REF_STATUS_EXPECTING_REPORT, so I didn't want\nto modify the behavior in the case that ref->status was that. What does\nexpecting report mean?\n\n --larry\n"},{"id":"133712","messageId":"20100205201325.GA25697@coredump.intra.peff.net","threadId":"22526","inReplyTo":"20100205200524.GA24027@cthulhu","subject":"Re: [PATCH 3/3] make git push --dry-run --porcelain exit with status 0 even if updates will be rejected","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-05T20:13:25Z","receivedAt":"2010-02-05T20:13:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 05, 2010 at 03:05:24PM -0500, Larry D'Anna wrote:\n\n> > So shouldn't it just be:\n> > \n> > if (args->porcelain && args->dry_run)\n> > return 0;\n> > \n> > after we check for transport errors but before the loop that you are\n> > modifying.\n> \n> I don't know what the deal is with REF_STATUS_EXPECTING_REPORT, so I\n> didn't want to modify the behavior in the case that ref->status was\n> that. What does expecting report mean?\n\nIt means we told the other side we wanted to push that ref, and we\nexpect it to give us a status report. Most refs are in that state for a\nshort period, and then moved to their final state in\nbuiltin-send-pack.c:receive_status. But if we never get a status for\nthat ref for some reason, then that could be the final state.\n\nBut more to the point, I don't think this bit of code should _have_ to\ncare what it means. If there is a per-ref error with \"push --dry-run\n--porcelain\", it will be shown on that ref's output line. So I think\nyour proposal should simply be \"if dry-run and porcelain, don't bother\nlooking at per-ref errors at all\". You don't care what the per-ref\nerror is; they are all in the same class from the perspective of this\nchange.\n\n-Peff\n"},{"id":"133713","messageId":"7v1vgz5ta7.fsf@alter.siamese.dyndns.org","threadId":"22526","inReplyTo":"1265398462-17316-2-git-send-email-larry@elder-gods.org","subject":"Re: [PATCH 2/3] silence human readable info messages going to stderr from git push --porcelain","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-05T20:20:16Z","receivedAt":"2010-02-05T20:20:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Larry D'Anna writes:\n\n> These messages are redundant information to a script that's calling git-push.\n\nRedundant is not a reason for a change; unwanted would be. And some of\nthe messages you are trying to squelch indeed look like unwanted ones, but\nnot all of them.\n\n> -\tif (flags & TRANSPORT_PUSH_VERBOSE)\n> +\tif (flags & TRANSPORT_PUSH_VERBOSE && !(flags & TRANSPORT_PUSH_PORCELAIN))\n> \t\tfprintf(stderr, \"Pushing to %s\\n\", transport->url);\n\nWhy should you be forbidden to expect \"--porcelain -v\" to give you this\nmessage?\n\n> @@ -123,7 +123,7 @@ static int push_with_options(struct transport *transport, int flags)\n> \tif (!err)\n> \t\treturn 0;\n> \n> -\tif (nonfastforward && advice_push_nonfastforward) {\n> +\tif (!(flags & TRANSPORT_PUSH_PORCELAIN) && nonfastforward && advice_push_nonfastforward) {\n> \t\tfprintf(stderr, \"To prevent you from losing history, non-fast-forward updates were rejected\\n\"\n> \t\t\t\t\"Merge the remote changes before pushing again. See the 'Note about\\n\"\n> \t\t\t\t\"fast-forwards' section of 'git push --help' for details.\\n\");\n\nThis probably is a good change; the long lines are unsightly but that is a\nseparate topic.\n\n> diff --git a/transport.c b/transport.c\n> index 3846aac..f707c7b 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -674,7 +674,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)\n> \n> static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)\n> {\n> -\tif (!count)\n> +\tif (!count && !porcelain)\n> \t\tfprintf(stderr, \"To %s\\n\", dest);\n\nI don't think this is correct.\n\nIf you have more than one remote.there.pushURL, the calling Porcelain\nscript of \"git push --porcelain there\" should be able to tell which\ndestination the following report is about, and without this line you\ncannot tell.\n\nI would understand if this change were to make the message go to the\nstandard output when operating with --porcelain option, though.\n\n> @@ -1067,10 +1067,10 @@ int transport_push(struct transport *transport,\n> \t\tif (!(flags & TRANSPORT_PUSH_DRY_RUN)) {\n> \t\t\tstruct ref *ref;\n> \t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n> -\t\t\t\tupdate_tracking_ref(transport->remote, ref, verbose);\n> +\t\t\t\tupdate_tracking_ref(transport->remote, ref, verbose && !porcelain);\n\nAgain, why should you be forbidden to expect \"--porcelain -v\" to work?\n\n> -\t\tif (!quiet && !ret && !refs_pushed(remote_refs))\n> +\t\tif (!quiet && !porcelain && !ret && !refs_pushed(remote_refs))\n> \t\t\tfprintf(stderr, \"Everything up-to-date\\n\");\n\nThis is a borderline. If you are truly up-to-date, the calling script\nwon't get anything. It may be easier for Porcelain scripts to see this\nmessage on the standard output as an explicit succeses report instead.\n"},{"id":"133715","messageId":"20100205203045.GA1937@cthulhu","threadId":"22526","inReplyTo":"7v1vgz5ta7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] silence human readable info messages going to stderr from git push --porcelain","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-05T20:30:45Z","receivedAt":"2010-02-05T20:30:45Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"* Junio C Hamano (gitster@pobox.com) [100205 15:20]:\n> > -\t\tif (!quiet && !ret && !refs_pushed(remote_refs))\n> > +\t\tif (!quiet && !porcelain && !ret && !refs_pushed(remote_refs))\n> > \t\t\tfprintf(stderr, \"Everything up-to-date\\n\");\n> \n> This is a borderline. If you are truly up-to-date, the calling script\n> won't get anything. It may be easier for Porcelain scripts to see this\n> message on the standard output as an explicit succeses report instead.\n\nhow about this?\n\nif (!quiet && (!porcelain || verbose) && !ret && !refs_pushed(remote_refs))\n fprintf(stderr, \"Everything up-to-date\\n\"); \n\n --larry\n\n\n\n \n"}]}