From: Justin Tobler Date: Wed, 11 Sep 2024 17:14:55 GMT Subject: Re: [PATCH v2 16/21] branch: stop modifying `log_all_ref_updates` variable Message-ID: <6hoykgw3gm5hchmtfaci6gwwha6iouidtqvupx55vsjfoedquo@nfgaumnmu4po> In-Reply-To: On 24/08/30 11:09AM, Patrick Steinhardt wrote: > In "branch.c" we modify the global `log_all_ref_updates` variable to > force creation of a reflog entry. Modifying global state like this is > discouraged, as it may have all kinds of consequences in other places of > our codebase. > > Stop modifying the variable and pass the `REF_FORCE_CREATE_REFLOG` flag, > which has the same effect. > > Signed-off-by: Patrick Steinhardt > --- > branch.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/branch.c b/branch.c > index c887ea21514..08fa4094d2b 100644 > --- a/branch.c > +++ b/branch.c > @@ -601,6 +601,7 @@ void create_branch(struct repository *r, > int forcing = 0; > struct ref_transaction *transaction; > struct strbuf err = STRBUF_INIT; > + int flags = 0; > char *msg; > > if (track == BRANCH_TRACK_OVERRIDE) > @@ -619,7 +620,7 @@ void create_branch(struct repository *r, > goto cleanup; > > if (reflog) > - log_all_ref_updates = LOG_REFS_NORMAL; > + flags |= REF_FORCE_CREATE_REFLOG; I'm trying to understand how setting the `REF_FORCE_CREATE_REFLOG` flag is ultimately equivalent to setting `log_all_ref_updates = LOG_REFS_NORMAL`. From my understanding, when `log_all_ref_updates` is set to `LOG_REFS_NORMAL`, this is the equivalent to `core.logAllRefUpdates=true`. This means if a missing reflog file is created for a subset of references[1]. The `REF_FORCE_CREATE_REFLOG` flag seems more analogous to `always` value for `core.logAllRefUpdates` and would create reflogs for all references. Is this not the case? Or does it not really matter? [1] https://git-scm.com/docs/git-config#Documentation/git-config.txt-corelogAllRefUpdates > > if (forcing) > msg = xstrfmt("branch: Reset to %s", start_name); > @@ -630,7 +631,7 @@ void create_branch(struct repository *r, > if (!transaction || > ref_transaction_update(transaction, ref.buf, > &oid, forcing ? NULL : null_oid(), > - NULL, NULL, 0, msg, &err) || > + NULL, NULL, flags, msg, &err) || > ref_transaction_commit(transaction, &err)) > die("%s", err.buf); > ref_transaction_free(transaction); > -- > 2.46.0.421.g159f2d50e7.dirty >