{"thread":{"id":"21854","subject":"[PATCH resend 0/3] transport: catch non-fast-forwards","startedAt":"2009-12-04T06:54:37Z","lastAt":"2010-01-08T02:12:45Z","messageCount":12,"participants":["Tay Ray Chuan","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"129160","messageId":"20091204145437.1a9910db.rctay89@gmail.com","threadId":"21854","inReplyTo":null,"subject":"[PATCH resend 0/3] transport: catch non-fast-forwards","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-04T06:54:37Z","receivedAt":"2009-12-04T06:54:37Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This patch series applies on top of 'next', and deals with alerting\nthe user to non-fast-forward pushes when using helpers (smart).\n\nPreviously, git silently exited. This situation involves the curl\nhelper and the smart protocol. The fast-forward push is only detected\nwhen curl executes the rpc client (git-send-pack). Now, we detect it\nbefore telling the helper to push.\n\nThe first patch refactors ref status logic out of builtin-send-pack.c.\n\nThe second patch lets git know of non-fast-forwards before actually\ntelling the helper to push anything.\n\nThe third patch changes the return code when ref status indicate an\nerror (determined by push_had_errors()), so that the caller of\ntransport_push() is alerted.\n\nPS. There are at least 2 bug fixes related to this series. If you\nusually do so from repo.or.cz, please fetch from git.kernel.org; the\nformer is 2 days old.\n\n builtin-send-pack.c |   39 +++------------------------------------\n remote.c            |   42 ++++++++++++++++++++++++++++++++++++++++++\n remote.h            |    1 +\n transport-helper.c  |   10 +++-------\n transport.c         |   11 +++++++----\n 5 files changed, 56 insertions(+), 47 deletions(-)\n\n--\nCheers,\nRay Chuan\n"},{"id":"129161","messageId":"20091204145643.296ec414.rctay89@gmail.com","threadId":"21854","inReplyTo":"20091204145437.1a9910db.rctay89@gmail.com","subject":"[PATCH resend 1/3] refactor ref status logic for pushing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-04T06:56:43Z","receivedAt":"2009-12-04T06:56:43Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Move the logic to a new function in remote.[ch],\n set_ref_status_for_push().\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin-send-pack.c |   39 +++------------------------------------\n remote.c            |   42 ++++++++++++++++++++++++++++++++++++++++++\n remote.h            |    1 +\n 3 files changed, 46 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 8fffdbf..8c98624 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -412,44 +412,11 @@ int send_pack(struct send_pack_args *args,\n \t\telse if (!args->send_mirror)\n \t\t\tcontinue;\n\n-\t\tref->deletion = is_null_sha1(ref->new_sha1);\n-\t\tif (ref->deletion && !allow_deleting_refs) {\n-\t\t\tref->status = REF_STATUS_REJECT_NODELETE;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!ref->deletion &&\n-\t\t    !hashcmp(ref->old_sha1, ref->new_sha1)) {\n-\t\t\tref->status = REF_STATUS_UPTODATE;\n+\t\tif (set_ref_status_for_push(ref, args->force_update))\n \t\t\tcontinue;\n-\t\t}\n\n-\t\t/* This part determines what can overwrite what.\n-\t\t * The rules are:\n-\t\t *\n-\t\t * (0) you can always use --force or +A:B notation to\n-\t\t *     selectively force individual ref pairs.\n-\t\t *\n-\t\t * (1) if the old thing does not exist, it is OK.\n-\t\t *\n-\t\t * (2) if you do not have the old thing, you are not allowed\n-\t\t *     to overwrite it; you would not know what you are losing\n-\t\t *     otherwise.\n-\t\t *\n-\t\t * (3) if both new and old are commit-ish, and new is a\n-\t\t *     descendant of old, it is OK.\n-\t\t *\n-\t\t * (4) regardless of all of the above, removing :B is\n-\t\t *     always allowed.\n-\t\t */\n-\n-\t\tref->nonfastforward =\n-\t\t    !ref->deletion &&\n-\t\t    !is_null_sha1(ref->old_sha1) &&\n-\t\t    (!has_sha1_file(ref->old_sha1)\n-\t\t      || !ref_newer(ref->new_sha1, ref->old_sha1));\n-\n-\t\tif (ref->nonfastforward && !ref->force && !args->force_update) {\n-\t\t\tref->status = REF_STATUS_REJECT_NONFASTFORWARD;\n+\t\tif (ref->deletion && !allow_deleting_refs) {\n+\t\t\tref->status = REF_STATUS_REJECT_NODELETE;\n \t\t\tcontinue;\n \t\t}\n\ndiff --git a/remote.c b/remote.c\nindex e3afecd..188869a 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1247,6 +1247,48 @@ int match_refs(struct ref *src, struct ref **dst,\n \treturn 0;\n }\n\n+int set_ref_status_for_push(struct ref *ref, int force_update)\n+{\n+\tref->deletion = is_null_sha1(ref->new_sha1);\n+\tif (!ref->deletion &&\n+\t\t!hashcmp(ref->old_sha1, ref->new_sha1)) {\n+\t\tref->status = REF_STATUS_UPTODATE;\n+\t\treturn 1;\n+\t}\n+\n+\t/* This part determines what can overwrite what.\n+\t * The rules are:\n+\t *\n+\t * (0) you can always use --force or +A:B notation to\n+\t *     selectively force individual ref pairs.\n+\t *\n+\t * (1) if the old thing does not exist, it is OK.\n+\t *\n+\t * (2) if you do not have the old thing, you are not allowed\n+\t *     to overwrite it; you would not know what you are losing\n+\t *     otherwise.\n+\t *\n+\t * (3) if both new and old are commit-ish, and new is a\n+\t *     descendant of old, it is OK.\n+\t *\n+\t * (4) regardless of all of the above, removing :B is\n+\t *     always allowed.\n+\t */\n+\n+\tref->nonfastforward =\n+\t\t!ref->deletion &&\n+\t\t!is_null_sha1(ref->old_sha1) &&\n+\t\t(!has_sha1_file(ref->old_sha1)\n+\t\t  || !ref_newer(ref->new_sha1, ref->old_sha1));\n+\n+\tif (ref->nonfastforward && !ref->force && !force_update) {\n+\t\tref->status = REF_STATUS_REJECT_NONFASTFORWARD;\n+\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n struct branch *branch_get(const char *name)\n {\n \tstruct branch *ret;\ndiff --git a/remote.h b/remote.h\nindex 8b7ecf9..0f45553 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -98,6 +98,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,\n\n int match_refs(struct ref *src, struct ref **dst,\n \t       int nr_refspec, const char **refspec, int all);\n+int set_ref_status_for_push(struct ref *ref, int force_update);\n\n /*\n  * Given a list of the remote refs and the specification of things to\n--\n1.6.6.rc1.286.gbc15a\n"},{"id":"129162","messageId":"20091204145755.4d793b0b.rctay89@gmail.com","threadId":"21854","inReplyTo":"20091204145437.1a9910db.rctay89@gmail.com","subject":"[PATCH resend 2/3] transport-helper.c::push_refs(): know more about refs before pushing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-04T06:57:55Z","receivedAt":"2009-12-04T06:57:55Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Know about rejected non-fast-forward refs, in addition to up-to-date\nones, by calling set_ref_status_for_push() in remote.[ch], before\npassing push commands to the helper.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n transport-helper.c |   10 +++-------\n 1 files changed, 3 insertions(+), 7 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 11f3d7e..6ed7b55 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -334,16 +334,12 @@ static int push_refs(struct transport *transport,\n \t\telse if (!mirror)\n \t\t\tcontinue;\n\n-\t\tref->deletion = is_null_sha1(ref->new_sha1);\n-\t\tif (!ref->deletion &&\n-\t\t\t!hashcmp(ref->old_sha1, ref->new_sha1)) {\n-\t\t\tref->status = REF_STATUS_UPTODATE;\n-\t\t\tcontinue;\n-\t\t}\n-\n \t\tif (force_all)\n \t\t\tref->force = 1;\n\n+\t\tif (set_ref_status_for_push(ref, force_all))\n+\t\t\tcontinue;\n+\n \t\tstrbuf_addstr(&buf, \"push \");\n \t\tif (!ref->deletion) {\n \t\t\tif (ref->force)\n--\n1.6.6.rc1.286.gbc15a\n"},{"id":"129163","messageId":"20091204145842.57c3c51c.rctay89@gmail.com","threadId":"21854","inReplyTo":"20091204145437.1a9910db.rctay89@gmail.com","subject":"[PATCH resend 3/3] transport.c::transport_push(): make ref status affect return value","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-04T06:58:42Z","receivedAt":"2009-12-04T06:58:42Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"When determining whether to print the ref status table,\npush_had_errors() is called. Piggy-back this and modify the return\nvalue accordingly.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n transport.c |   11 +++++++----\n 1 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 3eea836..32c122b 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -889,10 +889,13 @@ int transport_push(struct transport *transport,\n\n \t\tret = transport->push_refs(transport, remote_refs, flags);\n\n-\t\tif (!quiet || push_had_errors(remote_refs))\n-\t\t\tprint_push_status(transport->url, remote_refs,\n-\t\t\t\t\tverbose | porcelain, porcelain,\n-\t\t\t\t\tnonfastforward);\n+\t\tif (!quiet)\n+\t\t\tif (push_had_errors(remote_refs)) {\n+\t\t\t\tret = -1;\n+\t\t\t\tprint_push_status(transport->url, remote_refs,\n+\t\t\t\t\t\tverbose | porcelain, porcelain,\n+\t\t\t\t\t\tnonfastforward);\n+\t\t\t}\n\n \t\tif (!(flags & TRANSPORT_PUSH_DRY_RUN)) {\n \t\t\tstruct ref *ref;\n--\n1.6.6.rc1.286.gbc15a\n"},{"id":"129170","messageId":"20091204102039.GB27495@coredump.intra.peff.net","threadId":"21854","inReplyTo":"20091204145842.57c3c51c.rctay89@gmail.com","subject":"Re: [PATCH resend 3/3] transport.c::transport_push(): make ref status affect return value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-12-04T10:20:39Z","receivedAt":"2009-12-04T10:20:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 04, 2009 at 02:58:42PM +0800, Tay Ray Chuan wrote:\n\n> -\t\tif (!quiet || push_had_errors(remote_refs))\n> -\t\t\tprint_push_status(transport->url, remote_refs,\n> -\t\t\t\t\tverbose | porcelain, porcelain,\n> -\t\t\t\t\tnonfastforward);\n> +\t\tif (!quiet)\n> +\t\t\tif (push_had_errors(remote_refs)) {\n> +\t\t\t\tret = -1;\n> +\t\t\t\tprint_push_status(transport->url, remote_refs,\n> +\t\t\t\t\t\tverbose | porcelain, porcelain,\n> +\t\t\t\t\t\tnonfastforward);\n> +\t\t\t}\n\nEr, this doesn't seem right. It will no longer print anything under\nnon-quiet mode unless there are errors, and it will only set a return\nvalue in non-quiet mode.\n\nI think you want:\n\n  if (push_had_errors(remote_refs)) {\n          ret = -1;\n          if (!quiet)\n                  print_push_status(...)\n  }\n  else if (!quiet)\n          print_push_status(...)\n\nActually, it may simply be more readable by storing the error result:\n\n  int err = push_had_errors(remote_refs);\n  if (err)\n    ret = -1;\n  if (!quiet || err)\n    print_push_status(...);\n\n-Peff\n"},{"id":"131057","messageId":"1262916765-3728-1-git-send-email-rctay89@gmail.com","threadId":"21854","inReplyTo":"20091204102039.GB27495@coredump.intra.peff.net","subject":"[PATCH v4 0/6] transport: catch non-fast forwards","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-08T02:12:39Z","receivedAt":"2010-01-08T02:12:39Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Junio, this is re-roll of the 'tr/http-push-ref-status' branch in 'pu'.\n\nJeff, please don't see this re-roll as me assuming you gave your go-\nahead for the latest discussion; I didn't, I thought squashing in the\nhunks I mentioned would make things easier for you, rather than having\nthem floating around.\n\nSummary:\n  This patch series applies on top of 'next', and deals with alerting\n  the user to rejected non-fast-forward pushes when using helpers\n  (smart).\n\n  Previously, git silently exited. This situation involves the curl\n  helper and the smart protocol. The non-fast-forward push is only\n  detected when curl executes the rpc client (git-send-pack). Now, we\n  detect it before telling the helper to push.\n\n  The series also remedies the scenario where a user would not be aware\n  of rejected non-fast-forward pushes.\n\n  It occurs when\n    1) there are one or more pushes that succeed and\n    2) there are one or more rejected non-fast-forward pushes that\n\t   involve refs that cannot be matched without explicit refspecs.\n\n  This is due to the re-marking of ref status in transport-helper.c::\n  push_refs() when interacting with the remote helper.\n\n  If only non-matched, non-fast-forward refs are involved (ie.\n  condition #2 is present without #1), then the situtation does not\n  occur - no 'push' commands are passed to the remote helper, no\n  interaction with the helper takes place, and no re-marking of ref\n  status takes place. The user will be alerted to the rejected\n  non-fast-forward push.\n\n  If both are present, re-marking occurs. Even if a ref was not part\n  of a 'push' command to the helper, 'ok'/'error' status reports might\n  be made for it, since the remote helper does ref matching on its\n  side. This led to refs that cannot be matched without an explicit\n  refspec marked as REF_STATUS_REJECT_NONFASTFORWARD to be re-marked\n  REF_STATUS_NONE.\n\n  (Note: helpers fail to match refs as explicit refspecs are always\n  available to the top-level transport mechanism, but only on a need-\n  to-know basis to the remote helper via a 'push' command when a ref\n  is to be pushed.)\n\n  Interestingly, it is possible for a user to be forever unaware of\n  the rejected push if the user relies on information from git push\n  alone. As long as the user ensures that one or more other pushes are\n  successful (eg. change a tracked ref fast-forwardedly) between the\n  execution of git push, the rejected non-fast-forward push won't be\n  reported.\n\nChanges from v3:\n - reworded commit message for the test in patch 2\n - added comment on what the switch block is checking for in patch 3\n - changed condition under which ref status reported by remote helper\n   is ignored in patch 5\n\nTay Ray Chuan (6):\n  t5541-http-push.sh: add tests for non-fast-forward pushes\n  t5541-http-push.sh: add test for unmatched, non-fast-forwarded refs\n  refactor ref status logic for pushing\n  transport.c::transport_push(): make ref status affect return value\n  transport-helper.c::push_refs(): ignore helper-reported status if ref\n    is not to be pushed\n  transport-helper.c::push_refs(): emit \"no refs\" error message\n\n builtin-send-pack.c  |   51 +++++++++++--------------------------------------\n remote.c             |   50 +++++++++++++++++++++++++++++++++++++++++++++++++\n remote.h             |    2 +\n t/t5541-http-push.sh |   44 +++++++++++++++++++++++++++++++++++++++++++\n transport-helper.c   |   28 +++++++++++++++++++-------\n transport.c          |   11 ++++++++-\n 6 files changed, 137 insertions(+), 49 deletions(-)\n"},{"id":"131058","messageId":"1262916765-3728-2-git-send-email-rctay89@gmail.com","threadId":"21854","inReplyTo":"1262916765-3728-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v4 1/6] t5541-http-push.sh: add tests for non-fast-forward pushes","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-08T02:12:40Z","receivedAt":"2010-01-08T02:12:40Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n t/t5541-http-push.sh |   23 +++++++++++++++++++++++\n 1 files changed, 23 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex 2a58d0c..f49c7c4 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -88,5 +88,28 @@ test_expect_success 'used receive-pack service' '\n \ttest_cmp exp act\n '\n \n+test_expect_success 'non-fast-forward push fails' '\n+\tcd \"$ROOT_PATH\"/test_repo_clone &&\n+\tgit checkout master &&\n+\techo \"changed\" > path2 &&\n+\tgit commit -a -m path2 --amend &&\n+\n+\tHEAD=$(git rev-parse --verify HEAD) &&\n+\t!(git push -v origin >output 2>&1) &&\n+\t(cd \"$HTTPD_DOCUMENT_ROOT_PATH\"/test_repo.git &&\n+\t test $HEAD != $(git rev-parse --verify HEAD))\n+'\n+\n+test_expect_failure 'non-fast-forward push show ref status' '\n+\tgrep \"^ ! \\[rejected\\][ ]*master -> master (non-fast-forward)$\" output\n+'\n+\n+test_expect_failure 'non-fast-forward push shows help message' '\n+\tgrep \\\n+\"To prevent you from losing history, non-fast-forward updates were rejected\n+Merge the remote changes before pushing again.  See the '\"'non-fast-forward'\"'\n+section of '\"'git push --help'\"' for details.\" output\n+'\n+\n stop_httpd\n test_done\n-- \n1.6.6.341.ga7aec\n"},{"id":"131059","messageId":"1262916765-3728-3-git-send-email-rctay89@gmail.com","threadId":"21854","inReplyTo":"1262916765-3728-2-git-send-email-rctay89@gmail.com","subject":"[PATCH v4 2/6] t5541-http-push.sh: add test for unmatched, non-fast-forwarded refs","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-08T02:12:41Z","receivedAt":"2010-01-08T02:12:41Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Some refs can only be matched to a remote ref with an explicit refspec.\nWhen such a ref is a non-fast-forward of its remote ref,  test that\npushing them (with the explicit refspec specified) fails with a non-\nfast-foward-type error (viz. printing of ref status and help message).\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n\nChanged from v3:\n - Reworded commit message\n - Reword the comments\n - Used '*' instead of '\\+' for grep expressions\n - Used [a-f0-9] instead of [a-z0-9] for matching hexadecimals\n - Used ' ' instead of '[ ]' for matching SP\n\n t/t5541-http-push.sh |   21 +++++++++++++++++++++\n 1 files changed, 21 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex f49c7c4..cc740fe 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -111,5 +111,26 @@ Merge the remote changes before pushing again.  See the '\"'non-fast-forward'\"'\n section of '\"'git push --help'\"' for details.\" output\n '\n\n+test_expect_failure 'push fails for non-fast-forward refs unmatched by remote helper' '\n+\t# create a dissimilarly-named remote ref so that git is unable to match the\n+\t# two refs (viz. local, remote) unless an explicit refspec is provided.\n+\tgit push origin master:retsam\n+\n+\techo \"change changed\" > path2 &&\n+\tgit commit -a -m path2 --amend &&\n+\n+\t# push master too; this ensures there is at least one '\"'push'\"' command to\n+\t# the remote helper and triggers interaction with the helper.\n+\t!(git push -v origin +master master:retsam >output 2>&1) &&\n+\n+\tgrep \"^ + [a-f0-9]*\\.\\.\\.[a-f0-9]* *master -> master (forced update)$\" output &&\n+\tgrep \"^ ! \\[rejected\\] *master -> retsam (non-fast-forward)$\" output &&\n+\n+\tgrep \\\n+\"To prevent you from losing history, non-fast-forward updates were rejected\n+Merge the remote changes before pushing again.  See the '\"'non-fast-forward'\"'\n+section of '\"'git push --help'\"' for details.\" output\n+'\n+\n stop_httpd\n test_done\n--\n1.6.6.341.ga7aec\n"},{"id":"131060","messageId":"1262916765-3728-4-git-send-email-rctay89@gmail.com","threadId":"21854","inReplyTo":"1262916765-3728-3-git-send-email-rctay89@gmail.com","subject":"[PATCH v4 3/6] refactor ref status logic for pushing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-08T02:12:42Z","receivedAt":"2010-01-08T02:12:42Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Move the logic that detects up-to-date and non-fast-forward refs to a\nnew function in remote.[ch], set_ref_status_for_push().\n\nMake transport_push() invoke set_ref_status_for_push() before invoking\nthe push_refs() implementation. (As a side-effect, the push_refs()\nimplementation in transport-helper.c now knows of non-fast-forward\npushes.)\n\nRemoved logic for detecting up-to-date refs from the push_refs()\nimplementation in transport-helper.c, as transport_push() has already\ndone so for it.\n\nMake cmd_send_pack() invoke set_ref_status_for_push() before invoking\nsend_pack(), as transport_push() can't do it for send_pack() here.\n\nMark the test on the return status of non-fast-forward push to fail.\nGit now exits with success, as transport.c::transport_push() does not\ncheck for refs with status REF_STATUS_REJECT_NONFASTFORWARD nor does it\nindicate rejected pushes with its return value.\n\nMark the test for ref status to succeed. As mentioned earlier, refs\nmight be marked as non-fast-forwards, triggering the push status\nprinting mechanism in transport.c.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin-send-pack.c  |   51 +++++++++++--------------------------------------\n remote.c             |   50 +++++++++++++++++++++++++++++++++++++++++++++++++\n remote.h             |    2 +\n t/t5541-http-push.sh |    4 +-\n transport-helper.c   |   14 ++++++------\n transport.c          |    4 +++\n 6 files changed, 77 insertions(+), 48 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 8fffdbf..76c7206 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -406,50 +406,20 @@ int send_pack(struct send_pack_args *args,\n \t */\n \tnew_refs = 0;\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n-\n-\t\tif (ref->peer_ref)\n-\t\t\thashcpy(ref->new_sha1, ref->peer_ref->new_sha1);\n-\t\telse if (!args->send_mirror)\n+\t\tif (!ref->peer_ref && !args->send_mirror)\n \t\t\tcontinue;\n \n-\t\tref->deletion = is_null_sha1(ref->new_sha1);\n-\t\tif (ref->deletion && !allow_deleting_refs) {\n-\t\t\tref->status = REF_STATUS_REJECT_NODELETE;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!ref->deletion &&\n-\t\t    !hashcmp(ref->old_sha1, ref->new_sha1)) {\n-\t\t\tref->status = REF_STATUS_UPTODATE;\n+\t\t/* Check for statuses set by set_ref_status_for_push() */\n+\t\tswitch (ref->status) {\n+\t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n+\t\tcase REF_STATUS_UPTODATE:\n \t\t\tcontinue;\n+\t\tdefault:\n+\t\t\t; /* do nothing */\n \t\t}\n \n-\t\t/* This part determines what can overwrite what.\n-\t\t * The rules are:\n-\t\t *\n-\t\t * (0) you can always use --force or +A:B notation to\n-\t\t *     selectively force individual ref pairs.\n-\t\t *\n-\t\t * (1) if the old thing does not exist, it is OK.\n-\t\t *\n-\t\t * (2) if you do not have the old thing, you are not allowed\n-\t\t *     to overwrite it; you would not know what you are losing\n-\t\t *     otherwise.\n-\t\t *\n-\t\t * (3) if both new and old are commit-ish, and new is a\n-\t\t *     descendant of old, it is OK.\n-\t\t *\n-\t\t * (4) regardless of all of the above, removing :B is\n-\t\t *     always allowed.\n-\t\t */\n-\n-\t\tref->nonfastforward =\n-\t\t    !ref->deletion &&\n-\t\t    !is_null_sha1(ref->old_sha1) &&\n-\t\t    (!has_sha1_file(ref->old_sha1)\n-\t\t      || !ref_newer(ref->new_sha1, ref->old_sha1));\n-\n-\t\tif (ref->nonfastforward && !ref->force && !args->force_update) {\n-\t\t\tref->status = REF_STATUS_REJECT_NONFASTFORWARD;\n+\t\tif (ref->deletion && !allow_deleting_refs) {\n+\t\t\tref->status = REF_STATUS_REJECT_NODELETE;\n \t\t\tcontinue;\n \t\t}\n \n@@ -673,6 +643,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tif (match_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))\n \t\treturn -1;\n \n+\tset_ref_status_for_push(remote_refs, args.send_mirror,\n+\t\targs.force_update);\n+\n \tret = send_pack(&args, fd, conn, remote_refs, &extra_have);\n \n \tif (helper_status)\ndiff --git a/remote.c b/remote.c\nindex e3afecd..c70181c 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1247,6 +1247,56 @@ int match_refs(struct ref *src, struct ref **dst,\n \treturn 0;\n }\n \n+void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n+\tint force_update)\n+{\n+\tstruct ref *ref;\n+\n+\tfor (ref = remote_refs; ref; ref = ref->next) {\n+\t\tif (ref->peer_ref)\n+\t\t\thashcpy(ref->new_sha1, ref->peer_ref->new_sha1);\n+\t\telse if (!send_mirror)\n+\t\t\tcontinue;\n+\n+\t\tref->deletion = is_null_sha1(ref->new_sha1);\n+\t\tif (!ref->deletion &&\n+\t\t\t!hashcmp(ref->old_sha1, ref->new_sha1)) {\n+\t\t\tref->status = REF_STATUS_UPTODATE;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* This part determines what can overwrite what.\n+\t\t * The rules are:\n+\t\t *\n+\t\t * (0) you can always use --force or +A:B notation to\n+\t\t *     selectively force individual ref pairs.\n+\t\t *\n+\t\t * (1) if the old thing does not exist, it is OK.\n+\t\t *\n+\t\t * (2) if you do not have the old thing, you are not allowed\n+\t\t *     to overwrite it; you would not know what you are losing\n+\t\t *     otherwise.\n+\t\t *\n+\t\t * (3) if both new and old are commit-ish, and new is a\n+\t\t *     descendant of old, it is OK.\n+\t\t *\n+\t\t * (4) regardless of all of the above, removing :B is\n+\t\t *     always allowed.\n+\t\t */\n+\n+\t\tref->nonfastforward =\n+\t\t\t!ref->deletion &&\n+\t\t\t!is_null_sha1(ref->old_sha1) &&\n+\t\t\t(!has_sha1_file(ref->old_sha1)\n+\t\t\t  || !ref_newer(ref->new_sha1, ref->old_sha1));\n+\n+\t\tif (ref->nonfastforward && !ref->force && !force_update) {\n+\t\t\tref->status = REF_STATUS_REJECT_NONFASTFORWARD;\n+\t\t\tcontinue;\n+\t\t}\n+\t}\n+}\n+\n struct branch *branch_get(const char *name)\n {\n \tstruct branch *ret;\ndiff --git a/remote.h b/remote.h\nindex 8b7ecf9..6e13643 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -98,6 +98,8 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,\n \n int match_refs(struct ref *src, struct ref **dst,\n \t       int nr_refspec, const char **refspec, int all);\n+void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n+\tint force_update);\n \n /*\n  * Given a list of the remote refs and the specification of things to\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex cc740fe..6d92196 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -88,7 +88,7 @@ test_expect_success 'used receive-pack service' '\n \ttest_cmp exp act\n '\n \n-test_expect_success 'non-fast-forward push fails' '\n+test_expect_failure 'non-fast-forward push fails' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \tgit checkout master &&\n \techo \"changed\" > path2 &&\n@@ -100,7 +100,7 @@ test_expect_success 'non-fast-forward push fails' '\n \t test $HEAD != $(git rev-parse --verify HEAD))\n '\n \n-test_expect_failure 'non-fast-forward push show ref status' '\n+test_expect_success 'non-fast-forward push show ref status' '\n \tgrep \"^ ! \\[rejected\\][ ]*master -> master (non-fast-forward)$\" output\n '\n \ndiff --git a/transport-helper.c b/transport-helper.c\nindex 11f3d7e..7c9b569 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -329,16 +329,16 @@ static int push_refs(struct transport *transport,\n \t\treturn 1;\n \n \tfor (ref = remote_refs; ref; ref = ref->next) {\n-\t\tif (ref->peer_ref)\n-\t\t\thashcpy(ref->new_sha1, ref->peer_ref->new_sha1);\n-\t\telse if (!mirror)\n+\t\tif (!ref->peer_ref && !mirror)\n \t\t\tcontinue;\n \n-\t\tref->deletion = is_null_sha1(ref->new_sha1);\n-\t\tif (!ref->deletion &&\n-\t\t\t!hashcmp(ref->old_sha1, ref->new_sha1)) {\n-\t\t\tref->status = REF_STATUS_UPTODATE;\n+\t\t/* Check for statuses set by set_ref_status_for_push() */\n+\t\tswitch (ref->status) {\n+\t\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n+\t\tcase REF_STATUS_UPTODATE:\n \t\t\tcontinue;\n+\t\tdefault:\n+\t\t\t; /* do nothing */\n \t\t}\n \n \t\tif (force_all)\ndiff --git a/transport.c b/transport.c\nindex 3eea836..12c4423 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -887,6 +887,10 @@ int transport_push(struct transport *transport,\n \t\t\treturn -1;\n \t\t}\n \n+\t\tset_ref_status_for_push(remote_refs,\n+\t\t\tflags & TRANSPORT_PUSH_MIRROR,\n+\t\t\tflags & TRANSPORT_PUSH_FORCE);\n+\n \t\tret = transport->push_refs(transport, remote_refs, flags);\n \n \t\tif (!quiet || push_had_errors(remote_refs))\n-- \n1.6.6.341.ga7aec\n"},{"id":"131061","messageId":"1262916765-3728-5-git-send-email-rctay89@gmail.com","threadId":"21854","inReplyTo":"1262916765-3728-4-git-send-email-rctay89@gmail.com","subject":"[PATCH v4 4/6] transport.c::transport_push(): make ref status affect return value","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-08T02:12:43Z","receivedAt":"2010-01-08T02:12:43Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Use push_had_errors() to check the refs for errors and modify the\nreturn value.\n\nMark the non-fast-forward push tests to succeed.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n t/t5541-http-push.sh |    4 ++--\n transport.c          |    7 +++++--\n 2 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex 6d92196..979624d 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -88,7 +88,7 @@ test_expect_success 'used receive-pack service' '\n \ttest_cmp exp act\n '\n \n-test_expect_failure 'non-fast-forward push fails' '\n+test_expect_success 'non-fast-forward push fails' '\n \tcd \"$ROOT_PATH\"/test_repo_clone &&\n \tgit checkout master &&\n \techo \"changed\" > path2 &&\n@@ -104,7 +104,7 @@ test_expect_success 'non-fast-forward push show ref status' '\n \tgrep \"^ ! \\[rejected\\][ ]*master -> master (non-fast-forward)$\" output\n '\n \n-test_expect_failure 'non-fast-forward push shows help message' '\n+test_expect_success 'non-fast-forward push shows help message' '\n \tgrep \\\n \"To prevent you from losing history, non-fast-forward updates were rejected\n Merge the remote changes before pushing again.  See the '\"'non-fast-forward'\"'\ndiff --git a/transport.c b/transport.c\nindex 12c4423..9b23989 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -875,7 +875,7 @@ int transport_push(struct transport *transport,\n \t\tint verbose = flags & TRANSPORT_PUSH_VERBOSE;\n \t\tint quiet = flags & TRANSPORT_PUSH_QUIET;\n \t\tint porcelain = flags & TRANSPORT_PUSH_PORCELAIN;\n-\t\tint ret;\n+\t\tint ret, err;\n \n \t\tif (flags & TRANSPORT_PUSH_ALL)\n \t\t\tmatch_flags |= MATCH_REFS_ALL;\n@@ -892,8 +892,11 @@ 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 \n-\t\tif (!quiet || push_had_errors(remote_refs))\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 \t\t\t\t\tnonfastforward);\n-- \n1.6.6.341.ga7aec\n"},{"id":"131062","messageId":"1262916765-3728-6-git-send-email-rctay89@gmail.com","threadId":"21854","inReplyTo":"1262916765-3728-5-git-send-email-rctay89@gmail.com","subject":"[PATCH v4 5/6] transport-helper.c::push_refs(): ignore helper-reported status if ref is not to be pushed","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-08T02:12:44Z","receivedAt":"2010-01-08T02:12:44Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"If the status of a ref is REF_STATUS_NONE, the remote helper will not\nbe told to push the ref (via a 'push' command).\n\nHowever, the remote helper may still act on these refs.\n\nIf the helper does act on the ref, and prints a status for it, ignore\nthe report (ie. don't overwrite the status of the ref with it, nor the\nmessage in the remote_status member) if the reported status is 'no\nmatch'.\n\nThis allows the user to be alerted to more \"interesting\" ref statuses,\nlike REF_STATUS_NONFASTFORWARD.\n\nCc: Jeff King <peff@peff.net>\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n\nChanged from v3:\n - consider a ref as marked not for pushing if its status is _not_\n   none, rather than checking if the status is nonff or uptodate.\n - ignore ref status reported by remote helper only if it reported 'no\n   match'\n\n t/t5541-http-push.sh |    2 +-\n transport-helper.c   |    9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh\nindex 979624d..83a8e14 100755\n--- a/t/t5541-http-push.sh\n+++ b/t/t5541-http-push.sh\n@@ -111,7 +111,7 @@ Merge the remote changes before pushing again.  See the '\"'non-fast-forward'\"'\n section of '\"'git push --help'\"' for details.\" output\n '\n\n-test_expect_failure 'push fails for non-fast-forward refs unmatched by remote helper' '\n+test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper' '\n \t# create a dissimilarly-named remote ref so that git is unable to match the\n \t# two refs (viz. local, remote) unless an explicit refspec is provided.\n \tgit push origin master:retsam\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 7c9b569..71a1e50 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -430,6 +430,15 @@ static int push_refs(struct transport *transport,\n \t\t\tcontinue;\n \t\t}\n\n+\t\tif (ref->status != REF_STATUS_NONE) {\n+\t\t\t/*\n+\t\t\t * Earlier, the ref was marked not to be pushed, so ignore the ref\n+\t\t\t * status reported by the remote helper if the latter is 'no match'.\n+\t\t\t */\n+\t\t\tif (status == REF_STATUS_NONE)\n+\t\t\t\tcontinue;\n+\t\t}\n+\n \t\tref->status = status;\n \t\tref->remote_status = msg;\n \t}\n--\n1.6.6.341.ga7aec\n"},{"id":"131063","messageId":"1262916765-3728-7-git-send-email-rctay89@gmail.com","threadId":"21854","inReplyTo":"1262916765-3728-6-git-send-email-rctay89@gmail.com","subject":"[PATCH v4 6/6] transport-helper.c::push_refs(): emit \"no refs\" error message","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-01-08T02:12:45Z","receivedAt":"2010-01-08T02:12:45Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Emit an error message when remote_refs is not set.\n\nThis behaviour is consistent with that of builtin-send-pack.c and\nhttp-push.c.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n transport-helper.c |    5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 71a1e50..8c0b575 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -321,8 +321,11 @@ static int push_refs(struct transport *transport,\n \tstruct child_process *helper;\n \tstruct ref *ref;\n \n-\tif (!remote_refs)\n+\tif (!remote_refs) {\n+\t\tfprintf(stderr, \"No refs in common and none specified; doing nothing.\\n\"\n+\t\t\t\"Perhaps you should specify a branch such as 'master'.\\n\");\n \t\treturn 0;\n+\t}\n \n \thelper = get_helper(transport);\n \tif (!data->push)\n-- \n1.6.6.341.ga7aec\n"}]}