From 5ece083fc7ffd60d38b9abf7797fbf00decd2bcc Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 21 Jan 2013 20:24:07 -0800 Subject: push: further clean up fields of "struct ref" The "nonfastforward" and "update" fields are only used while deciding what value to assign to the "status" locally in a single function. Remove them from the "struct ref". The "requires_force" field is not used to decide if the proposed update requires a --force option to succeed, or to record such a decision made elsewhere. It is used by status reporting code that the particular update was "forced". Rename it to "forced_update", and move the code to assign to it around to further clarify how it is used and what it is used for. Signed-off-by: Junio C Hamano --- remote.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'remote.c') diff --git a/remote.c b/remote.c index d3a1ca233b..3375914abc 100644 --- a/remote.c +++ b/remote.c @@ -1317,27 +1317,23 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, * passing the --force argument */ - ref->update = - !ref->deletion && - !is_null_sha1(ref->old_sha1); - - if (ref->update) { - ref->nonfastforward = + if (!ref->deletion && !is_null_sha1(ref->old_sha1)) { + int nonfastforward = !has_sha1_file(ref->old_sha1) - || !ref_newer(ref->new_sha1, ref->old_sha1); + || !ref_newer(ref->new_sha1, ref->old_sha1); if (!prefixcmp(ref->name, "refs/tags/")) { - ref->requires_force = 1; if (!force_ref_update) { ref->status = REF_STATUS_REJECT_ALREADY_EXISTS; continue; } - } else if (ref->nonfastforward) { - ref->requires_force = 1; + ref->forced_update = 1; + } else if (nonfastforward) { if (!force_ref_update) { ref->status = REF_STATUS_REJECT_NONFASTFORWARD; continue; } + ref->forced_update = 1; } } } -- cgit v1.2.1 From 0f4d498dbecbc1b6da66f926df3bc12446bd44dd Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 23 Jan 2013 13:14:48 -0800 Subject: push: further simplify the logic to assign rejection reason First compute the reason why this push would fail if done without "--force", and then fail it by assigning that reason when the push was not forced (or if there is no reason to require force, allow it to succeed). Record the fact that the push was forced in the forced_update field only when the push would have failed without the option. The code becomes shorter, less repetitive and easier to read this way, especially given that the set of rejection reasons will be extended in a later patch. Signed-off-by: Junio C Hamano --- remote.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) (limited to 'remote.c') diff --git a/remote.c b/remote.c index 3375914abc..969aa11690 100644 --- a/remote.c +++ b/remote.c @@ -1318,23 +1318,18 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, */ if (!ref->deletion && !is_null_sha1(ref->old_sha1)) { - int nonfastforward = - !has_sha1_file(ref->old_sha1) - || !ref_newer(ref->new_sha1, ref->old_sha1); - - if (!prefixcmp(ref->name, "refs/tags/")) { - if (!force_ref_update) { - ref->status = REF_STATUS_REJECT_ALREADY_EXISTS; - continue; - } - ref->forced_update = 1; - } else if (nonfastforward) { - if (!force_ref_update) { - ref->status = REF_STATUS_REJECT_NONFASTFORWARD; - continue; - } + int why = 0; /* why would this push require --force? */ + + if (!prefixcmp(ref->name, "refs/tags/")) + why = REF_STATUS_REJECT_ALREADY_EXISTS; + else if (!has_sha1_file(ref->old_sha1) + || !ref_newer(ref->new_sha1, ref->old_sha1)) + why = REF_STATUS_REJECT_NONFASTFORWARD; + + if (!force_ref_update) + ref->status = why; + else if (why) ref->forced_update = 1; - } } } } -- cgit v1.2.1 From 75e5c0dc5529aed42122b3a774e6b17383e51b66 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 23 Jan 2013 13:55:30 -0800 Subject: push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE When we push to update an existing ref, if: * the object at the tip of the remote is not a commit; or * the object we are pushing is not a commit, it won't be correct to suggest to fetch, integrate and push again, as the old and new objects will not "merge". We should explain that the push must be forced when there is a non-committish object is involved in such a case. If we do not have the current object at the tip of the remote, we do not even know that object, when fetched, is something that can be merged. In such a case, suggesting to pull first just like non-fast-forward case may not be technically correct, but in practice, most such failures are seen when you try to push your work to a branch without knowing that somebody else already pushed to update the same branch since you forked, so "pull first" would work as a suggestion most of the time. And if the object at the tip is not a commit, "pull first" will fail, without making any permanent damage. As a side effect, it also makes the error message the user will get during the next "push" attempt easier to understand, now the user is aware that a non-commit object is involved. In these cases, the current code already rejects such a push on the client end, but we used the same error and advice messages as the ones used when rejecting a non-fast-forward push, i.e. pull from there and integrate before pushing again. Introduce new rejection reasons and reword the messages appropriately. [jc: with help by Peff on message details] 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 969aa11690..a772e747b7 100644 --- a/remote.c +++ b/remote.c @@ -1322,8 +1322,12 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, if (!prefixcmp(ref->name, "refs/tags/")) why = REF_STATUS_REJECT_ALREADY_EXISTS; - else if (!has_sha1_file(ref->old_sha1) - || !ref_newer(ref->new_sha1, ref->old_sha1)) + else if (!has_sha1_file(ref->old_sha1)) + why = REF_STATUS_REJECT_FETCH_FIRST; + else if (!lookup_commit_reference_gently(ref->old_sha1, 1) || + !lookup_commit_reference_gently(ref->new_sha1, 1)) + why = REF_STATUS_REJECT_NEEDS_FORCE; + else if (!ref_newer(ref->new_sha1, ref->old_sha1)) why = REF_STATUS_REJECT_NONFASTFORWARD; if (!force_ref_update) @@ -1512,7 +1516,8 @@ int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1) struct commit_list *list, *used; int found = 0; - /* Both new and old must be commit-ish and new is descendant of + /* + * Both new and old must be commit-ish and new is descendant of * old. Otherwise we require --force. */ o = deref_tag(parse_object(old_sha1), NULL, 0); -- cgit v1.2.1