Thread (2 messages) 2 messages, 2 authors, 2024-12-04

Re: [RFC PATCH v1 1/2] fetch set_head: add warn-if-not-$branch option

From: Junio C Hamano <hidden>
Date: 2024-12-04 02:20:14

Bence Ferdinandy [off-list ref] writes:
+static void set_head_advice_msg(const char *remote, const char *head_name) {
+
+	printf("Run 'git remote set-head %s %s' to follow the change, or\n"
+		"'git config set remote.%s.warn-if-not-%s' to disable this warning\n"
+		"until the remote changes HEAD again.\n",
+		remote, head_name, remote, head_name);
+
+}
Style. "{" that encloses the function body sits on a line of its
own.

Perhaps use the advise_if_enabled(), so that those who already
learned how to deal with the situation can squelch the "how to fix"
message.
quoted hunk
-static int set_head(const struct ref *remote_refs, int follow_remote_head)
+static int set_head(const struct ref *remote_refs, int follow_remote_head,
+		const char *no_warn_branch)
 {
 	int result = 0, create_only, is_bare, was_detached;
 	struct strbuf b_head = STRBUF_INIT, b_remote_head = STRBUF_INIT,
@@ -1660,7 +1668,10 @@ static int set_head(const struct ref *remote_refs, int follow_remote_head)
 		result = 1;
 		goto cleanup;
 	}
-	if (follow_remote_head == FOLLOW_REMOTE_WARN && verbosity >= 0)
+	if ((follow_remote_head == FOLLOW_REMOTE_WARN ||
+		(follow_remote_head == FOLLOW_REMOTE_WARN_IF_NOT_BRANCH &&
+		strcmp(no_warn_branch, head_name))
+		) && verbosity >= 0)
Reorder conditions combined with &&- to have more expensive ones
later.

	if (verbosity >= 0 &&
            (follow_remote_head == FOLLOW_REMOTE_WARN ||
	    (follow_remote_head == FOLLOW_REMOTE_WARN_IF_NOT_BRANCH &&
	     strcmp(no_warn_branch, head_name)))

As human readers, we may know that no_warn_branch is always valid
when (follow_remote_head == FOLLOW_REMOTE_WARN_IF_NOT_BRANCH), but
semi clever compilers may not realize it and give a false warning
about using no_warn_branch potentially uninitialized.

We could do without adding FOLLOW_REMOTE_WARN_IF_NOT_BRANCH and reuse
FOLLOW_REMOTE_WARN, like so:

	if (verbosity >= 0 &&
            follow_remote_head == FOLLOW_REMOTE_WARN &&
	    (!no_warn_branch || strcmp(no_warn_branch, head_name)))

That is, "if set to remote-warn, always warn, or no_warn_branch is
not NULL, only warn if the current head is different from it".
quoted hunk
diff --git a/remote.c b/remote.c
index 0b18840d43..f0e1b1b76a 100644
--- a/remote.c
+++ b/remote.c
@@ -515,6 +515,7 @@ static int handle_config(const char *key, const char *value,
 		return parse_transport_option(key, value,
 					      &remote->server_options);
 	} else if (!strcmp(subkey, "followremotehead")) {
+		const char *no_warn_branch;
 		if (!strcmp(value, "never"))
 			remote->follow_remote_head = FOLLOW_REMOTE_NEVER;
 		else if (!strcmp(value, "create"))
@@ -523,6 +524,10 @@ static int handle_config(const char *key, const char *value,
 			remote->follow_remote_head = FOLLOW_REMOTE_WARN;
 		else if (!strcmp(value, "always"))
 			remote->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
+		else if (skip_prefix(value, "warn-if-not-", &no_warn_branch)) {
+			remote->follow_remote_head = FOLLOW_REMOTE_WARN_IF_NOT_BRANCH;
+			remote->no_warn_branch = no_warn_branch;
+		}
If we were to do without FOLLOW_REMOTE_WARN_IF_NOT_BRANCH, then the
above becomes

			remote->follow_remote_head = FOLLOW_REMOTE_WARN;
			remote->no_warn_branch = NULL;
		} else if (skip_prefix(value, "warn-if-not-", &no_warn_branch)) {
			remote->follow_remote_head = FOLLOW_REMOTE_WARN;
			remote->no_warn_branch = no_warn_branch;
		} else if (!strcmp(value, "always")) {
			remote->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
		} else {
			warn(_("unrecognized followRemoteHEAD value '%s' ignored"),
			     value);
		}

We'd want the new choice documented before we graduate this topic
out of the RFC status.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help