# [PATCH resend 0/3] transport: catch non-fast-forwards 12 messages from 2009-12-04 to 2010-01-08. Participants: Tay Ray Chuan, Jeff King. Thread: https://gitlist.dev/t/21854 ## Tay Ray Chuan, 2009-12-04 06:54 Subject: [PATCH resend 0/3] transport: catch non-fast-forwards Message-ID: <20091204145437.1a9910db.rctay89@gmail.com> URL: https://gitlist.dev/e/20091204145437.1a9910db.rctay89%40gmail.com ``` This patch series applies on top of 'next', and deals with alerting the user to non-fast-forward pushes when using helpers (smart). Previously, git silently exited. This situation involves the curl helper and the smart protocol. The fast-forward push is only detected when curl executes the rpc client (git-send-pack). Now, we detect it before telling the helper to push. The first patch refactors ref status logic out of builtin-send-pack.c. The second patch lets git know of non-fast-forwards before actually telling the helper to push anything. The third patch changes the return code when ref status indicate an error (determined by push_had_errors()), so that the caller of transport_push() is alerted. PS. There are at least 2 bug fixes related to this series. If you usually do so from repo.or.cz, please fetch from git.kernel.org; the former is 2 days old. builtin-send-pack.c | 39 +++------------------------------------ remote.c | 42 ++++++++++++++++++++++++++++++++++++++++++ remote.h | 1 + transport-helper.c | 10 +++------- transport.c | 11 +++++++---- 5 files changed, 56 insertions(+), 47 deletions(-) -- Cheers, Ray Chuan ``` ## Tay Ray Chuan, 2009-12-04 06:56 Subject: [PATCH resend 1/3] refactor ref status logic for pushing Message-ID: <20091204145643.296ec414.rctay89@gmail.com> URL: https://gitlist.dev/e/20091204145643.296ec414.rctay89%40gmail.com In-Reply-To: <20091204145437.1a9910db.rctay89@gmail.com> ``` Move the logic to a new function in remote.[ch], set_ref_status_for_push(). Signed-off-by: Tay Ray Chuan --- builtin-send-pack.c | 39 +++------------------------------------ remote.c | 42 ++++++++++++++++++++++++++++++++++++++++++ remote.h | 1 + 3 files changed, 46 insertions(+), 36 deletions(-) diff --git a/builtin-send-pack.c b/builtin-send-pack.c index 8fffdbf..8c98624 100644 --- a/builtin-send-pack.c +++ b/builtin-send-pack.c @@ -412,44 +412,11 @@ int send_pack(struct send_pack_args *args, else if (!args->send_mirror) continue; - ref->deletion = is_null_sha1(ref->new_sha1); - if (ref->deletion && !allow_deleting_refs) { - ref->status = REF_STATUS_REJECT_NODELETE; - continue; - } - if (!ref->deletion && - !hashcmp(ref->old_sha1, ref->new_sha1)) { - ref->status = REF_STATUS_UPTODATE; + if (set_ref_status_for_push(ref, args->force_update)) continue; - } - /* This part determines what can overwrite what. - * The rules are: - * - * (0) you can always use --force or +A:B notation to - * selectively force individual ref pairs. - * - * (1) if the old thing does not exist, it is OK. - * - * (2) if you do not have the old thing, you are not allowed - * to overwrite it; you would not know what you are losing - * otherwise. - * - * (3) if both new and old are commit-ish, and new is a - * descendant of old, it is OK. - * - * (4) regardless of all of the above, removing :B is - * always allowed. - */ - - ref->nonfastforward = - !ref->deletion && - !is_null_sha1(ref->old_sha1) && - (!has_sha1_file(ref->old_sha1) - || !ref_newer(ref->new_sha1, ref->old_sha1)); - - if (ref->nonfastforward && !ref->force && !args->force_update) { - ref->status = REF_STATUS_REJECT_NONFASTFORWARD; + if (ref->deletion && !allow_deleting_refs) { + ref->status = REF_STATUS_REJECT_NODELETE; continue; } diff --git a/remote.c b/remote.c index e3afecd..188869a 100644 --- a/remote.c +++ b/remote.c @@ -1247,6 +1247,48 @@ int match_refs(struct ref *src, struct ref **dst, return 0; } +int set_ref_status_for_push(struct ref *ref, int force_update) +{ + ref->deletion = is_null_sha1(ref->new_sha1); + if (!ref->deletion && + !hashcmp(ref->old_sha1, ref->new_sha1)) { + ref->status = REF_STATUS_UPTODATE; + return 1; + } + + /* This part determines what can overwrite what. + * The rules are: + * + * (0) you can always use --force or +A:B notation to + * selectively force individual ref pairs. + * + * (1) if the old thing does not exist, it is OK. + * + * (2) if you do not have the old thing, you are not allowed + * to overwrite it; you would not know what you are losing + * otherwise. + * + * (3) if both new and old are commit-ish, and new is a + * descendant of old, it is OK. + * + * (4) regardless of all of the above, removing :B is + * always allowed. + */ + + ref->nonfastforward = + !ref->deletion && + !is_null_sha1(ref->old_sha1) && + (!has_sha1_file(ref->old_sha1) + || !ref_newer(ref->new_sha1, ref->old_sha1)); + + if (ref->nonfastforward && !ref->force && !force_update) { + ref->status = REF_STATUS_REJECT_NONFASTFORWARD; + return 1; + } + + return 0; +} + struct branch *branch_get(const char *name) { struct branch *ret; diff --git a/remote.h b/remote.h index 8b7ecf9..0f45553 100644 --- a/remote.h +++ b/remote.h @@ -98,6 +98,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec, int match_refs(struct ref *src, struct ref **dst, int nr_refspec, const char **refspec, int all); +int set_ref_status_for_push(struct ref *ref, int force_update); /* * Given a list of the remote refs and the specification of things to -- 1.6.6.rc1.286.gbc15a ``` ## Tay Ray Chuan, 2009-12-04 06:57 Subject: [PATCH resend 2/3] transport-helper.c::push_refs(): know more about refs before pushing Message-ID: <20091204145755.4d793b0b.rctay89@gmail.com> URL: https://gitlist.dev/e/20091204145755.4d793b0b.rctay89%40gmail.com In-Reply-To: <20091204145437.1a9910db.rctay89@gmail.com> ``` Know about rejected non-fast-forward refs, in addition to up-to-date ones, by calling set_ref_status_for_push() in remote.[ch], before passing push commands to the helper. Signed-off-by: Tay Ray Chuan --- transport-helper.c | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-) diff --git a/transport-helper.c b/transport-helper.c index 11f3d7e..6ed7b55 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -334,16 +334,12 @@ static int push_refs(struct transport *transport, else if (!mirror) continue; - ref->deletion = is_null_sha1(ref->new_sha1); - if (!ref->deletion && - !hashcmp(ref->old_sha1, ref->new_sha1)) { - ref->status = REF_STATUS_UPTODATE; - continue; - } - if (force_all) ref->force = 1; + if (set_ref_status_for_push(ref, force_all)) + continue; + strbuf_addstr(&buf, "push "); if (!ref->deletion) { if (ref->force) -- 1.6.6.rc1.286.gbc15a ``` ## Tay Ray Chuan, 2009-12-04 06:58 Subject: [PATCH resend 3/3] transport.c::transport_push(): make ref status affect return value Message-ID: <20091204145842.57c3c51c.rctay89@gmail.com> URL: https://gitlist.dev/e/20091204145842.57c3c51c.rctay89%40gmail.com In-Reply-To: <20091204145437.1a9910db.rctay89@gmail.com> ``` When determining whether to print the ref status table, push_had_errors() is called. Piggy-back this and modify the return value accordingly. Signed-off-by: Tay Ray Chuan --- transport.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/transport.c b/transport.c index 3eea836..32c122b 100644 --- a/transport.c +++ b/transport.c @@ -889,10 +889,13 @@ int transport_push(struct transport *transport, ret = transport->push_refs(transport, remote_refs, flags); - if (!quiet || push_had_errors(remote_refs)) - print_push_status(transport->url, remote_refs, - verbose | porcelain, porcelain, - nonfastforward); + if (!quiet) + if (push_had_errors(remote_refs)) { + ret = -1; + print_push_status(transport->url, remote_refs, + verbose | porcelain, porcelain, + nonfastforward); + } if (!(flags & TRANSPORT_PUSH_DRY_RUN)) { struct ref *ref; -- 1.6.6.rc1.286.gbc15a ``` ## Jeff King, 2009-12-04 10:20 Subject: Re: [PATCH resend 3/3] transport.c::transport_push(): make ref status affect return value Message-ID: <20091204102039.GB27495@coredump.intra.peff.net> URL: https://gitlist.dev/e/20091204102039.GB27495%40coredump.intra.peff.net In-Reply-To: <20091204145842.57c3c51c.rctay89@gmail.com> ``` On Fri, Dec 04, 2009 at 02:58:42PM +0800, Tay Ray Chuan wrote: > - if (!quiet || push_had_errors(remote_refs)) > - print_push_status(transport->url, remote_refs, > - verbose | porcelain, porcelain, > - nonfastforward); > + if (!quiet) > + if (push_had_errors(remote_refs)) { > + ret = -1; > + print_push_status(transport->url, remote_refs, > + verbose | porcelain, porcelain, > + nonfastforward); > + } Er, this doesn't seem right. It will no longer print anything under non-quiet mode unless there are errors, and it will only set a return value in non-quiet mode. I think you want: if (push_had_errors(remote_refs)) { ret = -1; if (!quiet) print_push_status(...) } else if (!quiet) print_push_status(...) Actually, it may simply be more readable by storing the error result: int err = push_had_errors(remote_refs); if (err) ret = -1; if (!quiet || err) print_push_status(...); -Peff ``` ## Tay Ray Chuan, 2010-01-08 02:12 Subject: [PATCH v4 0/6] transport: catch non-fast forwards Message-ID: <1262916765-3728-1-git-send-email-rctay89@gmail.com> URL: https://gitlist.dev/e/1262916765-3728-1-git-send-email-rctay89%40gmail.com In-Reply-To: <20091204102039.GB27495@coredump.intra.peff.net> ``` Junio, this is re-roll of the 'tr/http-push-ref-status' branch in 'pu'. Jeff, please don't see this re-roll as me assuming you gave your go- ahead for the latest discussion; I didn't, I thought squashing in the hunks I mentioned would make things easier for you, rather than having them floating around. Summary: This patch series applies on top of 'next', and deals with alerting the user to rejected non-fast-forward pushes when using helpers (smart). Previously, git silently exited. This situation involves the curl helper and the smart protocol. The non-fast-forward push is only detected when curl executes the rpc client (git-send-pack). Now, we detect it before telling the helper to push. The series also remedies the scenario where a user would not be aware of rejected non-fast-forward pushes. It occurs when 1) there are one or more pushes that succeed and 2) there are one or more rejected non-fast-forward pushes that involve refs that cannot be matched without explicit refspecs. This is due to the re-marking of ref status in transport-helper.c:: push_refs() when interacting with the remote helper. If only non-matched, non-fast-forward refs are involved (ie. condition #2 is present without #1), then the situtation does not occur - no 'push' commands are passed to the remote helper, no interaction with the helper takes place, and no re-marking of ref status takes place. The user will be alerted to the rejected non-fast-forward push. If both are present, re-marking occurs. Even if a ref was not part of a 'push' command to the helper, 'ok'/'error' status reports might be made for it, since the remote helper does ref matching on its side. This led to refs that cannot be matched without an explicit refspec marked as REF_STATUS_REJECT_NONFASTFORWARD to be re-marked REF_STATUS_NONE. (Note: helpers fail to match refs as explicit refspecs are always available to the top-level transport mechanism, but only on a need- to-know basis to the remote helper via a 'push' command when a ref is to be pushed.) Interestingly, it is possible for a user to be forever unaware of the rejected push if the user relies on information from git push alone. As long as the user ensures that one or more other pushes are successful (eg. change a tracked ref fast-forwardedly) between the execution of git push, the rejected non-fast-forward push won't be reported. Changes from v3: - reworded commit message for the test in patch 2 - added comment on what the switch block is checking for in patch 3 - changed condition under which ref status reported by remote helper is ignored in patch 5 Tay Ray Chuan (6): t5541-http-push.sh: add tests for non-fast-forward pushes t5541-http-push.sh: add test for unmatched, non-fast-forwarded refs refactor ref status logic for pushing transport.c::transport_push(): make ref status affect return value transport-helper.c::push_refs(): ignore helper-reported status if ref is not to be pushed transport-helper.c::push_refs(): emit "no refs" error message builtin-send-pack.c | 51 +++++++++++-------------------------------------- remote.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ remote.h | 2 + t/t5541-http-push.sh | 44 +++++++++++++++++++++++++++++++++++++++++++ transport-helper.c | 28 +++++++++++++++++++------- transport.c | 11 ++++++++- 6 files changed, 137 insertions(+), 49 deletions(-) ``` ## Tay Ray Chuan, 2010-01-08 02:12 Subject: [PATCH v4 1/6] t5541-http-push.sh: add tests for non-fast-forward pushes Message-ID: <1262916765-3728-2-git-send-email-rctay89@gmail.com> URL: https://gitlist.dev/e/1262916765-3728-2-git-send-email-rctay89%40gmail.com In-Reply-To: <1262916765-3728-1-git-send-email-rctay89@gmail.com> ``` Signed-off-by: Tay Ray Chuan --- t/t5541-http-push.sh | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-) diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh index 2a58d0c..f49c7c4 100755 --- a/t/t5541-http-push.sh +++ b/t/t5541-http-push.sh @@ -88,5 +88,28 @@ test_expect_success 'used receive-pack service' ' test_cmp exp act ' +test_expect_success 'non-fast-forward push fails' ' + cd "$ROOT_PATH"/test_repo_clone && + git checkout master && + echo "changed" > path2 && + git commit -a -m path2 --amend && + + HEAD=$(git rev-parse --verify HEAD) && + !(git push -v origin >output 2>&1) && + (cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git && + test $HEAD != $(git rev-parse --verify HEAD)) +' + +test_expect_failure 'non-fast-forward push show ref status' ' + grep "^ ! \[rejected\][ ]*master -> master (non-fast-forward)$" output +' + +test_expect_failure 'non-fast-forward push shows help message' ' + grep \ +"To prevent you from losing history, non-fast-forward updates were rejected +Merge the remote changes before pushing again. See the '"'non-fast-forward'"' +section of '"'git push --help'"' for details." output +' + stop_httpd test_done -- 1.6.6.341.ga7aec ``` ## Tay Ray Chuan, 2010-01-08 02:12 Subject: [PATCH v4 2/6] t5541-http-push.sh: add test for unmatched, non-fast-forwarded refs Message-ID: <1262916765-3728-3-git-send-email-rctay89@gmail.com> URL: https://gitlist.dev/e/1262916765-3728-3-git-send-email-rctay89%40gmail.com In-Reply-To: <1262916765-3728-2-git-send-email-rctay89@gmail.com> ``` Some refs can only be matched to a remote ref with an explicit refspec. When such a ref is a non-fast-forward of its remote ref, test that pushing them (with the explicit refspec specified) fails with a non- fast-foward-type error (viz. printing of ref status and help message). Signed-off-by: Tay Ray Chuan --- Changed from v3: - Reworded commit message - Reword the comments - Used '*' instead of '\+' for grep expressions - Used [a-f0-9] instead of [a-z0-9] for matching hexadecimals - Used ' ' instead of '[ ]' for matching SP t/t5541-http-push.sh | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh index f49c7c4..cc740fe 100755 --- a/t/t5541-http-push.sh +++ b/t/t5541-http-push.sh @@ -111,5 +111,26 @@ Merge the remote changes before pushing again. See the '"'non-fast-forward'"' section of '"'git push --help'"' for details." output ' +test_expect_failure 'push fails for non-fast-forward refs unmatched by remote helper' ' + # create a dissimilarly-named remote ref so that git is unable to match the + # two refs (viz. local, remote) unless an explicit refspec is provided. + git push origin master:retsam + + echo "change changed" > path2 && + git commit -a -m path2 --amend && + + # push master too; this ensures there is at least one '"'push'"' command to + # the remote helper and triggers interaction with the helper. + !(git push -v origin +master master:retsam >output 2>&1) && + + grep "^ + [a-f0-9]*\.\.\.[a-f0-9]* *master -> master (forced update)$" output && + grep "^ ! \[rejected\] *master -> retsam (non-fast-forward)$" output && + + grep \ +"To prevent you from losing history, non-fast-forward updates were rejected +Merge the remote changes before pushing again. See the '"'non-fast-forward'"' +section of '"'git push --help'"' for details." output +' + stop_httpd test_done -- 1.6.6.341.ga7aec ``` ## Tay Ray Chuan, 2010-01-08 02:12 Subject: [PATCH v4 3/6] refactor ref status logic for pushing Message-ID: <1262916765-3728-4-git-send-email-rctay89@gmail.com> URL: https://gitlist.dev/e/1262916765-3728-4-git-send-email-rctay89%40gmail.com In-Reply-To: <1262916765-3728-3-git-send-email-rctay89@gmail.com> ``` Move the logic that detects up-to-date and non-fast-forward refs to a new function in remote.[ch], set_ref_status_for_push(). Make transport_push() invoke set_ref_status_for_push() before invoking the push_refs() implementation. (As a side-effect, the push_refs() implementation in transport-helper.c now knows of non-fast-forward pushes.) Removed logic for detecting up-to-date refs from the push_refs() implementation in transport-helper.c, as transport_push() has already done so for it. Make cmd_send_pack() invoke set_ref_status_for_push() before invoking send_pack(), as transport_push() can't do it for send_pack() here. Mark the test on the return status of non-fast-forward push to fail. Git now exits with success, as transport.c::transport_push() does not check for refs with status REF_STATUS_REJECT_NONFASTFORWARD nor does it indicate rejected pushes with its return value. Mark the test for ref status to succeed. As mentioned earlier, refs might be marked as non-fast-forwards, triggering the push status printing mechanism in transport.c. Signed-off-by: Tay Ray Chuan --- builtin-send-pack.c | 51 +++++++++++-------------------------------------- remote.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ remote.h | 2 + t/t5541-http-push.sh | 4 +- transport-helper.c | 14 ++++++------ transport.c | 4 +++ 6 files changed, 77 insertions(+), 48 deletions(-) diff --git a/builtin-send-pack.c b/builtin-send-pack.c index 8fffdbf..76c7206 100644 --- a/builtin-send-pack.c +++ b/builtin-send-pack.c @@ -406,50 +406,20 @@ int send_pack(struct send_pack_args *args, */ new_refs = 0; for (ref = remote_refs; ref; ref = ref->next) { - - if (ref->peer_ref) - hashcpy(ref->new_sha1, ref->peer_ref->new_sha1); - else if (!args->send_mirror) + if (!ref->peer_ref && !args->send_mirror) continue; - ref->deletion = is_null_sha1(ref->new_sha1); - if (ref->deletion && !allow_deleting_refs) { - ref->status = REF_STATUS_REJECT_NODELETE; - continue; - } - if (!ref->deletion && - !hashcmp(ref->old_sha1, ref->new_sha1)) { - ref->status = REF_STATUS_UPTODATE; + /* Check for statuses set by set_ref_status_for_push() */ + switch (ref->status) { + case REF_STATUS_REJECT_NONFASTFORWARD: + case REF_STATUS_UPTODATE: continue; + default: + ; /* do nothing */ } - /* This part determines what can overwrite what. - * The rules are: - * - * (0) you can always use --force or +A:B notation to - * selectively force individual ref pairs. - * - * (1) if the old thing does not exist, it is OK. - * - * (2) if you do not have the old thing, you are not allowed - * to overwrite it; you would not know what you are losing - * otherwise. - * - * (3) if both new and old are commit-ish, and new is a - * descendant of old, it is OK. - * - * (4) regardless of all of the above, removing :B is - * always allowed. - */ - - ref->nonfastforward = - !ref->deletion && - !is_null_sha1(ref->old_sha1) && - (!has_sha1_file(ref->old_sha1) - || !ref_newer(ref->new_sha1, ref->old_sha1)); - - if (ref->nonfastforward && !ref->force && !args->force_update) { - ref->status = REF_STATUS_REJECT_NONFASTFORWARD; + if (ref->deletion && !allow_deleting_refs) { + ref->status = REF_STATUS_REJECT_NODELETE; continue; } @@ -673,6 +643,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) if (match_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags)) return -1; + set_ref_status_for_push(remote_refs, args.send_mirror, + args.force_update); + ret = send_pack(&args, fd, conn, remote_refs, &extra_have); if (helper_status) diff --git a/remote.c b/remote.c index e3afecd..c70181c 100644 --- a/remote.c +++ b/remote.c @@ -1247,6 +1247,56 @@ int match_refs(struct ref *src, struct ref **dst, return 0; } +void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, + int force_update) +{ + struct ref *ref; + + for (ref = remote_refs; ref; ref = ref->next) { + if (ref->peer_ref) + hashcpy(ref->new_sha1, ref->peer_ref->new_sha1); + else if (!send_mirror) + continue; + + ref->deletion = is_null_sha1(ref->new_sha1); + if (!ref->deletion && + !hashcmp(ref->old_sha1, ref->new_sha1)) { + ref->status = REF_STATUS_UPTODATE; + continue; + } + + /* This part determines what can overwrite what. + * The rules are: + * + * (0) you can always use --force or +A:B notation to + * selectively force individual ref pairs. + * + * (1) if the old thing does not exist, it is OK. + * + * (2) if you do not have the old thing, you are not allowed + * to overwrite it; you would not know what you are losing + * otherwise. + * + * (3) if both new and old are commit-ish, and new is a + * descendant of old, it is OK. + * + * (4) regardless of all of the above, removing :B is + * always allowed. + */ + + ref->nonfastforward = + !ref->deletion && + !is_null_sha1(ref->old_sha1) && + (!has_sha1_file(ref->old_sha1) + || !ref_newer(ref->new_sha1, ref->old_sha1)); + + if (ref->nonfastforward && !ref->force && !force_update) { + ref->status = REF_STATUS_REJECT_NONFASTFORWARD; + continue; + } + } +} + struct branch *branch_get(const char *name) { struct branch *ret; diff --git a/remote.h b/remote.h index 8b7ecf9..6e13643 100644 --- a/remote.h +++ b/remote.h @@ -98,6 +98,8 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec, int match_refs(struct ref *src, struct ref **dst, int nr_refspec, const char **refspec, int all); +void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, + int force_update); /* * Given a list of the remote refs and the specification of things to diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh index cc740fe..6d92196 100755 --- a/t/t5541-http-push.sh +++ b/t/t5541-http-push.sh @@ -88,7 +88,7 @@ test_expect_success 'used receive-pack service' ' test_cmp exp act ' -test_expect_success 'non-fast-forward push fails' ' +test_expect_failure 'non-fast-forward push fails' ' cd "$ROOT_PATH"/test_repo_clone && git checkout master && echo "changed" > path2 && @@ -100,7 +100,7 @@ test_expect_success 'non-fast-forward push fails' ' test $HEAD != $(git rev-parse --verify HEAD)) ' -test_expect_failure 'non-fast-forward push show ref status' ' +test_expect_success 'non-fast-forward push show ref status' ' grep "^ ! \[rejected\][ ]*master -> master (non-fast-forward)$" output ' diff --git a/transport-helper.c b/transport-helper.c index 11f3d7e..7c9b569 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -329,16 +329,16 @@ static int push_refs(struct transport *transport, return 1; for (ref = remote_refs; ref; ref = ref->next) { - if (ref->peer_ref) - hashcpy(ref->new_sha1, ref->peer_ref->new_sha1); - else if (!mirror) + if (!ref->peer_ref && !mirror) continue; - ref->deletion = is_null_sha1(ref->new_sha1); - if (!ref->deletion && - !hashcmp(ref->old_sha1, ref->new_sha1)) { - ref->status = REF_STATUS_UPTODATE; + /* Check for statuses set by set_ref_status_for_push() */ + switch (ref->status) { + case REF_STATUS_REJECT_NONFASTFORWARD: + case REF_STATUS_UPTODATE: continue; + default: + ; /* do nothing */ } if (force_all) diff --git a/transport.c b/transport.c index 3eea836..12c4423 100644 --- a/transport.c +++ b/transport.c @@ -887,6 +887,10 @@ int transport_push(struct transport *transport, return -1; } + set_ref_status_for_push(remote_refs, + flags & TRANSPORT_PUSH_MIRROR, + flags & TRANSPORT_PUSH_FORCE); + ret = transport->push_refs(transport, remote_refs, flags); if (!quiet || push_had_errors(remote_refs)) -- 1.6.6.341.ga7aec ``` ## Tay Ray Chuan, 2010-01-08 02:12 Subject: [PATCH v4 4/6] transport.c::transport_push(): make ref status affect return value Message-ID: <1262916765-3728-5-git-send-email-rctay89@gmail.com> URL: https://gitlist.dev/e/1262916765-3728-5-git-send-email-rctay89%40gmail.com In-Reply-To: <1262916765-3728-4-git-send-email-rctay89@gmail.com> ``` Use push_had_errors() to check the refs for errors and modify the return value. Mark the non-fast-forward push tests to succeed. Signed-off-by: Tay Ray Chuan --- t/t5541-http-push.sh | 4 ++-- transport.c | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh index 6d92196..979624d 100755 --- a/t/t5541-http-push.sh +++ b/t/t5541-http-push.sh @@ -88,7 +88,7 @@ test_expect_success 'used receive-pack service' ' test_cmp exp act ' -test_expect_failure 'non-fast-forward push fails' ' +test_expect_success 'non-fast-forward push fails' ' cd "$ROOT_PATH"/test_repo_clone && git checkout master && echo "changed" > path2 && @@ -104,7 +104,7 @@ test_expect_success 'non-fast-forward push show ref status' ' grep "^ ! \[rejected\][ ]*master -> master (non-fast-forward)$" output ' -test_expect_failure 'non-fast-forward push shows help message' ' +test_expect_success 'non-fast-forward push shows help message' ' grep \ "To prevent you from losing history, non-fast-forward updates were rejected Merge the remote changes before pushing again. See the '"'non-fast-forward'"' diff --git a/transport.c b/transport.c index 12c4423..9b23989 100644 --- a/transport.c +++ b/transport.c @@ -875,7 +875,7 @@ int transport_push(struct transport *transport, int verbose = flags & TRANSPORT_PUSH_VERBOSE; int quiet = flags & TRANSPORT_PUSH_QUIET; int porcelain = flags & TRANSPORT_PUSH_PORCELAIN; - int ret; + int ret, err; if (flags & TRANSPORT_PUSH_ALL) match_flags |= MATCH_REFS_ALL; @@ -892,8 +892,11 @@ int transport_push(struct transport *transport, flags & TRANSPORT_PUSH_FORCE); ret = transport->push_refs(transport, remote_refs, flags); + err = push_had_errors(remote_refs); - if (!quiet || push_had_errors(remote_refs)) + ret |= err; + + if (!quiet || err) print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain, nonfastforward); -- 1.6.6.341.ga7aec ``` ## Tay Ray Chuan, 2010-01-08 02:12 Subject: [PATCH v4 5/6] transport-helper.c::push_refs(): ignore helper-reported status if ref is not to be pushed Message-ID: <1262916765-3728-6-git-send-email-rctay89@gmail.com> URL: https://gitlist.dev/e/1262916765-3728-6-git-send-email-rctay89%40gmail.com In-Reply-To: <1262916765-3728-5-git-send-email-rctay89@gmail.com> ``` If the status of a ref is REF_STATUS_NONE, the remote helper will not be told to push the ref (via a 'push' command). However, the remote helper may still act on these refs. If the helper does act on the ref, and prints a status for it, ignore the report (ie. don't overwrite the status of the ref with it, nor the message in the remote_status member) if the reported status is 'no match'. This allows the user to be alerted to more "interesting" ref statuses, like REF_STATUS_NONFASTFORWARD. Cc: Jeff King Signed-off-by: Tay Ray Chuan --- Changed from v3: - consider a ref as marked not for pushing if its status is _not_ none, rather than checking if the status is nonff or uptodate. - ignore ref status reported by remote helper only if it reported 'no match' t/t5541-http-push.sh | 2 +- transport-helper.c | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletions(-) diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh index 979624d..83a8e14 100755 --- a/t/t5541-http-push.sh +++ b/t/t5541-http-push.sh @@ -111,7 +111,7 @@ Merge the remote changes before pushing again. See the '"'non-fast-forward'"' section of '"'git push --help'"' for details." output ' -test_expect_failure 'push fails for non-fast-forward refs unmatched by remote helper' ' +test_expect_success 'push fails for non-fast-forward refs unmatched by remote helper' ' # create a dissimilarly-named remote ref so that git is unable to match the # two refs (viz. local, remote) unless an explicit refspec is provided. git push origin master:retsam diff --git a/transport-helper.c b/transport-helper.c index 7c9b569..71a1e50 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -430,6 +430,15 @@ static int push_refs(struct transport *transport, continue; } + if (ref->status != REF_STATUS_NONE) { + /* + * Earlier, the ref was marked not to be pushed, so ignore the ref + * status reported by the remote helper if the latter is 'no match'. + */ + if (status == REF_STATUS_NONE) + continue; + } + ref->status = status; ref->remote_status = msg; } -- 1.6.6.341.ga7aec ``` ## Tay Ray Chuan, 2010-01-08 02:12 Subject: [PATCH v4 6/6] transport-helper.c::push_refs(): emit "no refs" error message Message-ID: <1262916765-3728-7-git-send-email-rctay89@gmail.com> URL: https://gitlist.dev/e/1262916765-3728-7-git-send-email-rctay89%40gmail.com In-Reply-To: <1262916765-3728-6-git-send-email-rctay89@gmail.com> ``` Emit an error message when remote_refs is not set. This behaviour is consistent with that of builtin-send-pack.c and http-push.c. Signed-off-by: Tay Ray Chuan --- transport-helper.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/transport-helper.c b/transport-helper.c index 71a1e50..8c0b575 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -321,8 +321,11 @@ static int push_refs(struct transport *transport, struct child_process *helper; struct ref *ref; - if (!remote_refs) + if (!remote_refs) { + fprintf(stderr, "No refs in common and none specified; doing nothing.\n" + "Perhaps you should specify a branch such as 'master'.\n"); return 0; + } helper = get_helper(transport); if (!data->push) -- 1.6.6.341.ga7aec ```