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

From: Derrick Stolee <derrickstolee@github.com>
To: Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, johannes.schindelin@gmx.de, me@ttaylorr.com,
	Jeff Hostetler <git@jeffhostetler.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH v4 00/12] rebase: update branches in multi-part topic
Date: Thu, 14 Jul 2022 10:50:56 -0400	[thread overview]
Message-ID: <73311eba-244b-3c88-d0b9-f97f426f1de6@github.com> (raw)
In-Reply-To: <xmqq8royibor.fsf@gitster.g>

[-- Attachment #1: Type: text/plain, Size: 3976 bytes --]

On 7/12/22 11:37 AM, Junio C Hamano wrote:> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> This is a tangent, but may serve as some food for thought.
> 
> When I queue (or develop myself) a topic that depends on another
> topic, I often do
> 
>     $ git checkout --detach vX.Y.Z ;# choose an appropriate base
>     $ git merge --into derived-topic base-topic
>     $ develop develop (or "git am")
> 
> which would end up in
> 
>     vX.Y.Z -----M---A---B---C derived-topic
>                /
>      base-topic
> 
> so that "git log --first-parent --oneline master.." would show the
> commits in the topic plus the fact that it depends on which other
> topic recorded in a single merge commit.  A topic that depends on
> two or more topics can be handled the same way.
> 
> One good thing about this arrangement, unlike the "totally linear"
> one depicted at the top of your cover letter, is that it is easier
> to rebuild each topic independently and the first-parent view is
> still useful.  If you futz with the base topic in a totally linear
> history, "log --decorate" of the derived topic would no longer tell
> you where the old iteration of the base topic ended.
> 
> It would be very nice to see the update-ref feature (or something
> like that) makes it easy to deal with such a topology, too.

Your topology is an interesting one, but --update-refs isn't limited to
linear history.

(Begin aside)

One goal of this feature is to make it easier to manage the topics that
are being juggled in friendly forks. For example, git-for-windows/git and
microsoft/git have some topics that evolve version-to-version but might be
cleaned up and sent upstream. Both of these forks use 'git rebase
--rebase-merges=rebase-cousins' when consuming new upstream versions to
keep the merge structure of these new topics. However, we lose the branch
names and need to reconstruct them from the context of the merge commits.

With --update-refs, we can automatically rewrite the branches that are
included in these individual topics. That might make it simpler to extract
a series to send upstream.

One test in this series does test such a case with the --rebase-merges
option.

(End aside)

Back to your topology, I wonder what your rebase command looks like when
tracking those topics.

The goal of --update-refs is to help rebase multiple branches at the same
time, and with your example here, it would imply you want to rebase both
dependent topics.

 Before:

  A---B----C---M---Q1---Q2---Q3 <-- refs/heads/Q
   \          /
    P1--P2--P3 <-- refs/heads/P

 After rebasing both topics simultaneously (with 'git rebase --update-refs
 C' while Q is checked out):

  A---B---C---D---P1---P2---P3---Q1---Q2---Q3
			    ^              ^
			refs/heads/P  refs/heads/Q

But it seems what you mean to say is to update the merge commit M, which
means that the 'P' branch above has been updated independently of the 'Q'
branch, so we need to update 'Q' after-the-fact. I'm not sure what that
rebase would look like, indepdendent of updating refs.

Do you have an example rebase command that manipulates the commits the
way you want? Then I can better understand how the --update-refs could fit
in with that. (Or maybe the point of your tangent is that there isn't an
option.)

If instead we thought about an example like re-rolling the 'next' branch
entirely on top of the 'master' branch, then we have an example that is
closer to the friendly fork example. (I know this isn't a realistic
scenario since we don't rewrite the commits already merged to 'next', but
it's an interesting stress test.)

While having 'next' checked out, I ran

  git rebase -i --update-refs --rebase-merges=rebase-cousins master

and it updated all of the branches currently in the region master..next.
I've attached the output of

  git -c log.excludeDecoration="refs/remotes/*" \
        log --oneline --graph --boundary master..next

just to show what this looks like.

Thanks,
-Stolee

[-- Attachment #2: rebase-result.txt --]
[-- Type: text/plain, Size: 13575 bytes --]

*   bdf2d8e9e1 Merge branch 'll/curl-accept-language' into next
|\  
| * b0c4adcdd7 remote-curl: send Accept-Language header to server
* |   36fcf76053 Merge branch 'jk/diff-files-cleanup-fix' into next
|\ \  
| * | 04393ae7f7 diff-files: move misplaced cleanup label
* | |   3f6ccaf58c Merge branch 'rs/cocci-array-copy' into next
|\ \ \  
| * | | dc574132a0 cocci: avoid normalization rules for memcpy
* | | |   2125d3847d Merge branch 'jk/ref-filter-discard-commit-buffer' into next
|\ \ \ \  
| * | | | 359b01ca84 ref-filter: disable save_commit_buffer while traversing
* | | | |   fd0e10a90f Merge branch 'jk/clone-unborn-confusion' into next
|\ \ \ \ \  
| * | | | | 2eb5d38386 clone: move unborn head creation to update_head()
| * | | | | 73a0858224 clone: use remote branch if it matches default HEAD
| * | | | | 54a0c1896c clone: propagate empty remote HEAD even with other branches
| * | | | | 51fa61ba3e clone: drop extra newline from warning message
| | |/ / /  
| |/| | |   
* | | | |   05d6fbe0e5 Merge branch 'hx/lookup-commit-in-graph-fix' into next
|\ \ \ \ \  
| * | | | | 854d62d9fe t5330: remove run_with_limited_processses()
* | | | | |   5386228a28 Merge branch 'jc/resolve-undo' into next
|\ \ \ \ \ \  
| * | | | | | b237e81516 fsck: do not dereference NULL while checking resolve-undo data
* | | | | | |   a34a4435d9 Merge branch 'sg/multi-pack-index-parse-options-fix' into next
|\ \ \ \ \ \ \  
| * | | | | | | cc74afb83f multi-pack-index: simplify handling of unknown --options
| | |_|_|/ / /  
| |/| | | | |   
* | | | | | |   57b36b2e9b Merge branch 'bc/nettle-sha256' into next
|\ \ \ \ \ \ \  
| * | | | | | | e555735836 sha256: add support for Nettle
| |/ / / / / /  
* | | | | | |   11cf608d10 Merge branch 'kk/p4-client-name-encoding-fix' into next
|\ \ \ \ \ \ \  
| * | | | | | | 34f67c9619 git-p4: fix bug with encoding of p4 client name
| |/ / / / / /  
* | | | | | |   99a9342263 Merge branch 'jd/gpg-interface-trust-level-string' into next
|\ \ \ \ \ \ \  
| * | | | | | | 803978da49 gpg-interface: add function for converting trust level to string
| |/ / / / / /  
* | | | | | |   47942e9ac7 Merge branch 'ab/cocci-unused' into next
|\ \ \ \ \ \ \  
| * | | | | | | 06f5f8940c cocci: generalize "unused" rule to cover more than "strbuf"
| * | | | | | | 4f40f6cb73 cocci: add and apply a rule to find "unused" strbufs
| * | | | | | | 7a9a10b10e cocci: have "coccicheck{,-pending}" depend on "coccicheck-test"
| * | | | | | | f7ff6597a7 cocci: add a "coccicheck-test" target and test *.cocci rules
| * | | | | | | af0aa6904b Makefile & .gitignore: ignore & clean "git.res", not "*.res"
| * | | | | | | 7b63ea5750 Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGS
* | | | | | | |   9720950472 Merge branch 'gc/submodule-use-super-prefix' into next
|\ \ \ \ \ \ \ \  
| * | | | | | | | 5ad87271cf submodule--helper: remove display path helper
| * | | | | | | | d7a714fddc submodule--helper update: use --super-prefix
| * | | | | | | | b0f8b21305 submodule--helper: remove unused SUPPORT_SUPER_PREFIX flags
| * | | | | | | | 58cec298f1 submodule--helper: use correct display path helper
| * | | | | | | | cb49e1e8d3 submodule--helper: don't recreate recursive prefix
| * | | | | | | | 618b8445d9 submodule--helper update: use display path helper
| * | | | | | | | 8fc36c39d9 submodule--helper tests: add missing "display path" coverage
| * | | | | | | |   c9e221b124 Merge branch 'ab/submodule-cleanup' into gc/submodule-use-super-prefix
| |\ \ \ \ \ \ \ \  
| | |_|/ / / / / /  
| |/| | | | | | |   
* | | | | | | | |   172d4d2a2b Merge branch 'en/merge-dual-dir-renames-fix' into next
|\ \ \ \ \ \ \ \ \  
| * | | | | | | | | 8fe673ee1d merge-ort: fix issue with dual rename and add/add conflict
| * | | | | | | | | 2ef5b00eba merge-ort: shuffle the computation and cleanup of potential collisions
| * | | | | | | | | b3468c2055 merge-ort: make a separate function for freeing struct collisions
| * | | | | | | | | a4719d9e46 merge-ort: small cleanups of check_for_directory_rename
| * | | | | | | | | 70fae629e6 t6423: add tests of dual directory rename plus add/add conflict
| | |_|_|_|_|/ / /  
| |/| | | | | | |   
* | | | | | | | |   9fc51f4bdd Merge branch 'ab/test-without-templates' into next
|\ \ \ \ \ \ \ \ \  
| * | | | | | | | | c8ab2aa9ce tests: don't assume a .git/info for .git/info/sparse-checkout
| * | | | | | | | | 913769a360 tests: don't assume a .git/info for .git/info/exclude
| * | | | | | | | | af4eb37df7 tests: don't assume a .git/info for .git/info/refs
| * | | | | | | | | 5383f7f9de tests: don't assume a .git/info for .git/info/attributes
| * | | | | | | | | dc7e8c3b3a tests: don't assume a .git/info for .git/info/grafts
| * | | | | | | | | fdc489c66c tests: don't depend on template-created .git/branches
| * | | | | | | | | 7a1cb29f61 t0008: don't rely on default ".git/info/exclude"
| |/ / / / / / / /  
* | | | | | | | |   8d446e7df8 Merge branch 'ab/build-gitweb' into next
|\ \ \ \ \ \ \ \ \  
| * | | | | | | | | a35258c62a gitweb/Makefile: add a "NO_GITWEB" parameter
| * | | | | | | | | d3b827408c Makefile: build 'gitweb' in the default target
| * | | | | | | | | affc3b755c gitweb/Makefile: include in top-level Makefile
| * | | | | | | | | 27438ef5e0 gitweb: remove "test" and "test-installed" targets
| * | | | | | | | | b82d66eb0c gitweb/Makefile: prepare to merge into top-level Makefile
| * | | | | | | | | 564ebde3d3 gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars
| * | | | | | | | | 1e08fa5e2b gitweb/Makefile: add a $(GITWEB_ALL) variable
| * | | | | | | | | 7decdb9b4a gitweb/Makefile: define all .PHONY prerequisites inline
* | | | | | | | | |   3ab689137a Merge branch 'ab/test-tool-leakfix' into next
|\ \ \ \ \ \ \ \ \ \  
| * | | | | | | | | | f40a693450 test-tool delta: fix a memory leak
| * | | | | | | | | | 34e691288d test-tool ref-store: fix a memory leak
| * | | | | | | | | | 9794633b4e test-tool bloom: fix memory leaks
| * | | | | | | | | | 1caaa858cc test-tool json-writer: fix memory leaks
| * | | | | | | | | | a20b0dc796 test-tool regex: call regfree(), fix memory leaks
| * | | | | | | | | | 1c343e5aef test-tool urlmatch-normalization: fix a memory leak
| * | | | | | | | | | 9afa46d4a6 test-tool {dump,scrap}-cache-tree: fix memory leaks
| * | | | | | | | | | e287a5b0a4 test-tool path-utils: fix a memory leak
| * | | | | | | | | | 330ca8501b test-tool test-hash: fix a memory leak
| | |_|/ / / / / / /  
| |/| | | | | | | |   
* | | | | | | | | |   b9183a05c5 Merge branch 'ab/leakfix' into next
|\ \ \ \ \ \ \ \ \ \  
| * | | | | | | | | | ece3974ba6 pull: fix a "struct oid_array" memory leak
| * | | | | | | | | | 27472b5195 cat-file: fix a common "struct object_context" memory leak
| * | | | | | | | | | 55916bba0f gc: fix a memory leak
| * | | | | | | | | | 33d0dda633 checkout: avoid "struct unpack_trees_options" leak
| * | | | | | | | | | e72e12cc02 merge-file: fix memory leaks on error path
| * | | | | | | | | | 480a0e30a7 merge-file: refactor for subsequent memory leak fix
| * | | | | | | | | | d90dafbe31 cat-file: fix a memory leak in --batch-command mode
| * | | | | | | | | | fd74ac95ac revert: free "struct replay_opts" members
| * | | | | | | | | | bc57ba1d54 submodule.c: free() memory from xgetcwd()
| * | | | | | | | | | 74a06a9f21 clone: fix memory leak in wanted_peer_refs()
| * | | | | | | | | | 99b6c45d8f check-ref-format: fix trivial memory leak
| |/ / / / / / / / /  
* | | | | | | | | |   542f8fec7e Merge branch 'jc/builtin-mv-move-array' into next
|\ \ \ \ \ \ \ \ \ \  
| * | | | | | | | | | 4901a36b4b builtin/mv.c: use the MOVE_ARRAY() macro instead of memmove()
| | |_|/ / / / / / /  
| |/| | | | | | | |   
* | | | | | | | | |   1b5d8c02cf Merge branch 'fr/vimdiff-layout-fix' into next
|\ \ \ \ \ \ \ \ \ \  
| * | | | | | | | | | a62efbaabb vimdiff: make layout engine more robust against user vim settings
| |/ / / / / / / / /  
* | | | | | | | | |   f022414e31 Merge branch 'hx/lookup-commit-in-graph-fix' into next
|\ \ \ \ \ \ \ \ \ \  
| | |_|_|_|_|_|/ / /  
| |/| | | | | | | |   
| * | | | | | | | | a4d60a6221 commit-graph.c: no lazy fetch in lookup_commit_in_graph()
| |/ / / / / / / /  
* | | | | | | | |   90790c4455 Merge branch 'ab/submodule-cleanup' into next
|\ \ \ \ \ \ \ \ \  
| | |_|_|/ / / / /  
| |/| | | | | | |   
| * | | | | | | | 5b893f7d81 git-sh-setup.sh: remove "say" function, change last users
| * | | | | | | | 2eec463739 git-submodule.sh: use "$quiet", not "$GIT_QUIET"
| * | | | | | | | b788fc671b submodule--helper: eliminate internal "--update" option
| * | | | | | | | 8f12108c29 submodule--helper: understand --checkout, --merge and --rebase synonyms
| * | | | | | | | 36d45163b6 submodule--helper: report "submodule" as our name in some "-h" output
| * | | | | | | | 6e556c412e submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs"
| * | | | | | | | 0d68ee723e submodule update: remove "-v" option
| * | | | | | | | d9c7f69aaa submodule--helper: have --require-init imply --init
| * | | | | | | | da3aae9e84 git-submodule.sh: remove unused top-level "--branch" argument
| * | | | | | | | 757d092797 git-submodule.sh: make the "$cached" variable a boolean
| * | | | | | | | 960fad98e8 git-submodule.sh: remove unused $prefix variable
| * | | | | | | | 85775255f1 git-submodule.sh: remove unused sanitize_submodule_env()
* | | | | | | | |   d1ce5a9649 Merge branch 'sy/mv-out-of-cone' into next
|\ \ \ \ \ \ \ \ \  
| * | | | | | | | | b91a2b6594 mv: add check_dir_in_index() and solve general dir check issue
| * | | | | | | | | 24ea81d9ac mv: use flags mode for update_mode
| * | | | | | | | | 8a26a3915f mv: check if <destination> exists in index to handle overwriting
| * | | | | | | | | 6645b03ca5 mv: check if out-of-cone file exists in index with SKIP_WORKTREE bit
| * | | | | | | | | 7889755bae mv: decouple if/else-if checks using goto
| * | | | | | | | | 707fa2f76a mv: update sparsity after moving from out-of-cone to in-cone
| * | | | | | | | | 1143cc01b7 t1092: mv directory from out-of-cone to in-cone
| * | | | | | | | | 367844e5b7 t7002: add tests for moving out-of-cone file/directory
| | |_|/ / / / / /  
| |/| | | | | | |   
* | | | | | | | |   97316a96ed Merge branch 'hx/unpack-streaming' into next
|\ \ \ \ \ \ \ \ \  
| * | | | | | | | | aaf81223f4 unpack-objects: use stream_loose_object() to unpack large objects
| * | | | | | | | | 3c3ca0b0c1 core doc: modernize core.bigFileThreshold documentation
| * | | | | | | | | 2b6070ac4c object-file.c: add "stream_loose_object()" to handle large object
| * | | | | | | | | 21e7d88140 object-file.c: factor out deflate part of write_loose_object()
| * | | | | | | | | 97a9db6ffb object-file.c: refactor write_loose_object() to several steps
| * | | | | | | | | a1bf5ca29f unpack-objects: low memory footprint for get_data() in dry_run mode
* | | | | | | | | |   b4846d0daa Merge branch 'en/merge-tree' into next
|\ \ \ \ \ \ \ \ \ \  
| * | | | | | | | | | 7260e87248 git-merge-tree.txt: add a section on potentional usage mistakes
| * | | | | | | | | | 7976721d17 merge-tree: add a --allow-unrelated-histories flag
| * | | | | | | | | | 7c48b27822 merge-tree: allow `ls-files -u` style info to be NUL terminated
| * | | | | | | | | | de90581141 merge-ort: optionally produce machine-readable output
| * | | | | | | | | | cb2607759e merge-ort: store more specific conflict information
| * | | | | | | | | | 2715e8a931 merge-ort: make `path_messages` a strmap to a string_list
| * | | | | | | | | | 6debb7527b merge-ort: store messages in a list, not in a single strbuf
| * | | | | | | | | | b520bc6caa merge-tree: provide easy access to `ls-files -u` style info
| * | | | | | | | | | 7fa3338870 merge-tree: provide a list of which files have conflicts
| * | | | | | | | | | a4040cfa35 merge-ort: remove command-line-centric submodule message from merge-ort
| * | | | | | | | | | fae26ce79c merge-ort: provide a merge_get_conflicted_files() helper function
| * | | | | | | | | | a1a7811975 merge-tree: support including merge messages in output
| * | | | | | | | | | a34edae68a merge-ort: split out a separate display_update_messages() function
| * | | | | | | | | | 1f0c3a29da merge-tree: implement real merges
| * | | | | | | | | | 6ec755a0e1 merge-tree: add option parsing and initial shell for real merge function
| * | | | | | | | | | 55e48f6bf7 merge-tree: move logic for existing merge into new function
| * | | | | | | | | | 70176b7015 merge-tree: rename merge_trees() to trivial_merge_trees()
* | | | | | | | | | |   d340a03e89 Merge branch 'gg/worktree-from-the-above' into next
|\ \ \ \ \ \ \ \ \ \ \  
| * | | | | | | | | | | 299f88a3b3 dir: minor refactoring / clean-up
| * | | | | | | | | | | 992bb2eff9 dir: traverse into repository
| | |_|_|_|/ / / / / /  
| |/| | | | | | | | |   
* | | | | | | | | | |   a06ded0851 Merge branch 'jc/resolve-undo' into next
|\ \ \ \ \ \ \ \ \ \ \  
| |/ / / / / / / / / /  
|/| | | | | | | / / /   
| | |_|_|_|_|_|/ / /    
| |/| | | | | | | |     
| * | | | | | | | | 0c2d8a5b28 revision: mark blobs needed for resolve-undo as reachable
|/ / / / / / / / /  
| o / / / / / / / f770e9f396 Git 2.37-rc2
|  / / / / / / /  
| o / / / / / / ab336e8f1c Seventh batch
|  / / / / / /  
| | o / / / / 8168d5e9c2 Git 2.37-rc0
| |  / / / /  
| | o / / / 2668e3608e Sixth batch
| |  / / /  
| | o / / 30cc8d0f14 A regression fix for 2.37
| |  / /  
| o / / e4a4b31577 Git 2.37
|  / /  
o / / 4e2a4d1dd4 The second batch
 / /  
o / 54c8a7c379 revisions API: add a TODO for diff_free(&revs->diffopt)
 /  
o 1e59178e3f Sync with 'maint'