On Wed, Sep 11, 2024 at 12:14:55PM -0500, Justin Tobler wrote: > 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 <ps@xxxxxx> > > --- > > 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 It's not equivalent in general, but in this case it is. Note that we are only talking about branches here, not about arbitrary references. Quoting "core.logAllRefUpdates": If this configuration variable is set to true, missing "$GIT_DIR/logs/<ref>" file is automatically created for branch heads (i.e. under refs/heads/), remote refs (i.e. under refs/remotes/), note refs (i.e. under refs/notes/), and the symbolic ref HEAD. If it is set to always, then a missing reflog is automatically created for any ref under refs/. So for branches, "true" and "always" have the same effect on the technical level. And on the conceptual level, `reflog` is set whenever "--create-reflog" is passed to git-branch(1), and asks the command to cretae the reflog regardless of "core.logAllRefUpdates". I'll add this information to the commit message. Thanks for your review! Patrick
