Re: [PATCH v6 2/5] branch: make create_branch() always create a branch
- From
Glen Choo <chooglen@google.com>
- Date
- Jan 11, 2022, 17:51 UTC
- Message-ID
- <kl6l7db6kvp2.fsf@chooglen-macbookpro.roam.corp.google.com>
- In-Reply-To
- <20220111021912.1228794-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 24 quoted lines
> Glen Choo <chooglen@google.com> writes: >> create_branch() was formerly used to set tracking without creating a >> branch. Since the previous commit replaces this use case with >> dwim_and_setup_tracking(), we can simplify create_branch() so that it >> always creates a branch. >> >> Do this simplification, in particular: >> >> * remove the special handling of BRANCH_TRACK_OVERRIDE because it is no >> longer used >> * assert that clobber_head_ok can only be provided with force >> * check that we're handling clobber_head_ok and force correctly by >> introducing tests for `git branch --force` > > This might have been more simply explained as: > > With the previous commit, these are true of create_branch(): > * BRANCH_TRACK_OVERRIDE is no longer passed > * if clobber_head_ok is true, force is also true > > Assert these situations, delete dead code, and ensure that we're > handling clobber_head_ok and force correctly by introducing tests for > `git branch --force`. This also means that create_branch() now always > creates a branch.
This is a great suggestion, I'll incorporate some of this.
As an aside, it has always been the case that clobber_head_ok is only true when force is true, so it might be misleading to include it under the umbrella of 'With the previous commit'. I'll move things around accordingly.
Show 27 quoted lines
>> @@ -426,15 +426,17 @@ void create_branch(struct repository *r, const char *name,
>> char *real_ref;
>> struct strbuf ref = STRBUF_INIT;
>> int forcing = 0;
>> - int dont_change_ref = 0;
>> -
>> - if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)
>> - ? validate_branchname(name, &ref)
>> - : validate_new_branchname(name, &ref, force)) {
>> - if (!force)
>> - dont_change_ref = 1;
>> - else
>> - forcing = 1;
>> + struct ref_transaction *transaction;
>> + struct strbuf err = STRBUF_INIT;
>> + char *msg;
>> +
>> + if (clobber_head_ok && !force)
>> + BUG("'clobber_head_ok' can only be used with 'force'");
>> +
>> + if (clobber_head_ok ?
>> + validate_branchname(name, &ref) :
>> + validate_new_branchname(name, &ref, force)) {
>> + forcing = 1;
>> }
>
> Also assert that track is not BRANCH_TRACK_OVERRIDE.Makes sense, I'll do that.