Re: [PATCH v5 00/12] rebase: update branches in multi-part topic

[Elijah Newren <newren@gmail.com>]

On Tue, Jul 19, 2022 at 11:33 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> This series is based on ds/branch-checked-out.
>
> This is a feature I've wanted for quite a while. When working on the sparse
> index topic, I created a long RFC that actually broke into three topics for
> full review upstream. These topics were sequential, so any feedback on an
> earlier one required updates to the later ones. I would work on the full
> feature and use interactive rebase to update the full list of commits.
> However, I would need to update the branches pointing to those sub-topics.
>
> This series adds a new --update-refs option to 'git rebase' (along with a
> rebase.updateRefs config option) that adds 'update-ref' commands into the
> TODO list. This is powered by the commit decoration machinery.
>
> As an example, here is my in-progress bundle URI RFC split into subtopics as
> they appear during the TODO list of a git rebase -i --update-refs:
>
> pick 2d966282ff3 docs: document bundle URI standard
> pick 31396e9171a remote-curl: add 'get' capability
> pick 54c6ab70f67 bundle-uri: create basic file-copy logic
> pick 96cb2e35af1 bundle-uri: add support for http(s):// and file://
> pick 6adaf842684 fetch: add --bundle-uri option
> pick 6c5840ed77e fetch: add 'refs/bundle/' to log.excludeDecoration
> update-ref refs/heads/bundle-redo/fetch
>
> pick 1e3f6546632 clone: add --bundle-uri option
> pick 9e4a6fe9b68 clone: --bundle-uri cannot be combined with --depth
> update-ref refs/heads/bundle-redo/clone
>
> pick 5451cb6599c bundle-uri: create bundle_list struct and helpers
> pick 3029c3aca15 bundle-uri: create base key-value pair parsing
> pick a8b2de79ce8 bundle-uri: create "key=value" line parsing
> pick 92625a47673 bundle-uri: unit test "key=value" parsing
> pick a8616af4dc2 bundle-uri: limit recursion depth for bundle lists
> pick 9d6809a8d53 bundle-uri: parse bundle list in config format
> pick 287a732b54c bundle-uri: fetch a list of bundles
> update-ref refs/heads/bundle-redo/list
>
> pick b09f8226185 protocol v2: add server-side "bundle-uri" skeleton
> pick 520204dcd1c bundle-uri client: add minimal NOOP client
> pick 62e8b457b48 bundle-uri client: add "git ls-remote-bundle-uri"
> pick 00eae925043 bundle-uri: serve URI advertisement from bundle.* config
> pick 4277440a250 bundle-uri client: add boolean transfer.bundleURI setting
> pick caf4599a81d bundle-uri: allow relative URLs in bundle lists
> pick df255000b7e bundle-uri: download bundles from an advertised list
> pick d71beabf199 clone: unbundle the advertised bundles
> pick c9578391976 t5601: basic bundle URI tests
> # Ref refs/heads/bundle-redo/rfc-3 checked out at '/home/stolee/_git/git-bundles'
>
> update-ref refs/heads/bundle-redo/advertise
>
>
> Here is an outline of the series:
>
>  * Patch 1 updates some tests for branch_checked_out() to use 'git bisect'
>    and 'git rebase' as black-boxes instead of manually editing files inside
>    $GIT_DIR. (Thanks, Junio!)
>  * Patch 2 updates some tests for branch_checked_out() for the 'apply'
>    backend.
>  * Patch 3 updates branch_checked_out() to parse the
>    rebase-merge/update-refs file to block concurrent ref updates and
>    checkouts on branches selected by --update-refs.
>  * Patch 4 updates the todo list documentation to remove some unnecessary
>    dots in the 'merge' command. This makes it consistent with the 'fixup'
>    command before we document the 'update-ref' command.
>  * Patch 5 updates the definition of todo_command_info to use enum values as
>    array indices.
>  * Patches 6-8 implement the --update-refs logic itself.
>  * Patch 9 specifically updates the update-refs file every time the user
>    edits the todo-list (Thanks Phillip!)
>  * Patch 10 adds the rebase.updateRefs config option similar to
>    rebase.autoSquash.
>  * Patch 11 ignores the HEAD ref when creating the todo list instead of
>    making a comment (Thanks Elijah!)
>  * Patch 12 adds messaging to the end of the rebase stating which refs were
>    updated (Thanks Elijah!)
>
> During review, we have identified some areas that would be good for
> #leftoverbits:
>
>  * Warn the user when they add an 'update-ref ' command but is checked out
>    in another worktree.
>  * The checks in patch 9 are quadratic. They could be sped up using
>    hashtables.
>  * Consider whether we should include an 'update-ref ' command for the HEAD
>    ref, so that all refs are updated in the same way. This might help
>    confused users.

Not necessarily so they are updated in the same way; the behind the
scenes mechanism could perhaps still be different.  Just so that if
the user looks for the "list of things being updated" they don't get
surprised that HEAD is missing.

>  * The error message for failed ref updates could include information on the
>    commit IDs that would have been used. This can help the user fix the
>    situation by updating the refs manually.
>  * Modify the --update-refs option from a boolean to an
>    optionally-string-parameter that specifies refspecs for the 'update-ref'
>    commands.

refspecs?  Is that the term you really mean here?


> Updates in v5
> =============
>
>  * Rename 'wt_dir' to 'wt_git_dir' for clarity.
>  * The documented behavior around 'fixup!' and 'squash!' commits was
>    incorrect, so update the commit message, documentation, and test to
>    demonstrate the actual behavior.
>  * Use CALLOC_ARRAY() to be more idiomatic.
>  * Be much more careful about propagating errors.
>  * Commit message typo: "We an" to "We can"
>  * Remove unnecessary null OID check when writing refs, since those would
>    already be removed by a previous step.

Thanks, I've read over the range-diff and these changes look good to
me.  One thing I'm curious about (sorry to bring this up so late):
"pick" commands come with the old commit hash.  Perhaps the update-ref
commands should too?  (e.g. "update-ref refs/heads/topic from
<OLDHASH>")