From b24e6047a8da3cddfd686e6a9157ed4bac28ed4f Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:34 -0600 Subject: 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 --- remote.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'remote.c') 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) && -- cgit v1.2.1 From ffe81ef2ac5bcf83b9ab792e4d05ec95744a2fb6 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:35 -0600 Subject: 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 --- remote.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'remote.c') 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; + } } } } -- cgit v1.2.1 From 8c5f6f717d136c5a0e9d6d3879bf2a7bdeb42154 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:36 -0600 Subject: 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 --- remote.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'remote.c') 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; + } } } } -- cgit v1.2.1 From dbfeddb12e5bb540ed3c852eebda3df9117bd150 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:37 -0600 Subject: 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 --- remote.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'remote.c') 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; -- cgit v1.2.1 From 40eff1799983b958d6dbe09fb499ad505bcf6f8d Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:38 -0600 Subject: 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 --- remote.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'remote.c') 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. -- cgit v1.2.1 From 80054cf9d53aad76631797d76ce33a56c855c61a Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:39 -0600 Subject: 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(+) (limited to 'remote.c') 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; } -- cgit v1.2.1 From a272b2896dcfd2758e97e0f35cb14df4436d7101 Mon Sep 17 00:00:00 2001 From: Chris Rorvick Date: Thu, 29 Nov 2012 19:41:40 -0600 Subject: 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(-) (limited to 'remote.c') 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); -- cgit v1.2.1