Re: [PATCH v2 5/9] reset: introduce ability to skip reference updates

[Patrick Steinhardt <ps@pks.im>]

On Tue, Jun 09, 2026 at 11:03:24AM +0100, Phillip Wood wrote:
> Hi Patrick
> 
> On 08/06/2026 10:18, Patrick Steinhardt wrote:
> > On Mon, Jun 08, 2026 at 11:14:08AM +0200, Patrick Steinhardt wrote:
> > > On Fri, Jun 05, 2026 at 04:12:42PM +0100, Phillip Wood wrote:
> > > > Hi Patrick
> > > > 
> > > > On 03/06/2026 17:14, Patrick Steinhardt wrote:
> > > > > In a subsequent commit we'll introduce a new caller to `reset_head()`
> > > > > that really only wants to update the index and working tree, without
> > > > > updating any references. Introduce a new flag that lets the caller
> > > > > perform this operation.
> > > > 
> > > > We already have a flag to update ORIG_HEAD so would it make more sense to
> > > > have a flag to update HEAD, rather than adding a flag to disable the
> > > > updates? It would mean updating the existing callers but I think it is a
> > > > clearer api and it avoids the pitfall of
> > > > 
> > > > 	RESET_HEAD_ORIG_HEAD | RESET_HEAD_SKIP_REF_UPDATES
> > > 
> > > Hm. The question is whether it's sensible to have
> > > `!RESET_HEAD_UPDATE_HEAD && RESET_HEAD_UPDATE_ORIG_HEAD`. That feels
> > > like a somewhat weird request, too, and we'd have to introduce extra
> > > logic to make that combination work.
> 
> As there are no users at the moment we could make that a BUG() and implement
> it later if required. At least that way we're not introducing conflicting
> flags, we're just not implementing setting a certain combination of refs.

Hm, okay, I'll have a look at this.

> > > > I wonder about the function name as well if we make updating HEAD optional
> > > > then what does reset_head() mean? Maybe we should rename it something along
> > > > the lines of reset_worktree() or update_working_copy()? I'm not really sure
> > > > what a good name would be.
> > > 
> > > That's a good point, the name does get somewhat awkward. I think we
> > > should keep "reset" in there, but `reset_worktree()` to me reads as it
> > > if was rather related to git-worktree(1) than anything else. Maybe
> > > `reset_working_tree()`?
> > 
> > I think I'll skip these changes for the next iteration for now. The
> > patch series has already exploded quite a bit in its scope due to the
> > refactorings of `reset_head()`, so I'd prefer to maybe do such changes
> > as a follow up.
> 
> Renaming the function can certainly wait. I'd quite like to sort out the
> flags though as the new flag is being added in this series.

If we introduce RESET_HEAD_UPDATE_HEAD I'd also like to rename so that
there's not a silent new caller where we don't have the new flag.

Patrick