{"thread":{"id":"10665","subject":"[PATCH 0/3] more terse push output","startedAt":"2007-11-05T05:05:17Z","lastAt":"2007-11-05T21:14:43Z","messageCount":6,"participants":["Jeff King","Steffen Prohaska","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"58347","messageId":"20071105050517.GA6244@sigill.intra.peff.net","threadId":"10665","inReplyTo":null,"subject":"[PATCH 0/3] more terse push output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-05T05:05:17Z","receivedAt":"2007-11-05T05:05:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here are some patches to match push output to the new fetch output. The\nentire output of \"git-push\" for my test repo now looks like this:\n\nTo file:///tmp/parent\n + f3325dc...3b91d1c hasforce -> mirror/hasforce (forced update)\n   f3325dc..bb022dc  master -> mirror/master\n ! [rejected]        needsforce -> mirror/needsforce (non-fast forward)\n * [new branch]      newbranch -> mirror/newbranch\n * [new tag]         v1.0 -> v1.0\nCounting objects: 5, done.\nWriting objects: 100% (3/3), done.\nTotal 3 (delta 0), reused 0 (delta 0)\nUnpacking objects: 100% (3/3), done.\n\n(where I'm mirroring the heads in refs/heads/mirror/* to show off the name\nmapping).\n\nThe interesting part is in the first patch; the other two are just\nremoving clutter on stderr. The patches are based on 'next' to follow\nDaniel's send-pack work. I'm happy to hold on to them and rebase once\nthings settle down in this area.\n\nA few caveats:\n\n  - the output is generated by send-pack, so it's actually \"here's what\n    I'm about to do\" rather than \"here's what has happened or is\n    happening.\" receive-pack could generate more accurate results\n    (including marking rejections based on hooks), but it has no\n    knowledge of the name mapping (so it can't say \"foo -> other/foo\").\n    Right now receive-pack generates 'error: blah blah blah' as it\n    always has.\n\n  - the name mapping is perhaps superfluous here, since most of the time\n    people are just pushing using the same branch name. But it matches\n    the new fetch output, and if you do use a refspec, it's a lot more\n    clear.\n\n  - this only covers send-pack, since that is where we do the ref\n    matching. Presumably it would be possible to cover pushing over http\n    and rsync, but it would have to be totally separate code.\n\n  - no code is shared with the git-fetch implementation (they just\n    happen to use the same format). The code is short enough that I\n    think it would just end up more confusing to try to factor out the\n    commonality.\n\n  - the 'ref is not up-to-date, maybe you need to push' message has gone\n    away in favor of the terse '[rejected] ... (non-fast forward)'. I\n    know there was some discussion recently of enhancing that message.\n    Is this perhaps too terse?\n\n-Peff\n"},{"id":"58348","messageId":"20071105051114.GA13144@sigill.intra.peff.net","threadId":"10665","inReplyTo":"20071105050517.GA6244@sigill.intra.peff.net","subject":"[PATCH 1/3] more terse push output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-05T05:11:15Z","receivedAt":"2007-11-05T05:11:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This changes the output of send-pack to match the new,\nmore terse fetch output. It looks like this:\n\nTo git://host.tld/path/to/repo\n + f3325dc...3b91d1c hasforce -> mirror/hasforce (forced update)\n   f3325dc..bb022dc  master -> mirror/master\n ! [rejected]        needsforce -> mirror/needsforce (non-fast forward)\n * [new branch]      newbranch -> mirror/newbranch\n * [new tag]         v1.0 -> v1.0\n\ninstead of:\n\nupdating 'refs/heads/mirror/hasforce' using 'refs/heads/hasforce'\n  from f3325dca9c4a34d74012c0e159254f454930cec7\n  to   3b91d1c310ca9d7b547b85466dd876e143498304\nupdating 'refs/heads/mirror/master' using 'refs/heads/master'\n  from f3325dca9c4a34d74012c0e159254f454930cec7\n  to   bb022dc363d5c2aa9aa3026beb9706d44fbe1328\nerror: remote 'refs/heads/mirror/needsforce' is not an ancestor of\n local  'refs/heads/needsforce'.\n Maybe you are not up-to-date and need to pull first?\nupdating 'refs/heads/mirror/newbranch' using 'refs/heads/newbranch'\n  from 0000000000000000000000000000000000000000\n  to   3b91d1c310ca9d7b547b85466dd876e143498304\nupdating 'refs/tags/v1.0'\n  from 0000000000000000000000000000000000000000\n  to   bb022dc363d5c2aa9aa3026beb9706d44fbe1328\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin-send-pack.c |   81 ++++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 64 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 947c42b..d74cc3c 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -206,7 +206,18 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)\n \t}\n }\n \n-static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec, const char **refspec)\n+static const char *prettify_ref(const char *name)\n+{\n+\treturn name + (\n+\t\t!prefixcmp(name, \"refs/heads/\") ? 11 :\n+\t\t!prefixcmp(name, \"refs/tags/\") ? 10 :\n+\t\t!prefixcmp(name, \"refs/remotes/\") ? 13 :\n+\t\t0);\n+}\n+\n+#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n+\n+static int do_send_pack(int in, int out, struct remote *remote, const char *dest, int nr_refspec, const char **refspec)\n {\n \tstruct ref *ref;\n \tint new_refs;\n@@ -214,6 +225,7 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,\n \tint ask_for_status_report = 0;\n \tint allow_deleting_refs = 0;\n \tint expect_status_report = 0;\n+\tint shown_dest = 0;\n \n \t/* No funny business with the matcher */\n \tremote_tail = get_remote_heads(in, &remote_refs, 0, NULL, REF_NORMAL);\n@@ -245,21 +257,33 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tchar old_hex[60], *new_hex;\n \t\tint will_delete_ref;\n+\t\tconst char *pretty_ref;\n+\t\tconst char *pretty_peer;\n \n \t\tif (!ref->peer_ref)\n \t\t\tcontinue;\n \n+\t\tif (!shown_dest) {\n+\t\t\tfprintf(stderr, \"To %s\\n\", dest);\n+\t\t\tshown_dest = 1;\n+\t\t}\n+\n+\t\tpretty_ref = prettify_ref(ref->name);\n+\t\tpretty_peer = prettify_ref(ref->peer_ref->name);\n \n \t\twill_delete_ref = is_null_sha1(ref->peer_ref->new_sha1);\n \t\tif (will_delete_ref && !allow_deleting_refs) {\n-\t\t\terror(\"remote does not support deleting refs\");\n+\t\t\tfprintf(stderr, \" ! %-*s %s (remote does not support deleting refs)\\n\",\n+\t\t\t\t\tSUMMARY_WIDTH, \"[rejected]\", pretty_ref);\n \t\t\tret = -2;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!will_delete_ref &&\n \t\t    !hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {\n \t\t\tif (args.verbose)\n-\t\t\t\tfprintf(stderr, \"'%s': up-to-date\\n\", ref->name);\n+\t\t\t\tfprintf(stderr, \" = %-*s %s -> %s\\n\",\n+\t\t\t\t\tSUMMARY_WIDTH, \"[up to date]\",\n+\t\t\t\t\tpretty_peer, pretty_ref);\n \t\t\tcontinue;\n \t\t}\n \n@@ -296,12 +320,9 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,\n \t\t\t\t * commits at the remote end and likely\n \t\t\t\t * we were not up to date to begin with.\n \t\t\t\t */\n-\t\t\t\terror(\"remote '%s' is not an ancestor of\\n\"\n-\t\t\t\t      \" local  '%s'.\\n\"\n-\t\t\t\t      \" Maybe you are not up-to-date and \"\n-\t\t\t\t      \"need to pull first?\",\n-\t\t\t\t      ref->name,\n-\t\t\t\t      ref->peer_ref->name);\n+\t\t\t\tfprintf(stderr, \" ! %-*s %s -> %s (non-fast forward)\\n\",\n+\t\t\t\t\t\tSUMMARY_WIDTH, \"[rejected]\",\n+\t\t\t\t\t\tpretty_peer, pretty_ref);\n \t\t\t\tret = -2;\n \t\t\t\tcontinue;\n \t\t\t}\n@@ -325,14 +346,40 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,\n \t\t\t\t\told_hex, new_hex, ref->name);\n \t\t}\n \t\tif (will_delete_ref)\n-\t\t\tfprintf(stderr, \"deleting '%s'\\n\", ref->name);\n+\t\t\tfprintf(stderr, \" - %-*s %s\\n\",\n+\t\t\t\tSUMMARY_WIDTH, \"[deleting]\",\n+\t\t\t\tpretty_ref);\n+\t\telse if (is_null_sha1(ref->old_sha1)) {\n+\t\t\tconst char *msg;\n+\n+\t\t\tif (!prefixcmp(ref->name, \"refs/tags/\"))\n+\t\t\t\tmsg = \"[new tag]\";\n+\t\t\telse\n+\t\t\t\tmsg = \"[new branch]\";\n+\t\t\tfprintf(stderr, \" * %-*s %s -> %s\\n\",\n+\t\t\t\tSUMMARY_WIDTH, msg,\n+\t\t\t\tpretty_peer, pretty_ref);\n+\t\t}\n \t\telse {\n-\t\t\tfprintf(stderr, \"updating '%s'\", ref->name);\n-\t\t\tif (strcmp(ref->name, ref->peer_ref->name))\n-\t\t\t\tfprintf(stderr, \" using '%s'\",\n-\t\t\t\t\tref->peer_ref->name);\n-\t\t\tfprintf(stderr, \"\\n  from %s\\n  to   %s\\n\",\n-\t\t\t\told_hex, new_hex);\n+\t\t\tchar quickref[83];\n+\t\t\tchar type = ' ';\n+\t\t\tconst char *msg = \"\";\n+\n+\t\t\tstrcpy(quickref, find_unique_abbrev(ref->old_sha1, DEFAULT_ABBREV));\n+\t\t\tif (ref_newer(ref->peer_ref->new_sha1, ref->old_sha1))\n+\t\t\t\tstrcat(quickref, \"..\");\n+\t\t\telse {\n+\t\t\t\tstrcat(quickref, \"...\");\n+\t\t\t\ttype = '+';\n+\t\t\t\tmsg = \" (forced update)\";\n+\t\t\t}\n+\t\t\tstrcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));\n+\n+\t\t\tfprintf(stderr, \" %c %-*s %s -> %s%s\\n\",\n+\t\t\t\ttype,\n+\t\t\t\tSUMMARY_WIDTH, quickref,\n+\t\t\t\tpretty_peer, pretty_ref,\n+\t\t\t\tmsg);\n \t\t}\n \t}\n \n@@ -460,7 +507,7 @@ int send_pack(struct send_pack_args *my_args,\n \tverify_remote_names(nr_heads, heads);\n \n \tconn = git_connect(fd, dest, args.receivepack, args.verbose ? CONNECT_VERBOSE : 0);\n-\tret = do_send_pack(fd[0], fd[1], remote, nr_heads, heads);\n+\tret = do_send_pack(fd[0], fd[1], remote, dest, nr_heads, heads);\n \tclose(fd[0]);\n \tclose(fd[1]);\n \tret |= finish_connect(conn);\n-- \n1.5.3.5.1530.g7353\n"},{"id":"58349","messageId":"20071105051140.GB13144@sigill.intra.peff.net","threadId":"10665","inReplyTo":"20071105050517.GA6244@sigill.intra.peff.net","subject":"[PATCH 2/3] receive-pack: don't mention successful updates","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-05T05:11:41Z","receivedAt":"2007-11-05T05:11:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The proposed updates are already shown to the user by\nsend-pack, so there's no point. We continue to show errors,\nsince they are unexpected.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n receive-pack.c |    4 ----\n 1 files changed, 0 insertions(+), 4 deletions(-)\n\ndiff --git a/receive-pack.c b/receive-pack.c\nindex 38e35c0..ed44b89 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -204,8 +204,6 @@ static const char *update(struct command *cmd)\n \t\t\terror(\"failed to delete %s\", name);\n \t\t\treturn \"failed to delete\";\n \t\t}\n-\t\tfprintf(stderr, \"%s: %s -> deleted\\n\", name,\n-\t\t\tsha1_to_hex(old_sha1));\n \t\treturn NULL; /* good */\n \t}\n \telse {\n@@ -217,8 +215,6 @@ static const char *update(struct command *cmd)\n \t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n \t\t\treturn \"failed to write\"; /* error() already called */\n \t\t}\n-\t\tfprintf(stderr, \"%s: %s -> %s\\n\", name,\n-\t\t\tsha1_to_hex(old_sha1), sha1_to_hex(new_sha1));\n \t\treturn NULL; /* good */\n \t}\n }\n-- \n1.5.3.5.1530.g7353\n"},{"id":"58350","messageId":"20071105051218.GC13144@sigill.intra.peff.net","threadId":"10665","inReplyTo":"20071105050517.GA6244@sigill.intra.peff.net","subject":"[PATCH 3/3] send-pack: require --verbose to show update of tracking refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-05T05:12:18Z","receivedAt":"2007-11-05T05:12:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is really an uninteresting detail, and it just takes\nattention away from the actual push updates and posssible\nerrors.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin-send-pack.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex d74cc3c..c1fd3f5 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -195,7 +195,8 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)\n \t\treturn;\n \n \tif (!remote_find_tracking(remote, &rs)) {\n-\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n+\t\tif (args.verbose)\n+\t\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n \t\tif (is_null_sha1(ref->peer_ref->new_sha1)) {\n \t\t\tif (delete_ref(rs.dst, NULL))\n \t\t\t\terror(\"Failed to delete\");\n-- \n1.5.3.5.1530.g7353\n"},{"id":"58352","messageId":"20FFDEF3-5F29-4494-9624-2CE30A585B92@zib.de","threadId":"10665","inReplyTo":"20071105050517.GA6244@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] more terse push output","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2007-11-05T07:09:59Z","receivedAt":"2007-11-05T07:09:59Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Nov 5, 2007, at 6:05 AM, Jeff King wrote:\n\n>   - the 'ref is not up-to-date, maybe you need to push' message has  \n> gone\n>     away in favor of the terse '[rejected] ... (non-fast forward)'. I\n>     know there was some discussion recently of enhancing that message.\n>     Is this perhaps too terse?\n\nI like the general idea of the terse output.\n\nIf we want a suggestion to the user, we could put it as\na summary. If a ref was rejected send-pack could print a\nconcluding message:\n\n---\nTo file:///tmp/parent\n  + f3325dc...3b91d1c hasforce -> mirror/hasforce (forced update)\n    f3325dc..bb022dc  master -> mirror/master\n  ! [rejected]        needsforce -> mirror/needsforce (non-fast forward)\n  * [new branch]      newbranch -> mirror/newbranch\n  * [new tag]         v1.0 -> v1.0\nCounting objects: 5, done.\nWriting objects: 100% (3/3), done.\nTotal 3 (delta 0), reused 0 (delta 0)\nUnpacking objects: 100% (3/3), done.\n\nwarning: some refs were rejected.\n  Maybe they are not up-to-date and you want to pull or rebase first?\n  Or you may need to force an update?\n---\n\nIn this way the terse output would not be disrupted and the\nsuggestion would be given to the user nonetheless.\n\nI propose to use \"warning\" because it is not a real error. push\nupdates all other refs as expected. It only rejects some\nrefs. An error would be appropriate only after push learnt\ntransactions, that is either completely succeeds or doesn't\nupdate any ref at all.\n\n\tSteffen\n"},{"id":"58416","messageId":"7v7ikwpf7g.fsf@gitster.siamese.dyndns.org","threadId":"10665","inReplyTo":"20071105050517.GA6244@sigill.intra.peff.net","subject":"Re: [PATCH 0/3] more terse push output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-05T21:14:43Z","receivedAt":"2007-11-05T21:14:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>   - the 'ref is not up-to-date, maybe you need to push' message has gone\n>     away in favor of the terse '[rejected] ... (non-fast forward)'. I\n>     know there was some discussion recently of enhancing that message.\n>     Is this perhaps too terse?\n\nI kind of like this part.\n\nI also like the part that stops mentioning the \"pretend fetching\nback\" action.  This would mostly parrot the same set of refs for\npeople who do use the tracking branches anyway.\n"}]}