Re: [PATCH 1/3] merge: new autosetupmerge option 'simple' for matching branches
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 24, 2022, 19:20 UTC
- Message-ID
- <xmqqbkywm5qt.fsf@gitster.g>
- In-Reply-To
- <89efc1e15646599753baeab38ba2399dcbe868f1.1645695940.git.gitgitgadget@gmail.com>
"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.
Show 14 quoted lines
> 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.
Show 30 quoted lines
> 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?
Show 31 quoted lines
> + }
> +
> 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.