diff mbox series

[1/3] merge: new autosetupmerge option 'simple' for matching branches

Message ID 89efc1e15646599753baeab38ba2399dcbe868f1.1645695940.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series adding new branch.autosetupmerge option "simple" | expand

Commit Message

Tao Klerks Feb. 24, 2022, 9:45 a.m. UTC
From: Tao Klerks <tao@klerks.biz>

The push.defaut option "simple" helps produce
predictable/understandable behavior for beginners,
where they don't accidentally push to the
"wrong" branch in centralized workflows. If they
create a local branch with a different name
and then try to do a plain push, it will
helpfully fail and explain why.

However, such users can often find themselves
confused by the behavior of git after they first
branch, and before they push. At that stage,
their upstream tracking branch is the original
remote branch, and pull (for example) behaves
very differently to how it later does when they
create their own same-name remote branch.

This commit introduces a new option to the
branch.autosetupmerge setting, "simple",
which is intended to be consistent with and
complementary to the push.default "simple"
option.

It will set up automatic tracking for a new
branch only if the remote ref is a branch and
that remote branch name matches the new local
branch name. It is a reduction in scope of
the existing default option, "true".

Signed-off-by: Tao Klerks <tao@klerks.biz>
---
 branch.c | 9 +++++++++
 branch.h | 1 +
 config.c | 3 +++
 3 files changed, 13 insertions(+)

Comments

Junio C Hamano Feb. 24, 2022, 7:20 p.m. UTC | #1
"Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Tao Klerks <tao@klerks.biz>
>
> The push.defaut option "simple" helps produce

The cover letter wrappeed around 70 columns, which was much easier
to read.

Please re-read Documentation/SubmittingPatches[[describe-changes]]
section before going forward.

> predictable/understandable behavior for beginners,
> where they don't accidentally push to the
> "wrong" branch in centralized workflows. If they
> create a local branch with a different name
> and then try to do a plain push, it will
> helpfully fail and explain why.
>
> However, such users can often find themselves
> confused by the behavior of git after they first
> branch, and before they push. At that stage,
> their upstream tracking branch is the original
> remote branch, and pull (for example) behaves
> very differently to how it later does when they
> create their own same-name remote branch.

Instead of saying "very differently", explain what happens before
and after the behaviour-change-triggering-event.

> This commit introduces a new option to the
> branch.autosetupmerge setting, "simple",
> which is intended to be consistent with and
> complementary to the push.default "simple"
> option.
>
> It will set up automatic tracking for a new
> branch only if the remote ref is a branch and
> that remote branch name matches the new local
> branch name. It is a reduction in scope of
> the existing default option, "true".
>
> Signed-off-by: Tao Klerks <tao@klerks.biz>
> ---
>  branch.c | 9 +++++++++
>  branch.h | 1 +
>  config.c | 3 +++
>  3 files changed, 13 insertions(+)
>
> diff --git a/branch.c b/branch.c
> index 6b31df539a5..246bc82ce3c 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -256,6 +256,15 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
>  		die(_("not tracking: ambiguous information for ref %s"),
>  		    orig_ref);
>  
> +	if (track == BRANCH_TRACK_SIMPLE) {
> +		// only track if remote branch name matches
> +		// (tracking.srcs must contain only one entry from find_tracked_branch with this config)

	/*
	 * Our multi-line comments look exactly
	 * like this.  They are not overly long,
	 * have their opening and closing slash-aster
	 * and aster-slash on their own line.
	 */

> +		if (strncmp(tracking.srcs->items[0].string, "refs/heads/", 11))
> +			return;
> +		if (strcmp(tracking.srcs->items[0].string + 11, new_ref))
> +			return;


Don't count hardcoded string length.  

		char *tracked_branch;
		if (!skip_prefix(tracking.srcs->items[0].string,
				 "refs/heads/", &tracked_branch) ||
		    strcmp(tracked_branch, new_ref))
			return;

or something along the line, perhaps?

But the post-context in this hunk makes the refernece to items[0] in
the above look very wrong.  It says tracking.srcs may not have even
a single item at this point in the original code flow.  If that is
true, the above reference to ->items[0] may not be safely done at
all.

Also, what happens when there are more than one in the items[]
array?  What makes it sensible to use the first one, ignoring the
others?

> +	}
> +
>  	if (tracking.srcs->nr < 1)
>  		string_list_append(tracking.srcs, orig_ref);
>  	if (install_branch_config_multiple_remotes(config_flags, new_ref,
> diff --git a/branch.h b/branch.h
> index 04df2aa5b51..560b6b96a8f 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -12,6 +12,7 @@ enum branch_track {
>  	BRANCH_TRACK_EXPLICIT,
>  	BRANCH_TRACK_OVERRIDE,
>  	BRANCH_TRACK_INHERIT,
> +	BRANCH_TRACK_SIMPLE,
>  };
>  
>  extern enum branch_track git_branch_track;
> diff --git a/config.c b/config.c
> index e0c03d154c9..cc586ac816c 100644
> --- a/config.c
> +++ b/config.c
> @@ -1673,6 +1673,9 @@ static int git_default_branch_config(const char *var, const char *value)
>  		} else if (value && !strcmp(value, "inherit")) {
>  			git_branch_track = BRANCH_TRACK_INHERIT;
>  			return 0;
> +		} else if (value && !strcmp(value, "simple")) {
> +			git_branch_track = BRANCH_TRACK_SIMPLE;
> +			return 0;
>  		}
>  		git_branch_track = git_config_bool(var, value);
>  		return 0;

These two hunks look perfect.
diff mbox series

Patch

diff --git a/branch.c b/branch.c
index 6b31df539a5..246bc82ce3c 100644
--- a/branch.c
+++ b/branch.c
@@ -256,6 +256,15 @@  static void setup_tracking(const char *new_ref, const char *orig_ref,
 		die(_("not tracking: ambiguous information for ref %s"),
 		    orig_ref);
 
+	if (track == BRANCH_TRACK_SIMPLE) {
+		// only track if remote branch name matches
+		// (tracking.srcs must contain only one entry from find_tracked_branch with this config)
+		if (strncmp(tracking.srcs->items[0].string, "refs/heads/", 11))
+			return;
+		if (strcmp(tracking.srcs->items[0].string + 11, new_ref))
+			return;
+	}
+
 	if (tracking.srcs->nr < 1)
 		string_list_append(tracking.srcs, orig_ref);
 	if (install_branch_config_multiple_remotes(config_flags, new_ref,
diff --git a/branch.h b/branch.h
index 04df2aa5b51..560b6b96a8f 100644
--- a/branch.h
+++ b/branch.h
@@ -12,6 +12,7 @@  enum branch_track {
 	BRANCH_TRACK_EXPLICIT,
 	BRANCH_TRACK_OVERRIDE,
 	BRANCH_TRACK_INHERIT,
+	BRANCH_TRACK_SIMPLE,
 };
 
 extern enum branch_track git_branch_track;
diff --git a/config.c b/config.c
index e0c03d154c9..cc586ac816c 100644
--- a/config.c
+++ b/config.c
@@ -1673,6 +1673,9 @@  static int git_default_branch_config(const char *var, const char *value)
 		} else if (value && !strcmp(value, "inherit")) {
 			git_branch_track = BRANCH_TRACK_INHERIT;
 			return 0;
+		} else if (value && !strcmp(value, "simple")) {
+			git_branch_track = BRANCH_TRACK_SIMPLE;
+			return 0;
 		}
 		git_branch_track = git_config_bool(var, value);
 		return 0;