From 10643d4ec3b9c5898d93d1c20e98b2ff1906bf79 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:33 -0600 Subject: [PATCH 01/10] push: return reject reasons as a bitset Pass all rejection reasons back from transport_push(). The logic is simpler and more flexible with regard to providing useful feedback. Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- builtin/push.c | 13 ++++--------- builtin/send-pack.c | 4 ++-- transport.c | 17 ++++++++--------- transport.h | 9 +++++---- 4 files changed, 19 insertions(+), 24 deletions(-) diff --git a/builtin/push.c b/builtin/push.c index db9ba30b08..9d17fc799c 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -244,7 +244,7 @@ static void advise_checkout_pull_push(void) static int push_with_options(struct transport *transport, int flags) { int err; - int nonfastforward; + unsigned int reject_reasons; transport_set_verbosity(transport, verbosity, progress); @@ -257,7 +257,7 @@ static int push_with_options(struct transport *transport, int flags) if (verbosity > 0) fprintf(stderr, _("Pushing to %s\n"), transport->url); err = transport_push(transport, refspec_nr, refspec, flags, - &nonfastforward); + &reject_reasons); if (err != 0) error(_("failed to push some refs to '%s'"), transport->url); @@ -265,18 +265,13 @@ static int push_with_options(struct transport *transport, int flags) if (!err) return 0; - switch (nonfastforward) { - default: - break; - case NON_FF_HEAD: + if (reject_reasons & REJECT_NON_FF_HEAD) { advise_pull_before_push(); - break; - case NON_FF_OTHER: + } else if (reject_reasons & REJECT_NON_FF_OTHER) { if (default_matching_used) advise_use_upstream(); else advise_checkout_pull_push(); - break; } return 1; diff --git a/builtin/send-pack.c b/builtin/send-pack.c index d34201372d..9f986077aa 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -85,7 +85,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) int send_all = 0; const char *receivepack = "git-receive-pack"; int flags; - int nonfastforward = 0; + unsigned int reject_reasons; int progress = -1; argv++; @@ -223,7 +223,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) ret |= finish_connect(conn); if (!helper_status) - transport_print_push_status(dest, remote_refs, args.verbose, 0, &nonfastforward); + transport_print_push_status(dest, remote_refs, args.verbose, 0, &reject_reasons); if (!args.dry_run && remote) { struct ref *ref; diff --git a/transport.c b/transport.c index 9932f402df..d4568e7b37 100644 --- a/transport.c +++ b/transport.c @@ -714,7 +714,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i } void transport_print_push_status(const char *dest, struct ref *refs, - int verbose, int porcelain, int *nonfastforward) + int verbose, int porcelain, unsigned int *reject_reasons) { struct ref *ref; int n = 0; @@ -733,18 +733,17 @@ void transport_print_push_status(const char *dest, struct ref *refs, if (ref->status == REF_STATUS_OK) n += print_one_push_status(ref, dest, n, porcelain); - *nonfastforward = 0; + *reject_reasons = 0; for (ref = refs; ref; ref = ref->next) { if (ref->status != REF_STATUS_NONE && ref->status != REF_STATUS_UPTODATE && ref->status != REF_STATUS_OK) n += print_one_push_status(ref, dest, n, porcelain); - if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD && - *nonfastforward != NON_FF_HEAD) { + if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) { if (!strcmp(head, ref->name)) - *nonfastforward = NON_FF_HEAD; + *reject_reasons |= REJECT_NON_FF_HEAD; else - *nonfastforward = NON_FF_OTHER; + *reject_reasons |= REJECT_NON_FF_OTHER; } } } @@ -1031,9 +1030,9 @@ static void die_with_unpushed_submodules(struct string_list *needs_pushing) int transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags, - int *nonfastforward) + unsigned int *reject_reasons) { - *nonfastforward = 0; + *reject_reasons = 0; transport_verify_remote_names(refspec_nr, refspec); if (transport->push) { @@ -1099,7 +1098,7 @@ int transport_push(struct transport *transport, if (!quiet || err) transport_print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain, - nonfastforward); + reject_reasons); if (flags & TRANSPORT_PUSH_SET_UPSTREAM) set_upstreams(transport, remote_refs, pretend); diff --git a/transport.h b/transport.h index 4a61c0c3f2..404b113014 100644 --- a/transport.h +++ b/transport.h @@ -140,11 +140,12 @@ int transport_set_option(struct transport *transport, const char *name, void transport_set_verbosity(struct transport *transport, int verbosity, int force_progress); -#define NON_FF_HEAD 1 -#define NON_FF_OTHER 2 +#define REJECT_NON_FF_HEAD 0x01 +#define REJECT_NON_FF_OTHER 0x02 + int transport_push(struct transport *connection, int refspec_nr, const char **refspec, int flags, - int * nonfastforward); + unsigned int * reject_reasons); const struct ref *transport_get_remote_refs(struct transport *transport); @@ -170,7 +171,7 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v int transport_refs_pushed(struct ref *ref); void transport_print_push_status(const char *dest, struct ref *refs, - int verbose, int porcelain, int *nonfastforward); + int verbose, int porcelain, unsigned int *reject_reasons); typedef void alternate_ref_fn(const struct ref *, void *); extern void for_each_alternate_ref(alternate_ref_fn, void *); From b24e6047a8da3cddfd686e6a9157ed4bac28ed4f Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:34 -0600 Subject: [PATCH 02/10] push: add advice for rejected tag reference Advising the user to fetch and merge only makes sense if the rejected reference is a branch. If none of the rejections are for branches, just tell the user the reference already exists. Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- builtin/push.c | 11 +++++++++++ cache.h | 1 + remote.c | 10 ++++++++++ transport.c | 2 ++ transport.h | 1 + 5 files changed, 25 insertions(+) diff --git a/builtin/push.c b/builtin/push.c index 9d17fc799c..e08485d123 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -220,6 +220,10 @@ static const char message_advice_checkout_pull_push[] = "(e.g. 'git pull') before pushing again.\n" "See the 'Note about fast-forwards' in 'git push --help' for details."); +static const char message_advice_ref_already_exists[] = + N_("Updates were rejected because the destination reference already exists\n" + "in the remote and the update is not a fast-forward."); + static void advise_pull_before_push(void) { if (!advice_push_non_ff_current || !advice_push_nonfastforward) @@ -241,6 +245,11 @@ static void advise_checkout_pull_push(void) advise(_(message_advice_checkout_pull_push)); } +static void advise_ref_already_exists(void) +{ + advise(_(message_advice_ref_already_exists)); +} + static int push_with_options(struct transport *transport, int flags) { int err; @@ -272,6 +281,8 @@ static int push_with_options(struct transport *transport, int flags) advise_use_upstream(); else advise_checkout_pull_push(); + } else if (reject_reasons & REJECT_ALREADY_EXISTS) { + advise_ref_already_exists(); } return 1; diff --git a/cache.h b/cache.h index dbd8018b58..d72b64d8b0 100644 --- a/cache.h +++ b/cache.h @@ -1002,6 +1002,7 @@ struct ref { unsigned int force:1, merge:1, nonfastforward:1, + not_forwardable:1, deletion:1; enum { REF_STATUS_NONE = 0, diff --git a/remote.c b/remote.c index 04fd9ea4bd..51016831b9 100644 --- a/remote.c +++ b/remote.c @@ -1279,6 +1279,14 @@ int match_push_refs(struct ref *src, struct ref **dst, return 0; } +static inline int is_forwardable(struct ref* ref) +{ + if (!prefixcmp(ref->name, "refs/tags/")) + return 0; + + return 1; +} + void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, int force_update) { @@ -1316,6 +1324,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, * always allowed. */ + ref->not_forwardable = !is_forwardable(ref); + ref->nonfastforward = !ref->deletion && !is_null_sha1(ref->old_sha1) && diff --git a/transport.c b/transport.c index d4568e7b37..bc31e8e66b 100644 --- a/transport.c +++ b/transport.c @@ -740,6 +740,8 @@ void transport_print_push_status(const char *dest, struct ref *refs, ref->status != REF_STATUS_OK) n += print_one_push_status(ref, dest, n, porcelain); if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) { + if (ref->not_forwardable) + *reject_reasons |= REJECT_ALREADY_EXISTS; if (!strcmp(head, ref->name)) *reject_reasons |= REJECT_NON_FF_HEAD; else diff --git a/transport.h b/transport.h index 404b113014..bfd2df5823 100644 --- a/transport.h +++ b/transport.h @@ -142,6 +142,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity, #define REJECT_NON_FF_HEAD 0x01 #define REJECT_NON_FF_OTHER 0x02 +#define REJECT_ALREADY_EXISTS 0x04 int transport_push(struct transport *connection, int refspec_nr, const char **refspec, int flags, From ffe81ef2ac5bcf83b9ab792e4d05ec95744a2fb6 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:35 -0600 Subject: [PATCH 03/10] push: keep track of "update" state separately If the reference exists on the remote and it is not being removed, then mark as an update. This is in preparation for handling tags (lightweight and annotated) exceptionally. Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- cache.h | 1 + remote.c | 18 +++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/cache.h b/cache.h index d72b64d8b0..722321c6da 100644 --- a/cache.h +++ b/cache.h @@ -1003,6 +1003,7 @@ struct ref { merge:1, nonfastforward:1, not_forwardable:1, + update:1, deletion:1; enum { REF_STATUS_NONE = 0, diff --git a/remote.c b/remote.c index 51016831b9..07040b8824 100644 --- a/remote.c +++ b/remote.c @@ -1326,15 +1326,19 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, ref->not_forwardable = !is_forwardable(ref); - ref->nonfastforward = + ref->update = !ref->deletion && - !is_null_sha1(ref->old_sha1) && - (!has_sha1_file(ref->old_sha1) - || !ref_newer(ref->new_sha1, ref->old_sha1)); + !is_null_sha1(ref->old_sha1); - if (ref->nonfastforward && !ref->force && !force_update) { - ref->status = REF_STATUS_REJECT_NONFASTFORWARD; - continue; + if (ref->update) { + ref->nonfastforward = + !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; + } } } } From 8c5f6f717d136c5a0e9d6d3879bf2a7bdeb42154 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:36 -0600 Subject: [PATCH 04/10] push: flag updates that require force Add a flag for indicating an update to a reference requires force. Currently the `nonfastforward` flag is used for this when generating the status message. A separate flag insulates dependent logic from the details of set_ref_status_for_push(). Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- cache.h | 4 +++- remote.c | 11 ++++++++--- transport.c | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cache.h b/cache.h index 722321c6da..b7ab4ac624 100644 --- a/cache.h +++ b/cache.h @@ -999,7 +999,9 @@ struct ref { unsigned char old_sha1[20]; unsigned char new_sha1[20]; char *symref; - unsigned int force:1, + unsigned int + force:1, + requires_force:1, merge:1, nonfastforward:1, not_forwardable:1, diff --git a/remote.c b/remote.c index 07040b8824..4a6f822089 100644 --- a/remote.c +++ b/remote.c @@ -1293,6 +1293,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, struct ref *ref; for (ref = remote_refs; ref; ref = ref->next) { + int force_ref_update = ref->force || force_update; + if (ref->peer_ref) hashcpy(ref->new_sha1, ref->peer_ref->new_sha1); else if (!send_mirror) @@ -1335,9 +1337,12 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, !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; + if (ref->nonfastforward) { + ref->requires_force = 1; + if (!force_ref_update) { + ref->status = REF_STATUS_REJECT_NONFASTFORWARD; + continue; + } } } } diff --git a/transport.c b/transport.c index bc31e8e66b..f3160b142f 100644 --- a/transport.c +++ b/transport.c @@ -659,7 +659,7 @@ static void print_ok_ref_status(struct ref *ref, int porcelain) const char *msg; strcpy(quickref, status_abbrev(ref->old_sha1)); - if (ref->nonfastforward) { + if (ref->requires_force) { strcat(quickref, "..."); type = '+'; msg = "forced update"; From dbfeddb12e5bb540ed3c852eebda3df9117bd150 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:37 -0600 Subject: [PATCH 05/10] push: require force for refs under refs/tags/ References are allowed to update from one commit-ish to another if the former is an ancestor of the latter. This behavior is oriented to branches which are expected to move with commits. Tag references are expected to be static in a repository, though, thus an update to something under refs/tags/ should be rejected unless the update is forced. Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- Documentation/git-push.txt | 11 ++++++----- builtin/push.c | 2 +- builtin/send-pack.c | 5 +++++ cache.h | 1 + remote.c | 18 ++++++++++++++---- send-pack.c | 1 + t/t5516-fetch-push.sh | 23 ++++++++++++++++++++++- transport-helper.c | 6 ++++++ transport.c | 8 ++++++-- 9 files changed, 62 insertions(+), 13 deletions(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index fe46c4258a..09bdec75bc 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -51,11 +51,12 @@ be named. If `:` is omitted, the same ref as will be updated. + The object referenced by is used to update the reference -on the remote side, but by default this is only allowed if the -update can fast-forward . By having the optional leading `+`, -you can tell git to update the ref even when the update is not a -fast-forward. This does *not* attempt to merge into . See -EXAMPLES below for details. +on the remote side. By default this is only allowed if is not +under refs/tags/, and then only if it can fast-forward . By having +the optional leading `+`, you can tell git to update the ref even +if it is not allowed by default (e.g., it is not a fast-forward.) This +does *not* attempt to merge into . See EXAMPLES below for +details. + `tag ` means the same as `refs/tags/:refs/tags/`. + diff --git a/builtin/push.c b/builtin/push.c index e08485d123..83a3cc80c3 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -222,7 +222,7 @@ static const char message_advice_checkout_pull_push[] = static const char message_advice_ref_already_exists[] = N_("Updates were rejected because the destination reference already exists\n" - "in the remote and the update is not a fast-forward."); + "in the remote."); static void advise_pull_before_push(void) { diff --git a/builtin/send-pack.c b/builtin/send-pack.c index 9f986077aa..f849e0a4a0 100644 --- a/builtin/send-pack.c +++ b/builtin/send-pack.c @@ -44,6 +44,11 @@ static void print_helper_status(struct ref *ref) msg = "non-fast forward"; break; + case REF_STATUS_REJECT_ALREADY_EXISTS: + res = "error"; + msg = "already exists"; + break; + case REF_STATUS_REJECT_NODELETE: case REF_STATUS_REMOTE_REJECT: res = "error"; diff --git a/cache.h b/cache.h index b7ab4ac624..a32a0ea91c 100644 --- a/cache.h +++ b/cache.h @@ -1011,6 +1011,7 @@ struct ref { REF_STATUS_NONE = 0, REF_STATUS_OK, REF_STATUS_REJECT_NONFASTFORWARD, + REF_STATUS_REJECT_ALREADY_EXISTS, REF_STATUS_REJECT_NODELETE, REF_STATUS_UPTODATE, REF_STATUS_REMOTE_REJECT, diff --git a/remote.c b/remote.c index 4a6f822089..012b52f6fd 100644 --- a/remote.c +++ b/remote.c @@ -1315,14 +1315,18 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, * * (1) if the old thing does not exist, it is OK. * - * (2) if you do not have the old thing, you are not allowed + * (2) if the destination is under refs/tags/ you are + * not allowed to overwrite it; tags are expected + * to be static once created + * + * (3) 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 + * (4) 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 + * (5) regardless of all of the above, removing :B is * always allowed. */ @@ -1337,7 +1341,13 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, !has_sha1_file(ref->old_sha1) || !ref_newer(ref->new_sha1, ref->old_sha1); - if (ref->nonfastforward) { + if (ref->not_forwardable) { + ref->requires_force = 1; + if (!force_ref_update) { + ref->status = REF_STATUS_REJECT_ALREADY_EXISTS; + continue; + } + } else if (ref->nonfastforward) { ref->requires_force = 1; if (!force_ref_update) { ref->status = REF_STATUS_REJECT_NONFASTFORWARD; diff --git a/send-pack.c b/send-pack.c index f50dfd9f48..1c375f0a28 100644 --- a/send-pack.c +++ b/send-pack.c @@ -229,6 +229,7 @@ int send_pack(struct send_pack_args *args, /* Check for statuses set by set_ref_status_for_push() */ switch (ref->status) { case REF_STATUS_REJECT_NONFASTFORWARD: + case REF_STATUS_REJECT_ALREADY_EXISTS: case REF_STATUS_UPTODATE: continue; default: diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index b5417cc951..8f024a08f0 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -368,7 +368,7 @@ test_expect_success 'push with colon-less refspec (2)' ' git branch -D frotz fi && git tag -f frotz && - git push testrepo frotz && + git push -f testrepo frotz && check_push_result $the_commit tags/frotz && check_push_result $the_first_commit heads/frotz @@ -929,6 +929,27 @@ test_expect_success 'push into aliased refs (inconsistent)' ' ) ' +test_expect_success 'push requires --force to update lightweight tag' ' + mk_test heads/master && + mk_child child1 && + mk_child child2 && + ( + cd child1 && + git tag Tag && + git push ../child2 Tag && + git push ../child2 Tag && + >file1 && + git add file1 && + git commit -m "file1" && + git tag -f Tag && + test_must_fail git push ../child2 Tag && + git push --force ../child2 Tag && + git tag -f Tag && + test_must_fail git push ../child2 Tag HEAD~ && + git push --force ../child2 Tag + ) +' + test_expect_success 'push --porcelain' ' mk_empty && echo >.git/foo "To testrepo" && diff --git a/transport-helper.c b/transport-helper.c index 4713b69302..965b778cb3 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -661,6 +661,11 @@ static void push_update_ref_status(struct strbuf *buf, free(msg); msg = NULL; } + else if (!strcmp(msg, "already exists")) { + status = REF_STATUS_REJECT_ALREADY_EXISTS; + free(msg); + msg = NULL; + } } if (*ref) @@ -720,6 +725,7 @@ static int push_refs_with_push(struct transport *transport, /* Check for statuses set by set_ref_status_for_push() */ switch (ref->status) { case REF_STATUS_REJECT_NONFASTFORWARD: + case REF_STATUS_REJECT_ALREADY_EXISTS: case REF_STATUS_UPTODATE: continue; default: diff --git a/transport.c b/transport.c index f3160b142f..2673d273ff 100644 --- a/transport.c +++ b/transport.c @@ -695,6 +695,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i print_ref_status('!', "[rejected]", ref, ref->peer_ref, "non-fast-forward", porcelain); break; + case REF_STATUS_REJECT_ALREADY_EXISTS: + print_ref_status('!', "[rejected]", ref, ref->peer_ref, + "already exists", porcelain); + break; case REF_STATUS_REMOTE_REJECT: print_ref_status('!', "[remote rejected]", ref, ref->deletion ? NULL : ref->peer_ref, @@ -740,12 +744,12 @@ void transport_print_push_status(const char *dest, struct ref *refs, ref->status != REF_STATUS_OK) n += print_one_push_status(ref, dest, n, porcelain); if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) { - if (ref->not_forwardable) - *reject_reasons |= REJECT_ALREADY_EXISTS; if (!strcmp(head, ref->name)) *reject_reasons |= REJECT_NON_FF_HEAD; else *reject_reasons |= REJECT_NON_FF_OTHER; + } else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS) { + *reject_reasons |= REJECT_ALREADY_EXISTS; } } } From 40eff1799983b958d6dbe09fb499ad505bcf6f8d Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:38 -0600 Subject: [PATCH 06/10] push: require force for annotated tags Do not allow fast-forwarding of references that point to a tag object. Updating from a tag is potentially destructive since it would likely leave the tag dangling. Disallowing updates to a tag also makes sense semantically and is consistent with the behavior of lightweight tags. Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- Documentation/git-push.txt | 10 +++++----- remote.c | 11 +++++++++-- t/t5516-fetch-push.sh | 21 +++++++++++++++++++++ 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 09bdec75bc..7a04ce5f21 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -52,11 +52,11 @@ updated. + The object referenced by is used to update the reference on the remote side. By default this is only allowed if is not -under refs/tags/, and then only if it can fast-forward . By having -the optional leading `+`, you can tell git to update the ref even -if it is not allowed by default (e.g., it is not a fast-forward.) This -does *not* attempt to merge into . See EXAMPLES below for -details. +a tag (annotated or lightweight), and then only if it can fast-forward +. By having the optional leading `+`, you can tell git to update +the ref even if it is not allowed by default (e.g., it is not a +fast-forward.) This does *not* attempt to merge into . See +EXAMPLES below for details. + `tag ` means the same as `refs/tags/:refs/tags/`. + diff --git a/remote.c b/remote.c index 012b52f6fd..f5bc4e7260 100644 --- a/remote.c +++ b/remote.c @@ -1281,9 +1281,16 @@ int match_push_refs(struct ref *src, struct ref **dst, static inline int is_forwardable(struct ref* ref) { + struct object *o; + if (!prefixcmp(ref->name, "refs/tags/")) return 0; + /* old object must be a commit */ + o = parse_object(ref->old_sha1); + if (!o || o->type != OBJ_COMMIT) + return 0; + return 1; } @@ -1323,8 +1330,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, * to overwrite it; you would not know what you are losing * otherwise. * - * (4) if both new and old are commit-ish, and new is a - * descendant of old, it is OK. + * (4) if old is a commit and new is a descendant of old + * (implying new is commit-ish), it is OK. * * (5) regardless of all of the above, removing :B is * always allowed. diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 8f024a08f0..60093728fe 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -950,6 +950,27 @@ test_expect_success 'push requires --force to update lightweight tag' ' ) ' +test_expect_success 'push requires --force to update annotated tag' ' + mk_test heads/master && + mk_child child1 && + mk_child child2 && + ( + cd child1 && + git tag -a -m "message 1" Tag && + git push ../child2 Tag:refs/tmp/Tag && + git push ../child2 Tag:refs/tmp/Tag && + >file1 && + git add file1 && + git commit -m "file1" && + git tag -f -a -m "message 2" Tag && + test_must_fail git push ../child2 Tag:refs/tmp/Tag && + git push --force ../child2 Tag:refs/tmp/Tag && + git tag -f -a -m "message 3" Tag HEAD~ && + test_must_fail git push ../child2 Tag:refs/tmp/Tag && + git push --force ../child2 Tag:refs/tmp/Tag + ) +' + test_expect_success 'push --porcelain' ' mk_empty && echo >.git/foo "To testrepo" && From 80054cf9d53aad76631797d76ce33a56c855c61a Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:39 -0600 Subject: [PATCH 07/10] push: clarify rejection of update to non-commit-ish Pushes must already (by default) update to a commit-ish due to the fast- forward check in set_ref_status_for_push(). But rejecting for not being a fast-forward suggests the situation can be resolved with a merge. Flag these updates (i.e., to a blob or a tree) as not forwardable so the user is presented with more appropriate advice. While updating *from* a tag object is potentially destructive, updating *to* a tag is not. Additionally, a push to the refs/tags/ hierarchy is already excluded from fast-forwarding, and refs/heads/ is protected from anything but commit objects by a check in write_ref_sha1(). Thus someone fast-forwarding to a tag is probably not doing so by accident. Since updating to a tag is benign and unlikely to cause confusion, allow it in case someone finds the behavior useful. Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- remote.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/remote.c b/remote.c index f5bc4e7260..ee0c1e54bc 100644 --- a/remote.c +++ b/remote.c @@ -1291,6 +1291,11 @@ static inline int is_forwardable(struct ref* ref) if (!o || o->type != OBJ_COMMIT) return 0; + /* new object must be commit-ish */ + o = deref_tag(parse_object(ref->new_sha1), NULL, 0); + if (!o || o->type != OBJ_COMMIT) + return 0; + return 1; } From a272b2896dcfd2758e97e0f35cb14df4436d7101 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:40 -0600 Subject: [PATCH 08/10] push: cleanup push rules comment Rewrite to remove inter-dependencies amongst the rules. Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- remote.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/remote.c b/remote.c index ee0c1e54bc..aa6b7199b7 100644 --- a/remote.c +++ b/remote.c @@ -1319,27 +1319,29 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, continue; } - /* This part determines what can overwrite what. - * The rules are: + /* + * The below logic determines whether an individual + * refspec A:B can be pushed. The push will succeed + * if any of the following are true: * - * (0) you can always use --force or +A:B notation to - * selectively force individual ref pairs. + * (1) the remote reference B does not exist * - * (1) if the old thing does not exist, it is OK. + * (2) the remote reference B is being removed (i.e., + * pushing :B where no source is specified) * - * (2) if the destination is under refs/tags/ you are - * not allowed to overwrite it; tags are expected - * to be static once created + * (3) the update meets all fast-forwarding criteria: * - * (3) if you do not have the old thing, you are not allowed - * to overwrite it; you would not know what you are losing - * otherwise. + * (a) the destination is not under refs/tags/ + * (b) the old is a commit + * (c) the new is a descendant of the old * - * (4) if old is a commit and new is a descendant of old - * (implying new is commit-ish), it is OK. + * NOTE: We must actually have the old object in + * order to overwrite it in the remote reference, + * and the new object must be commit-ish. These are + * implied by (b) and (c) respectively. * - * (5) regardless of all of the above, removing :B is - * always allowed. + * (4) it is forced using the +A:B notation, or by + * passing the --force argument */ ref->not_forwardable = !is_forwardable(ref); From 1184564eac8ef6c82da068a31f60aee0d6870265 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Sun, 2 Dec 2012 21:27:50 -0600 Subject: [PATCH 09/10] push: rename config variable for more general use The 'pushNonFastForward' advice config can be used to squelch several instances of push-related advice. Rename it to 'pushUpdateRejected' to cover other reject scenarios that are unrelated to fast-forwarding. Retain the old name for compatibility. Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- Documentation/config.txt | 2 +- advice.c | 7 +++++-- advice.h | 2 +- builtin/push.c | 6 +++--- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 9a0544cf1f..92903f22d9 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -140,7 +140,7 @@ advice.*:: can tell Git that you do not need help by setting these to 'false': + -- - pushNonFastForward:: + pushUpdateRejected:: Set this variable to 'false' if you want to disable 'pushNonFFCurrent', 'pushNonFFDefault', and 'pushNonFFMatching' simultaneously. diff --git a/advice.c b/advice.c index edfbd4a6fb..329e0773db 100644 --- a/advice.c +++ b/advice.c @@ -1,6 +1,6 @@ #include "cache.h" -int advice_push_nonfastforward = 1; +int advice_push_update_rejected = 1; int advice_push_non_ff_current = 1; int advice_push_non_ff_default = 1; int advice_push_non_ff_matching = 1; @@ -14,7 +14,7 @@ static struct { const char *name; int *preference; } advice_config[] = { - { "pushnonfastforward", &advice_push_nonfastforward }, + { "pushupdaterejected", &advice_push_update_rejected }, { "pushnonffcurrent", &advice_push_non_ff_current }, { "pushnonffdefault", &advice_push_non_ff_default }, { "pushnonffmatching", &advice_push_non_ff_matching }, @@ -23,6 +23,9 @@ static struct { { "resolveconflict", &advice_resolve_conflict }, { "implicitidentity", &advice_implicit_identity }, { "detachedhead", &advice_detached_head }, + + /* make this an alias for backward compatibility */ + { "pushnonfastforward", &advice_push_update_rejected } }; void advise(const char *advice, ...) diff --git a/advice.h b/advice.h index f3cdbbf29e..c28ef8ac23 100644 --- a/advice.h +++ b/advice.h @@ -3,7 +3,7 @@ #include "git-compat-util.h" -extern int advice_push_nonfastforward; +extern int advice_push_update_rejected; extern int advice_push_non_ff_current; extern int advice_push_non_ff_default; extern int advice_push_non_ff_matching; diff --git a/builtin/push.c b/builtin/push.c index 83a3cc80c3..cf5ecfaf04 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -226,21 +226,21 @@ static const char message_advice_ref_already_exists[] = static void advise_pull_before_push(void) { - if (!advice_push_non_ff_current || !advice_push_nonfastforward) + if (!advice_push_non_ff_current || !advice_push_update_rejected) return; advise(_(message_advice_pull_before_push)); } static void advise_use_upstream(void) { - if (!advice_push_non_ff_default || !advice_push_nonfastforward) + if (!advice_push_non_ff_default || !advice_push_update_rejected) return; advise(_(message_advice_use_upstream)); } static void advise_checkout_pull_push(void) { - if (!advice_push_non_ff_matching || !advice_push_nonfastforward) + if (!advice_push_non_ff_matching || !advice_push_update_rejected) return; advise(_(message_advice_checkout_pull_push)); } From b450568209c8ae270d26ee7fda2e4687ad8a5327 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Sun, 2 Dec 2012 21:27:51 -0600 Subject: [PATCH 10/10] push: allow already-exists advice to be disabled Add 'advice.pushAlreadyExists' option to disable the advice shown when an update is rejected for a reference that is not allowed to update at all (verses those that are allowed to fast-forward.) Signed-off-by: Chris Rorvick Signed-off-by: Junio C Hamano --- Documentation/config.txt | 8 ++++++-- advice.c | 2 ++ advice.h | 1 + builtin/push.c | 2 ++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 92903f22d9..90e7d10bad 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -142,8 +142,9 @@ advice.*:: -- pushUpdateRejected:: Set this variable to 'false' if you want to disable - 'pushNonFFCurrent', 'pushNonFFDefault', and - 'pushNonFFMatching' simultaneously. + 'pushNonFFCurrent', 'pushNonFFDefault', + 'pushNonFFMatching', and 'pushAlreadyExists' + simultaneously. pushNonFFCurrent:: Advice shown when linkgit:git-push[1] fails due to a non-fast-forward update to the current branch. @@ -158,6 +159,9 @@ advice.*:: 'matching refs' explicitly (i.e. you used ':', or specified a refspec that isn't your current branch) and it resulted in a non-fast-forward error. + pushAlreadyExists:: + Shown when linkgit:git-push[1] rejects an update that + does not qualify for fast-forwarding (e.g., a tag.) statusHints:: Show directions on how to proceed from the current state in the output of linkgit:git-status[1] and in diff --git a/advice.c b/advice.c index 329e0773db..d287927280 100644 --- a/advice.c +++ b/advice.c @@ -4,6 +4,7 @@ int advice_push_update_rejected = 1; int advice_push_non_ff_current = 1; int advice_push_non_ff_default = 1; int advice_push_non_ff_matching = 1; +int advice_push_already_exists = 1; int advice_status_hints = 1; int advice_commit_before_merge = 1; int advice_resolve_conflict = 1; @@ -18,6 +19,7 @@ static struct { { "pushnonffcurrent", &advice_push_non_ff_current }, { "pushnonffdefault", &advice_push_non_ff_default }, { "pushnonffmatching", &advice_push_non_ff_matching }, + { "pushalreadyexists", &advice_push_already_exists }, { "statushints", &advice_status_hints }, { "commitbeforemerge", &advice_commit_before_merge }, { "resolveconflict", &advice_resolve_conflict }, diff --git a/advice.h b/advice.h index c28ef8ac23..8bf63563a5 100644 --- a/advice.h +++ b/advice.h @@ -7,6 +7,7 @@ extern int advice_push_update_rejected; extern int advice_push_non_ff_current; extern int advice_push_non_ff_default; extern int advice_push_non_ff_matching; +extern int advice_push_already_exists; extern int advice_status_hints; extern int advice_commit_before_merge; extern int advice_resolve_conflict; diff --git a/builtin/push.c b/builtin/push.c index cf5ecfaf04..8491e431e4 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -247,6 +247,8 @@ static void advise_checkout_pull_push(void) static void advise_ref_already_exists(void) { + if (!advice_push_already_exists || !advice_push_update_rejected) + return; advise(_(message_advice_ref_already_exists)); }