Re: [PATCH v5 2/2] branch: add flags and config to inherit tracking

From: Glen Choo <chooglen@google.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Josh Steadmon" <steadmon@google.com>
Cc: git@vger.kernel.org, gitster@pobox.com, emilyshaffer@google.com
Subject: Re: [PATCH v5 2/2] branch: add flags and config to inherit tracking
Date: Tue, 07 Dec 2021 16:35:41 -0800	[thread overview]
Message-ID: <kl6lilw0aq76.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <211207.86ilw0pydg.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> @@ -10,7 +10,8 @@ enum branch_track {
>>  	BRANCH_TRACK_REMOTE,
>>  	BRANCH_TRACK_ALWAYS,
>>  	BRANCH_TRACK_EXPLICIT,
>> -	BRANCH_TRACK_OVERRIDE
>> +	BRANCH_TRACK_OVERRIDE,
>> +	BRANCH_TRACK_INHERIT
>>  };
>
> So we've got 5 items in this enum...
>
>>  
>>  extern enum branch_track git_branch_track;
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index b23b1d1752..ebde5023c3 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>>  		OPT__VERBOSE(&filter.verbose,
>>  			N_("show hash and subject, give twice for upstream branch")),
>>  		OPT__QUIET(&quiet, N_("suppress informational messages")),
>> -		OPT_SET_INT('t', "track",  &track, N_("set up tracking mode (see git-pull(1))"),
>> -			BRANCH_TRACK_EXPLICIT),
>> +		OPT_CALLBACK_F('t', "track",  &track, "direct|inherit",
>> +			N_("set branch tracking configuration"),
>> +			PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
>> +			parse_opt_tracking_mode),
>>  		OPT_SET_INT_F(0, "set-upstream", &track, N_("do not use"),
>>  			BRANCH_TRACK_OVERRIDE, PARSE_OPT_HIDDEN),
>
> But map --track, --track=direct --track=inherit to 3/5 of them. Will it
> ever make sense to do the oher 2/5 (I really haven't checked)....

Reasonable question, but I believe the answer is no, it doesn't make
sense to map all the values:

* BRANCH_TRACK_REMOTE is just a default value as far as I can tell (I
  don't think this does anything?)
* BRANCH_TRACK_ALWAYS behaves like BRANCH_TRACK_EXPLICIT but it's only
  meant to be set from config files, see 9ed36cfa35 (branch: optionally
  setup branch.*.merge from upstream local branches, 2008-02-19). We're
  more lenient with _ALWAYS than with _EXPLICIT; e.g. we don't die()
  when the upstream doesn't exist.

Even one of the other options doesn't really make that much sense...

* BRANCH_TRACK_OVERRIDE used to be used to implement --set-upstream, but
  that's not necessary any more. Now it's used to make create_branch()
  *not* create a branch sometimes, but that's going away if I get my
  refactor of create_branch()
  (https://lore.kernel.org/git/xmqq1r2pcnyw.fsf@gitster.g/T/#u) :)