From: Siddharth Asthana Date: Wed, 10 Sep 2025 20:26:06 GMT Subject: Re: [PATCH 2/2] replay: document --update-refs and --batch options Message-ID: <58064c0d-1139-4c57-ae34-756e52bf5695@gmail.com> In-Reply-To: On 09/09/25 12:56, Christian Couder wrote: > Hi Siddharth, > > On Tue, Sep 9, 2025 at 8:36 AM Siddharth Asthana > wrote: >> On 08/09/25 11:30, Christian Couder wrote: >>> On Mon, Sep 8, 2025 at 6:36 AM Siddharth Asthana >>>> Also document the --batch option which can be used with --update-refs >>>> to allow partial failures in ref updates. >>> It looks like a --update option was also added by the previous patch. >>> Is it documented here too? >>> >>> Why was this [--update | --update-refs [--batch]] set of options >>> selected over other possibilities like for example >>> [--update-iteratively | --update-atomically | --update-batch]? >> I was trying to provide both simple and advanced modes. --update for >> users who just want "make it work like piping to git update-ref --stdin" >> and --update-refs for those who want control over transaction modes. But >> I see this creates confusion. >> >> Would you prefer a single option like --update-refs with an optional >> mode parameter? Something like --update-refs[=batch] where default is >> atomic? > My preference would be something like [--update-atomically | > --update-batch] first. (Maybe names like `--batch-update` and > `--atomic-update` are better?) > > And then something like --update-iteratively could perhaps be added as > an alternative, if: > > - it works exactly the same as piping to `git update-ref --stdin`, and > - some users want to use it to blindly replace piping to `git > update-ref --stdin`, and > - we document that it is not efficient (compared to > update-atomically and --update-batch) and should only be used to > blindly (bug for bug) replace piping to `git update-ref --stdin` when > performance is not an issue. > >>> Also how does this --update-refs option compare to the --update-refs >>> option in git rebase? Is it working in the same way? >> No, they are different. git rebase --update-refs updates refs that point >> to commits being rebased. --update-refs updates the target branches from >> the replay operation itself. The naming collision is unfortunate should >> I use a different name? Hi Christian, > Yeah, my opinion is that "rebase" and "replay" are commands doing > similar things, so having an `--update-refs` option in both commands > is a good thing only if the option has the same purpose in both > commands. If the purpose is a bit different, I think it's better to > use different names to avoid confusion. You make an excellent point about the naming collision. The purposes are indeed different: - `git rebase --update-refs` updates refs that point to commits being rebased - `git replay --update-refs` (in my patch) updates the target branches from the replay operation Since Elijah and Junio have endorsed making ref updates the default behavior, this actually simplifies our naming significantly. The new design would be: - Default: atomic ref updates using transactions (no flag needed) - `--output-commands`: print update commands for traditional pipeline users - `--allow-partial`: enable partial failure tolerance when some refs can't be updated This completely avoids the rebase naming collision while providing the atomic transaction behavior that's important for server-side operations like Gitaly. The default behavior gives us the reliability we need without any naming confusion. > >>>> +--update-refs:: >>>> + Update the relevant refs using ref transactions instead of outputting >>>> + update-ref commands. By default, uses atomic mode where all ref updates >>>> + succeed or all fail. >>> This seems to imply that --update doesn't update the refs atomically. >> That correct --update doesn't use transactions it updates refs one by >> one like `git update-ref --stdin` does. Should I make this clearer in >> the documentation? > Yes, please. > >>>> Use with `--batch` to allow partial updates. >>> What about --update, when should it be used? >> Good point. My thinking was --update for simple cases where you want the >> exact same behavior as piping to `git update-ref --stdin` and >> --update-refs when you want transaction guarantees. But I am starting to >> think this distinction might be confusing users more than helping them. >> >> Would it be cleaner to just have --update-refs with the batch mode >> option and drop --update entirely? The sequential behavior can be >> achieved with --update-refs --batch if someone really needs it. > About the options that should be implemented, see my opinion above. > > About possible confusion, I think that to avoid it, it is important to: > > - name the options properly (see above what I think about the > `--update-refs` name), and to > > - document thoroughly how all the options differ from each other and > from piping to `git update-ref --stdin` > > Thanks. Absolutely agree. The simplified approach with default atomic behavior eliminates most of the confusion points you identified. I will ensure the documentation clearly explains when users would want `--output-commands` (for custom scripting) versus  the default atomic behavior (for reliable operations). The atomic-by-default approach also means better performance since we're using batched transactions (addressing Patrick's reftable concerns) and better UX since users get reliable behavior without needing to understand transaction modes. Thanks for catching the naming issue christian - it led to a much cleaner design, Siddharth