Merge branch 'cr/push-force-tag-update'

Require "-f" for push to update a tag, even if it is a fast-forward.

* cr/push-force-tag-update:
  push: allow already-exists advice to be disabled
  push: rename config variable for more general use
  push: cleanup push rules comment
  push: clarify rejection of update to non-commit-ish
  push: require force for annotated tags
  push: require force for refs under refs/tags/
  push: flag updates that require force
  push: keep track of "update" state separately
  push: add advice for rejected tag reference
  push: return reject reasons as a bitset

1 files changed, 63 insertions, 20 deletions

diff --git a/remote.c b/remote.c
index 6aa49c03d..225073909 100644
--- a/remote.c
+++ b/remote.c
@@ -1279,12 +1279,34 @@ int match_push_refs(struct ref *src, struct ref **dst,
 	return 0;
 }
 
+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;
+
+	/* 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;
+}
+
 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) {
+		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)
@@ -1297,34 +1319,55 @@ 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 you do not have the old thing, you are not allowed
-		 *     to overwrite it; you would not know what you are losing
-		 *     otherwise.
+		 * (3) the update meets all fast-forwarding criteria:
 		 *
-		 * (3) if both new and old are commit-ish, and new is a
-		 *     descendant of old, it is OK.
+		 *     (a) the destination is not under refs/tags/
+		 *     (b) the old is a commit
+		 *     (c) the new is a descendant of the old
 		 *
-		 * (4) regardless of all of the above, removing :B is
-		 *     always allowed.
+		 *     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.
+		 *
+		 * (4) it is forced using the +A:B notation, or by
+		 *     passing the --force argument
 		 */
 
-		ref->nonfastforward =
-			!ref->deletion &&
-			!is_null_sha1(ref->old_sha1) &&
-			(!has_sha1_file(ref->old_sha1)
-			  || !ref_newer(ref->new_sha1, ref->old_sha1));
+		ref->not_forwardable = !is_forwardable(ref);
 
-		if (ref->nonfastforward && !ref->force && !force_update) {
-			ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
-			continue;
+		ref->update =
+			!ref->deletion &&
+			!is_null_sha1(ref->old_sha1);
+
+		if (ref->update) {
+			ref->nonfastforward =
+				!has_sha1_file(ref->old_sha1)
+				  || !ref_newer(ref->new_sha1, ref->old_sha1);
+
+			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;
+					continue;
+				}
+			}
 		}
 	}
 }