{"thread":{"id":"22839","subject":"[PATCH 4/4] git-push: add tests for git push --porcelain","startedAt":"2010-02-27T03:59:47Z","lastAt":"2010-02-27T04:19:20Z","messageCount":6,"participants":["Larry D'Anna","Tay Ray Chuan"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"135799","messageId":"cover.1267242789.git.larry@elder-gods.org","threadId":"22839","inReplyTo":null,"subject":"[PATCH 0/4] ld/push-porcelain","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-27T03:59:47Z","receivedAt":"2010-02-27T03:59:47Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"Changes since last version:\n\n* instead of messing with the exit status git push --porcelain will now print\n  \"Done\" if nothing untowrd happned (ie all the error messages are in the ref\n  status lines)\n\n* added tests for git push --porcelain \n\nLarry D'Anna (4):\n  git-push: fix an advice message so it goes to stderr\n  git-push: send \"To <remoteurl>\" messages to the standard output in\n    --porcelain mode\n  git-push: make git push --porcelain print \"Done\"\n  git-push: add tests for git push --porcelain\n\n builtin-push.c        |    6 +++---\n builtin-send-pack.c   |    4 ++++\n send-pack.h           |    1 +\n t/t5516-fetch-push.sh |   35 +++++++++++++++++++++++++++++++++++\n transport.c           |   17 +++++++++++------\n 5 files changed, 54 insertions(+), 9 deletions(-)\n"},{"id":"135797","messageId":"68fd39ac6602471919939a84168be62f1fe464f7.1267243044.git.larry@elder-gods.org","threadId":"22839","inReplyTo":"cover.1267242789.git.larry@elder-gods.org","subject":"[PATCH 1/4] git-push: fix an advice message so it goes to stderr","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-27T03:59:48Z","receivedAt":"2010-02-27T03:59:48Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"These sort of messages typically go to the standard error.\n\nSigned-off-by: Larry D'Anna <larry@elder-gods.org>\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.rc2.40.g7d8aa\n"},{"id":"135798","messageId":"0ecbc2ed26f4fe0a253bd34f94ffae8f71f81aea.1267243044.git.larry@elder-gods.org","threadId":"22839","inReplyTo":"cover.1267242789.git.larry@elder-gods.org","subject":"[PATCH 2/4] git-push: send \"To <remoteurl>\" messages to the standard output in --porcelain mode","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-27T03:59:49Z","receivedAt":"2010-02-27T03:59:49Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"git-push prints the line \"To <remoteurl>\" before above each of the ref status\nlines.  In --porcelain mode, these \"To <remoteurl>\" lines go to the standard\nerror, but the ref status lines go to the standard output.  This makes it\ndifficult for the process reading standard output to know which ref status lines\ncorrespond to which remote.  This patch sends the \"To <remoteurl>\" lines to the\nthe standard output instead.\n\nSigned-off-by: Larry D'Anna <larry@elder-gods.org>\n---\n transport.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 08e4fa0..32885f7 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -675,7 +675,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)\n static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)\n {\n \tif (!count)\n-\t\tfprintf(stderr, \"To %s\\n\", dest);\n+\t\tfprintf(porcelain ? stdout : stderr, \"To %s\\n\", dest);\n \n \tswitch(ref->status) {\n \tcase REF_STATUS_NONE:\n-- \n1.7.0.rc2.40.g7d8aa\n"},{"id":"135800","messageId":"04fc63d0f8114f0cd721bcf3dff1f108e30b5308.1267243044.git.larry@elder-gods.org","threadId":"22839","inReplyTo":"cover.1267242789.git.larry@elder-gods.org","subject":"[PATCH 3/4] git-push: make git push --porcelain print \"Done\"","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-27T03:59:50Z","receivedAt":"2010-02-27T03:59:50Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"The script calling git push --porcelain can see clearly from the output if an\nupdate was rejected.  However, it will probably need to distinguish this\ncondition from the push failing for other reasons, such as the remote not being\nreachable.\n\nThis patch modifies git push --porcelain to print \"Done\" after the rest of its\noutput unless any errors have occurred which were not reported in the ref status\nlines.\n\nSigned-off-by: Larry D'Anna <larry@elder-gods.org>\n---\n builtin-send-pack.c |    4 ++++\n send-pack.h         |    1 +\n transport.c         |   15 ++++++++++-----\n 3 files changed, 15 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 2183a47..87795f5 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -510,6 +510,10 @@ int send_pack(struct send_pack_args *args,\n \n \tif (ret < 0)\n \t\treturn ret;\n+\n+\tif (args->porcelain)\n+\t\treturn 0;\n+\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tswitch (ref->status) {\n \t\tcase REF_STATUS_NONE:\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 32885f7..5b880d7 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -791,6 +791,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@@ -1055,8 +1056,6 @@ int transport_push(struct transport *transport,\n \t\tret = transport->push_refs(transport, remote_refs, flags);\n \t\terr = push_had_errors(remote_refs);\n \n-\t\tret |= err;\n-\n \t\tif (!quiet || err)\n \t\t\tprint_push_status(transport->url, remote_refs,\n \t\t\t\t\tverbose | porcelain, porcelain,\n@@ -1071,9 +1070,15 @@ int transport_push(struct transport *transport,\n \t\t\t\tupdate_tracking_ref(transport->remote, ref, verbose);\n \t\t}\n \n-\t\tif (!quiet && !ret && !refs_pushed(remote_refs))\n-\t\t\tfprintf(stderr, \"Everything up-to-date\\n\");\n-\t\treturn ret;\n+\n+\t\tif (porcelain) {\n+\t\t\tif (ret==0)\n+\t\t\t\tfprintf (stdout, \"Done\\n\");\n+\t\t} else\n+\t\t\tif (!quiet && !ret && !refs_pushed(remote_refs))\n+\t\t\t\tfprintf(stderr, \"Everything up-to-date\\n\");\n+\n+\t\treturn ret | err;\n \t}\n \treturn 1;\n }\n-- \n1.7.0.rc2.40.g7d8aa\n"},{"id":"135796","messageId":"c75ea98d8b208cb3eb81c1526c428cfa0af40185.1267243044.git.larry@elder-gods.org","threadId":"22839","inReplyTo":"cover.1267242789.git.larry@elder-gods.org","subject":"[PATCH 4/4] git-push: add tests for git push --porcelain","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-27T03:59:51Z","receivedAt":"2010-02-27T03:59:51Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"Verify that the output format is correct for successful, rejected, and\nflagrantly erroneous pushes.\n\nSigned-off-by: Larry D'Anna <larry@elder-gods.org>\n---\n t/t5516-fetch-push.sh |   35 +++++++++++++++++++++++++++++++++++\n 1 files changed, 35 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 0f04b2e..f3f113f 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -660,4 +660,39 @@ test_expect_success 'push with branches containing #' '\n \tgit checkout master\n '\n \n+test_expect_success 'push --porcelain' '\n+\tmk_empty &&\n+\techo >.git/foo  \"To testrepo\" &&\n+\techo >>.git/foo \"*\trefs/heads/master:refs/remotes/origin/master\t[new branch]\"  &&\n+\techo >>.git/foo \"Done\" &&\n+\tgit push >.git/bar --porcelain  testrepo refs/heads/master:refs/remotes/origin/master &&\n+\t(\n+\t\tcd testrepo &&\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t) &&\n+\tdiff -q .git/foo .git/bar\n+'\n+\n+test_expect_success 'push --porcelain bad url' '\n+\tmk_empty &&\n+\ttest_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&\n+\ttest_must_fail grep -q Done .git/bar\n+'\n+\n+test_expect_success 'push --porcelain rejected' '\n+\tmk_empty &&\n+\tgit push testrepo refs/heads/master:refs/remotes/origin/master &&\n+\t(cd testrepo &&\n+\t\tgit reset --hard origin/master^\n+\t\tgit config receive.denyCurrentBranch true) &&\n+\n+\techo >.git/foo  \"To testrepo\"  &&\n+\techo >>.git/foo \"!\trefs/heads/master:refs/heads/master\t[remote rejected] (branch is currently checked out)\" &&\n+\n+\ttest_must_fail git push >.git/bar --porcelain  testrepo refs/heads/master:refs/heads/master &&\n+\tdiff -q .git/foo .git/bar\n+'\n+\n test_done\n-- \n1.7.0.rc2.40.g7d8aa\n"},{"id":"135802","messageId":"be6fef0d1002262019j2088d20dp89f1587e3921a1ba@mail.gmail.com","threadId":"22839","inReplyTo":"04fc63d0f8114f0cd721bcf3dff1f108e30b5308.1267243044.git.larry@elder-gods.org","subject":"Re: [PATCH 3/4] git-push: make git push --porcelain print \"Done\"","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-27T04:19:20Z","receivedAt":"2010-02-27T04:19:20Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sat, Feb 27, 2010 at 11:59 AM, Larry D'Anna <larry@elder-gods.org> wrote:\n> diff --git a/transport.c b/transport.c\n> index 32885f7..5b880d7 100644\n> --- a/transport.c\n> +++ b/transport.c\n> [snip]\n\nThis hunk:\n\n> @@ -1055,8 +1056,6 @@ int transport_push(struct transport *transport,\n>                ret = transport->push_refs(transport, remote_refs, flags);\n>                err = push_had_errors(remote_refs);\n>\n> -               ret |= err;\n> -\n>                if (!quiet || err)\n>                        print_push_status(transport->url, remote_refs,\n>                                        verbose | porcelain, porcelain,\n\nand this hunk:\n\n> @@ -1071,9 +1070,15 @@ int transport_push(struct transport *transport,\n>                                update_tracking_ref(transport->remote, ref, verbose);\n>                }\n>\n> -               if (!quiet && !ret && !refs_pushed(remote_refs))\n> -                       fprintf(stderr, \"Everything up-to-date\\n\");\n> -               return ret;\n> +\n> +               if (porcelain) {\n> +                       if (ret==0)\n> +                               fprintf (stdout, \"Done\\n\");\n> +               } else\n> +                       if (!quiet && !ret && !refs_pushed(remote_refs))\n> +                               fprintf(stderr, \"Everything up-to-date\\n\");\n> +\n> +               return ret | err;\n>        }\n>        return 1;\n>  }\n\nslightly changes the conditions under which \"Everything up-to-date\" is\nprinted, since you did away with \"ret |= err\". I guess you can\nmitigate this by adding \"!err\" to the list of operands:\n\n  if (!quiet && !ret && !err && !refs_pushed(remote_refs))\n      ....\n\nEven better, create yet another temporary variable:\n\n  push_ret = transport->push_refs(transport, remote_refs, flags);\n  err = push_had_errors(remote_refs);\n\n  ret = push_ret | err;\n\nThat way, you can access transport->push_refs() status (what you\nwant), without messing with the other parts of the code and their\nassumptions.\n\nOne last thing: could the if statements towards the end be \"flattened\"?\n\n  if (porcelain && ret == 0)\n      fprintf(stdout, \"Done\\n\");\n  else if (!quiet && !ret && !refs_pushed(remote_refs))\n      fprintf(stderr, \"Everything up-to-date\\n\");\n\n(sorry about the lack of tabs, gmail doesn't let me type those easily.)\n\n-- \nCheers,\nRay Chuan\n"}]}