Commit Graph

78128 Commits

Author SHA1 Message Date
Junio C Hamano
b4e38c1acd Merge branch 'ly/changed-path-traversal-with-magic-pathspec'
Revision traversal limited with pathspec, like "git log dir/*",
used to ignore changed-paths Bloom filter when the pathspec
contained wildcards; now they take advantage of the filter when
they can.

* ly/changed-path-traversal-with-magic-pathspec:
  bloom: enable bloom filter with wildcard pathspec in revision traversal
2025-08-21 13:47:02 -07:00
Junio C Hamano
d1123cd810 Merge branch 'en/ort-rename-fixes'
Various bugs about rename handling in "ort" merge strategy have
been fixed.

* en/ort-rename-fixes:
  merge-ort: fix directory rename on top of source of other rename/delete
  merge-ort: fix incorrect file handling
  merge-ort: clarify the interning of strings in opt->priv->path
  t6423: fix missed staging of file in testcases 12i,12j,12k
  t6423: document two bugs with rename-to-self testcases
  merge-ort: drop unnecessary temporary in check_for_directory_rename()
  merge-ort: update comments to modern testfile location
2025-08-21 13:47:02 -07:00
Junio C Hamano
0cdf09b630 Merge branch 'ua/t1517-short-help-tests'
Test shuffling.

* ua/t1517-short-help-tests:
  t5304: move `prune -h` test from t1517
  t5200: move `update-server-info -h` test from t1517
  t/t1517: automate `git subcmd -h` tests outside a repository
2025-08-21 13:47:02 -07:00
Junio C Hamano
9e99f0e4ef Merge branch 'rj/t6137-cygwin-fix'
Test fix for breakage introduced in Git 2.50.

* rj/t6137-cygwin-fix:
  t6137-*.sh: fix test failure on cygwin
2025-08-21 13:47:01 -07:00
Junio C Hamano
a19f46970a Merge branch 'kh/doc-git-log-markup-fix'
Doc update.

* kh/doc-git-log-markup-fix:
  doc: git-log: fix description list
2025-08-21 13:47:01 -07:00
Junio C Hamano
3636c3a1d3 Merge branch 'dl/push-missing-object-error'
"git push" had a code path that led to BUG() but it should have
been a die(), as it is a response to a usual but invalid end-user
action to attempt pushing an object that does not exist.

* dl/push-missing-object-error:
  remote.c: convert if-else ladder to switch
  remote.c: remove BUG in show_push_unqualified_ref_name_error()
  t5516: remove surrounding empty lines in test bodies
2025-08-21 13:47:00 -07:00
Junio C Hamano
54fef16542 Merge branch 'jc/strbuf-split'
Arrays of strbuf is often a wrong data structure to use, and
strbuf_split*() family of functions that create them often have
better alternatives.

Update several code paths and replace strbuf_split*().

* jc/strbuf-split:
  trace2: do not use strbuf_split*()
  trace2: trim_trailing_newline followed by trim is a no-op
  sub-process: do not use strbuf_split*()
  environment: do not use strbuf_split*()
  config: do not use strbuf_split()
  notes: do not use strbuf_split*()
  merge-tree: do not use strbuf_split*()
  clean: do not use strbuf_split*() [part 2]
  clean: do not pass the whole structure when it is not necessary
  clean: do not use strbuf_split*() [part 1]
  clean: do not pass strbuf by value
  wt-status: avoid strbuf_split*()
2025-08-21 13:47:00 -07:00
Junio C Hamano
971ba42dd4 Merge branch 'jc/string-list-split'
string_list_split*() family of functions have been extended to
simplify common use cases.

* jc/string-list-split:
  string-list: split-then-remove-empty can be done while splitting
  string-list: optionally omit empty string pieces in string_list_split*()
  diff: simplify parsing of diff.colormovedws
  string-list: optionally trim string pieces split by string_list_split*()
  string-list: unify string_list_split* functions
  string-list: align string_list_split() with its _in_place() counterpart
  string-list: report programming error with BUG
2025-08-21 13:46:59 -07:00
Junio C Hamano
5a404a70c7 Merge branch 'rs/describe-with-prio-queue'
"git describe" has been optimized by using better data structure.

* rs/describe-with-prio-queue:
  describe: use prio_queue_replace()
  describe: use prio_queue
2025-08-21 13:46:59 -07:00
Junio C Hamano
9a85fa8406 Merge branch 'ps/remote-rename-fix'
"git remote rename origin upstream" failed to move origin/HEAD to
upstream/HEAD when origin/HEAD is unborn and performed other
renames extremely inefficiently, which has been corrected.

* ps/remote-rename-fix:
  builtin/remote: only iterate through refs that are to be renamed
  builtin/remote: rework how remote refs get renamed
  builtin/remote: determine whether refs need renaming early on
  builtin/remote: fix sign comparison warnings
  refs: simplify logic when migrating reflog entries
  refs: pass refname when invoking reflog entry callback
2025-08-21 13:46:58 -07:00
Junio C Hamano
c3c8b6910a Merge branch 'ps/reflog-migrate-fixes'
"git refs migrate" to migrate the reflog entries from a refs
backend to another had a handful of bugs squashed.

* ps/reflog-migrate-fixes:
  refs: fix invalid old object IDs when migrating reflogs
  refs: stop unsetting REF_HAVE_OLD for log-only updates
  refs/files: detect race when generating reflog entry for HEAD
  refs: fix identity for migrated reflogs
  ident: fix type of string length parameter
  builtin/reflog: implement subcommand to write new entries
  refs: export `ref_transaction_update_reflog()`
  builtin/reflog: improve grouping of subcommands
  Documentation/git-reflog: convert to use synopsis type
2025-08-21 13:46:57 -07:00
Junio C Hamano
1fe6955fd4 Merge branch 'js/rebase-i-allow-drop-on-a-merge'
During interactive rebase, using 'drop' on a merge commit lead to
an error, which was incorrect.

* js/rebase-i-allow-drop-on-a-merge:
  rebase -i: permit 'drop' of a merge commit
2025-08-21 13:46:57 -07:00
Junio C Hamano
c44beea485 Git 2.51
Signed-off-by: Junio C Hamano <gitster@pobox.com>
v2.51.0
2025-08-17 17:18:23 -07:00
Junio C Hamano
e5ab6b3e5a Merge tag 'l10n-2.51.0-2' of https://github.com/git-l10n/git-po
l10n-2.51.0-2

* tag 'l10n-2.51.0-2' of https://github.com/git-l10n/git-po:
  l10n: Update Catalan Translation for Git 2.51-rc2
  l10n: zh_CN: updated translation for 2.51
  l10n: uk: add 2.51 translation
  l10n: zh_TW: Git 2.51
  l10n: po-id for 2.51
  l10n: fr translation update for v2.51.0
  l10n: tr: Update Turkish translations for 2.51.0
  l10n: Updated translation for vi-2.51
  l10n: sv.po: Update Swedish translation
  l10n: bg.po: Updated Bulgarian translation (5856t)
2025-08-17 09:22:16 -07:00
Johannes Schindelin
ba8bef458c cmake: accommodate for UNIT_TEST_SOURCES
As part of 9bbc981c6f (t/unit-tests: finalize migration of
reftable-related tests, 2025-07-24), the explicit list of
`UNIT_TEST_PROGRAMS` was turned into a wildcard pattern-derived list.

Let's do the same in the CMake definition.

This fixes build errors with symptoms like this:

  CMake Error at CMakeLists.txt:132 (string):
    string sub-command REPLACE requires at least four arguments.
  Call Stack (most recent call first):
    CMakeLists.txt:1037 (parse_makefile_for_scripts)

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-17 09:12:53 -07:00
Mikel Forcada
79ee0dce2a l10n: Update Catalan Translation for Git 2.51-rc2
Edit: We are continuing to follow the existing PO file convention, which
includes filenames but strips out line numbers from the file-location
comments. This standard was set by our former lead, Jordi Mas, and we
are maintaining it for project-wide consistency.

Signed-off-by: Mikel Forcada <mikel.forcada@gmail.com>
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
2025-08-17 09:25:36 -04:00
Jiang Xin
f84a7b496d Merge branch 'jx/zh_CN-2.51' of github.com:jiangxin/git
* 'jx/zh_CN-2.51' of github.com:jiangxin/git:
  l10n: zh_CN: updated translation for 2.51
2025-08-17 09:03:47 -04:00
Teng Long
2000abefba l10n: zh_CN: updated translation for 2.51
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Reviewed-by: Fangyi Zhou <me@fangyi.io>
Reviewed-by: 依云 <lilydjwg@gmail.com>
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
2025-08-17 09:03:47 -04:00
Jiang Xin
b11d0d6f77 Merge branch '2.51-uk-update' of github.com:arkid15r/git-ukrainian-l10n
* '2.51-uk-update' of github.com:arkid15r/git-ukrainian-l10n:
  l10n: uk: add 2.51 translation
2025-08-17 09:03:46 -04:00
Arkadii Yakovets
63fbf0815b l10n: uk: add 2.51 translation
Co-authored-by: Kate Golovanova <kate@kgthreads.com>
Signed-off-by: Arkadii Yakovets <ark@cho.red>
Signed-off-by: Kate Golovanova <kate@kgthreads.com>
2025-08-16 08:40:52 -07:00
Jiang Xin
a7e6b5fe95 Merge branch 'fr_v2.51.0' of github.com:jnavila/git
* 'fr_v2.51.0' of github.com:jnavila/git:
  l10n: fr translation update for v2.51.0
2025-08-16 01:52:32 -04:00
Jiang Xin
c66900d7a8 Merge branch 'po-id' of github.com:bagasme/git-po
* 'po-id' of github.com:bagasme/git-po:
  l10n: po-id for 2.51
2025-08-16 01:51:25 -04:00
Jiang Xin
b40eaf15d1 Merge branch 'tr-l10n' of github.com:bitigchi/git-po
* 'tr-l10n' of github.com:bitigchi/git-po:
  l10n: tr: Update Turkish translations for 2.51.0
2025-08-16 01:50:53 -04:00
Jiang Xin
987d205097 Merge branch 'l10n/zh-TW/2025-08-08' of github.com:l10n-tw/git-po
* 'l10n/zh-TW/2025-08-08' of github.com:l10n-tw/git-po:
  l10n: zh_TW: Git 2.51
2025-08-16 01:50:04 -04:00
Jiang Xin
6a5a95df8e Merge branch 'master' of github.com:alshopov/git-po
* 'master' of github.com:alshopov/git-po:
  l10n: bg.po: Updated Bulgarian translation (5856t)
2025-08-16 01:47:43 -04:00
Jiang Xin
0eb21c229d Merge branch 'master' of github.com:nafmo/git-l10n-sv
* 'master' of github.com:nafmo/git-l10n-sv:
  l10n: sv.po: Update Swedish translation
2025-08-16 01:47:04 -04:00
Jiang Xin
7ad97958d8 Merge branch 'vi-2.51' of github.com:Nekosha/git-po
* 'vi-2.51' of github.com:Nekosha/git-po:
  l10n: Updated translation for vi-2.51
2025-08-16 01:43:07 -04:00
Yi-Jyun Pan
5590ee9132 l10n: zh_TW: Git 2.51
Co-authored-by: Lumynous <lumynou5.tw@gmail.com>
Co-authored-by: hms5232 <hms5232@hhming.moe>
Signed-off-by: Yi-Jyun Pan <pan93412@gmail.com>
2025-08-16 12:14:48 +08:00
Bagas Sanjaya
d65d66bb32 l10n: po-id for 2.51
Update following components:

  * add-interactive.c
  * builtin/add.c
  * builtin/config.c
  * builtin/fetch.c
  * builtin/for-each-ref.c
  * builtin/gc.c
  * builtin/merge.c
  * builtin/pack-objects.c
  * builtin/remote.c
  * builtin/repack.c
  * builtin/stash.c
  * builtin/submodule--helper.c
  * diff-no-index.c
  * git-send-email.perl
  * imap-send.c
  * parse-options.c
  * refs.c
  * t/helper/test-path-walk.c
  * usage.c

Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
2025-08-15 17:34:27 +07:00
Jean-Noël Avila
a9d72c5aec l10n: fr translation update for v2.51.0
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
2025-08-14 19:13:18 +02:00
Emir SARI
297f5bb8dc l10n: tr: Update Turkish translations for 2.51.0
Signed-off-by: Emir SARI <emir_sari@icloud.com>
2025-08-14 16:58:38 +03:00
Vũ Tiến Hưng
f7ecf8acea l10n: Updated translation for vi-2.51
Signed-off-by: Vũ Tiến Hưng <newcomerminecraft@gmail.com>
2025-08-14 16:28:09 +07:00
Peter Krefting
98ba88788c l10n: sv.po: Update Swedish translation
Also fix typo reported by Tuomas Ahola <taahol@utu.fi>.

Signed-off-by: Peter Krefting <peter@softwolves.pp.se>
2025-08-14 09:54:03 +01:00
Alexander Shopov
e2c8f63c13 l10n: bg.po: Updated Bulgarian translation (5856t)
Signed-off-by: Alexander Shopov <ash@kambanaria.org>
2025-08-13 22:07:28 +02:00
Junio C Hamano
724518f388 Git 2.51-rc2
Signed-off-by: Junio C Hamano <gitster@pobox.com>
v2.51.0-rc2
2025-08-13 07:57:49 -07:00
Junio C Hamano
8d2709d075 A few hotfixes before -rc2
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-11 21:30:16 -07:00
Junio C Hamano
22dd6abc32 Merge branch 'rs/merge-compact-summary'
Hotfix.

* rs/merge-compact-summary:
  merge: don't document non-existing --compact-summary argument
2025-08-11 21:30:16 -07:00
Junio C Hamano
10fa89aadc Merge branch 'rs/for-each-ref-start-after-marker-fix'
Hotfix.

* rs/for-each-ref-start-after-marker-fix:
  for-each-ref: call --start-after argument "marker"
2025-08-11 21:30:15 -07:00
Lidong Yan
6d192462eb bloom: enable bloom filter with wildcard pathspec in revision traversal
When traversing commits, a pathspec item can be used to limit the
traversal to commits that modify the specified paths. And the
commit-graph includes a Bloom filter to exclude commits that definitely
did not modify a given pathspec item. During commit traversal, the
Bloom filter can significantly improve performance. However, it is
disabled if the specified pathspec item contains wildcard characters
or magic signatures.

For performance reason, enable Bloom filter even if a pathspec item
contains wildcard characters by filtering only the non-wildcard part of
the pathspec item.

The function of pathspec magic signature is generally to narrow down
the path specified by the pathspecs. So, enable Bloom filter when
the magic signature is "top", "glob", "attr", "--depth" or "literal".
"exclude" is used to select paths other than the specified path, rather
than serving as a filtering function, so it cannot be used together with
the Bloom filter. Since Bloom filter is not case insensitive even in
case insensitive system (e.g. MacOS), it cannot be used together with
"icase" magic.

With this optimization, we get some improvements for pathspecs with
wildcards or magic signatures. First, in the Git repository we see these
modest results:

git log -100 -- "t/*"

Benchmark 1: new
  Time (mean ± σ):      20.4 ms ±   0.6 ms
  Range (min … max):    19.3 ms …  24.4 ms

Benchmark 2: old
  Time (mean ± σ):      23.4 ms ±   0.5 ms
  Range (min … max):    22.5 ms …  24.7 ms

git log -100 -- ":(top)t"

Benchmark 1: new
  Time (mean ± σ):      16.2 ms ±   0.4 ms
  Range (min … max):    15.3 ms …  17.2 ms

Benchmark 2: old
  Time (mean ± σ):      18.6 ms ±   0.5 ms
  Range (min … max):    17.6 ms …  20.4 ms

But in a larger repo, such as the LLVM project repo below, we get even
better results:

git log -100 -- "libc/*"

Benchmark 1: new
  Time (mean ± σ):      16.0 ms ±   0.6 ms
  Range (min … max):    14.7 ms …  17.8 ms

Benchmark 2: old
  Time (mean ± σ):      26.7 ms ±   0.5 ms
  Range (min … max):    25.4 ms …  27.8 ms

git log -100 -- ":(top)libc"

Benchmark 1: new
  Time (mean ± σ):      15.6 ms ±   0.6 ms
  Range (min … max):    14.4 ms …  17.7 ms

Benchmark 2: old
  Time (mean ± σ):      19.6 ms ±   0.5 ms
  Range (min … max):    18.6 ms …  20.6 ms

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Lidong Yan <yldhome2d2@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-11 08:56:39 -07:00
René Scharfe
ad459fd44c merge: don't document non-existing --compact-summary argument
3a54f5bd5d (merge/pull: add the "--compact-summary" option, 2025-06-12)
added the option --compact-summary to both merge and pull.  It takes no
no argument, but for merge it got an argument help string.  Remove it,
since it is unnecessary.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-09 17:11:19 -07:00
René Scharfe
51d9ed581f for-each-ref: call --start-after argument "marker"
dabecb9db2 (for-each-ref: introduce a '--start-after' option,
2025-07-15) added the option --start-after and referred to its argument
as "marker" in documentation and usage string, but not in the option's
short help.  Use "marker" there as well for consistency and brevity.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-09 17:10:39 -07:00
Ramsay Jones
cf7efa4f33 t6137-*.sh: fix test failure on cygwin
Commit 6fd1106aa4 ("t3700: Skip a test with backslashes in pathspec",
2009-03-13) introduced the BSLASHPSPEC prerequisite. This prerequisite
allows tests to check for systems that can use backslashes in pathspecs
(e.g. to escape glob special characters). On windows (and cygwin), this
does not work because backslashes are used as directory separators, and
git eagerly converts them to forward slashes.

This test file uses the FUNNYNAMES prerequisite to skip this test file
on windows, despite not really being appropriate for this test, which
does not hold on cygwin. The FUNNYNAMES prerequisite is set when the
system can create files with embedded quotes ("), tabs or newlines in
the name. Since cygwin can satisfy FUNNYNAMES, but not BSLASHPSPEC, this
leads to test failures on cygwin.

In order to skip these tests on cygwin, replace the FUNNYNAMES prerequisite
with BSLASHPSPEC, so that this test file is skipped on both windows and
cygwin. While here, fix a few test titles as well.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-08 13:30:00 -07:00
Kristoffer Haugsbakk
823d537fa7 doc: git-log: fix description list
b27be108c8 (doc: git-log: convert log config to new doc format,
2025-07-07) intended to convert a paragraph describing the different
options for `log.decorate` into a description list.  But the literal
block syntax was used by mistake.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-08 09:14:46 -07:00
Denton Liu
dfbfc2221b remote.c: convert if-else ladder to switch
For better readability, convert the if-else ladder into a switch
statement.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-08 09:01:23 -07:00
Denton Liu
b33c590e4f remote.c: remove BUG in show_push_unqualified_ref_name_error()
When "git push <remote> <src>:<dst>" does not spell out the
destination side of the ref fully, and when <src> is not given
as a reference but an object name, the code tries to give advice
messages based on the type of that object.

The type is determined by calling odb_read_object_info() and
signalled by its return value.  The code however reported a
programming error with BUG() when this function said that there
is no such object, which happens when the object name is given
as a full hexadecimal (if the object name is given as a partial
hexadecimal or an non-existing ref, the function would have died
without returning, so this BUG() wouldn't have triggered).  This
is wrong.  It is an ordinary end-user mistake to give an object
name that does not exist and treated as such.

An example of the error message produced is as follows:

	error: The destination you provided is not a full refname (i.e.,
	starting with "refs/"). We tried to guess what you meant by:

	- Looking for a ref that matches 'branch' on the remote side.
	- Checking if the <src> being pushed ('0000000000000000000000000000000000000001')
	  is a ref in "refs/{heads,tags}/". If so we add a corresponding
	  refs/{heads,tags}/ prefix on the remote side.

	Neither worked, so we gave up. You must fully qualify the ref.
	BUG: remote.c:1221: '0000000000000000000000000000000000000001' should be commit/tag/tree/blob, is '-1'
	fatal: the remote end hung up unexpectedly
	Aborted (core dumped)

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-08 08:54:49 -07:00
Denton Liu
31e5d037df t5516: remove surrounding empty lines in test bodies
This style with the empty lines in test bodies was from when the test
suite was being developed. Remove the empty lines to match the modern
test style.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-08 08:54:49 -07:00
Usman Akinyemi
529a60a885 t5304: move prune -h test from t1517
t1517 is now focused on testing subcommands outside a repository.
Move the in-repo `-h` test for `prune` to t5304, which covers
this command.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-08 07:48:27 -07:00
Usman Akinyemi
18aae638cb t5200: move update-server-info -h test from t1517
t1517 is now focused on testing subcommands outside a repository.
Move the in-repo `-h` test for `update-server-info` to t5200,
which covers this command.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-08 07:48:27 -07:00
Usman Akinyemi
39fc408562 t/t1517: automate git subcmd -h tests outside a repository
Replace manual `-h` tests with a loop over all subcommands using
`git --list-cmds=main`. This ensures consistent coverage of `-h`
behavior outside a repo and future-proofs the test by covering
new commands automatically.

Known exceptions are skipped or marked as expected failures.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-08 07:48:27 -07:00
Elijah Newren
f6ecb603ff merge-ort: fix directory rename on top of source of other rename/delete
At GitHub, we've got a real-world repository that has been triggering
failures of the form:

    git: merge-ort.c:3007: process_renames: Assertion `newinfo && !newinfo->merged.clean' failed.

which comes from the line:

    VERIFY_CI(newinfo);

Unfortunately, this one has been quite complex to unravel, and is a
bit complex to explain.  So, I'm going to carefully try to explain each
relevant piece needed to understand the fix, then carefully build up
from a simple testcase to some of the relevant testcases.

== New special case we need to consider ==

Rename pairs in the diffcore machinery connect the source path of a
rename with the destination path of a rename.  Since we have rename
pairs to consider on both sides of history since the merge base,
merging has to consider a few special cases of possible overlap:

  A) two rename pairs having the same target path
  B) two rename pairs having the same source path
  C) the source path of one rename pair being the target path of a
     different rename pair

Some of these came up often enough that we gave them names:
  A) a rename/rename(2to1) conflict (looks similar to an add/add conflict)
  B) a rename/rename(1to2) conflict, which represents the same path being
     renamed differently on the two sides of history
  C) not yet named

merge-ort is well-prepared to handle cases (A) and (B), as was
merge-recursive (which was merge-ort's predecessor).  Case (C) was
briefly considered during the years of merge-recursive maintenance,
but the full extent of support it got was a few FIXME/TODO comments
littered around the code highlighting some of the places that would
probably need to be fixed to support it.  When I wrote merge-ort I
ignored case (C) entirely, since I believed that case (C) was only
possible if we were to support break detection during merges.  Not
only had break detection never been supported by any merge algorithm,
I thought break detection wasn't worth the effort to support in a
merge algorithm.  However, it turns out that case (C) can be triggered
without break detection, if there's enough moving pieces.

Before I dive into how to trigger case (C) with directory renames plus
other renames, it might be helpful to use a simpler example with break
detection first.  And before we get to that it may help to explain
some more basics of handling renames in the merge algorithm.  So, let
me first backup and provide a quick refresher on each of

  * handling renames
  * what break detection would mean, if supported in merging
  * handling directory renames

From there, I'll build up from a basic directory rename detection case
to one that triggers a failure currently.

== Handling renames ==

In the merge machinery when we have a rename of a path A -> B,
processing that rename needs to remove path A, and make sure that path B
has the relevant information.  Note that if the content was also
modified on both sides, this may mean that we have 3 different stages
that need to be stored at path B instead of having some stored at path
A.

Having all stages stored at path B makes it much easier for users to
investigate and resolve the content conflict associated with a renamed
path.  For example:
  * "git status" doesn't have to figure out how to list paths A & B and
    attempt to connect them for users; it can just list path B.
  * Users can use "git ls-files -u B" (instead of trying to find the
    previous name of the file so they can list both, i.e. "git ls-files
    -u A B")
  * Users can resolve via "git add B" (without needing to "git rm A")

== What break detection would mean ==

If break detection were supported, we might have cases where A -> B
*and* C -> A, meaning that both rename pairs might believe they need to
update A.  In particular, the processing of A -> B would need to be
careful to not clear out all stages of A and mark it resolved, while
both renames would need to figure out which stages of A belong with A
and which belong with B, so that both paths have the right stages
associated with them.

merge-ort (like merge-recursive before it) makes no attempt to handle
break detection; it runs with break detection turned off.  It would
need to be retrofitted to handle such cases.

== Directory rename detection ==

If one side of history renames directory D/ -> E/, and the other side of
history adds new files to D/, then directory rename detection notices
and suggests moving those new files to E/.  A similar thing is done for
paths renamed into D/, causing them to be transitively renamed into E/.

The default in the merge machinery is to report a conflict whenever a
directory rename might modify the location of a path, so that users can
decide whether they wanted the original path or the
directory-rename-induced location.  However, that means the default
codepath still runs through all the directory rename detection logic, it
just supplements it with providing conflict notices when it is done.

== Building up increasingly complex testcases ==

I'll start with a really simple directory rename example, and then
slowly add twists that explain new pieces until we get to the
problematic cases:

=== Testcase 1 ===

Let's start with a concrete example, where particular files/directories of
interest that exist or are changed on each side are called out:

  Original:   <nothing of note>
  our side:   rename B/file -> C/file
  their side: rename C/     -> A/

For this case, we'd expect to see the original B/file appear not at
C/file but at A/file.

(We would also expect a conflict notice that the user will want to
choose between C/file and A/file, but I'm going to ignore conflict
notices from here on by assuming merge.directoryRenames is set to
`true` rather than `conflict`; the only difference that assumption
makes is whether that makes the merge be considered to be conflicted
and whether it prints a conflict notice; what is written to the index
or working directory is unchanged.)

=== Testcase 2 ===

Modify testcase 1 by having A/file exist from the start:

  Original:   A/file exists
  our side:   rename B/file -> C/file
  their side: rename C/     -> A/

In such a case, to avoid user confusion at what looks kind of like an
add/add conflict (even though the original path at A/file was not added
by either side of the merge), we turn off directory rename detection for
this path and print a "in the way" warning to the user:
    CONFLICT (implicit dir rename): Existing file/dir ... in the way ...
The testcases in section 5 of t6423 explore these in more detail.

=== Testcase 3 ===

Let's modify testcase 1 in a slightly different way: have A/file be
added by their side rather than it already existing.

  Original:   <nothing of note>
  our side:   rename B/file -> C/file
  their side: rename C/     -> A/
              add A/file

In this case, the directory rename detection basically transforms our
side's original B/file -> C/file into a B/file -> A/file, and so we
get a rename/add conflict, with one version of A/file coming from the
renamed file, and another coming from the new A/file, each stored as
stages 2 and 3 in conflicts.  This kind of add/add conflict is perhaps
slightly more complex than a regular add/add conflict, but with the
printed messages it makes sense where it came from and we have
different stages of the file to work with to resolve the conflict.

=== Testcase 4 ===

Let's do something similar to testcase 3, but have the opposite side of
history add A/file:

  Original:   <nothing of note>
  our side:   rename B/file -> C/file
              add    A/file
  their side: rename C/     -> A/

Now if we allow directory rename detection to modify C/file to A/file,
then we also get a rename/add conflict, but in this case we'd need both
higher order stages being recorded on side 2, which makes no sense.  The
index can't store multiple stage 2 entries, and even if we could, it
would probably be confusing for users to work with.  So, similar to what
we do when there was an A/file in the original version, we simply turn
off directory rename detection for cases like this and provide the "in
the way" CONFLICT notice to the user.

=== Testcase 5 ===

We're slowly getting closer.  Let's mix it up by having A/file exist at
the beginning but not exist on their side:

  original:   A/file exists
  our side:   rename B/file -> C/file
  their side: rename C/     -> A/
              rename A/file -> D/file

For this case, you could say that since A/file -> D/file, it's no longer
in the way of C/file being moved by directory rename detection to
A/file.  But that would give us a case where A/file is both the source
and the target of a rename, similar to break detection, which the code
isn't currently equipped to handle.

This is not yet the case that causes current failures; to the current
code, this kind of looks like testcase 4 in that A/file is in the way
on our side (since A/file was in the original and was umodified by our
side).  So, it results in a "in the way" notification with directory
rename detection being turned off for A/file so that B/file ends up at
C/file.

Perhaps the resolution could be improved in the future, but our "in
the way" checks prevented such problems by noticing that A/file exists
on our side and thus turns off directory rename detection from
affecting C/file's location.  So, while the merge result could be
perhaps improved, the fact that this is currently handled by giving
the user an "in the way" message gives the user a chance to resolve
and prevents the code from tripping itself up.

=== Testcase 6 ===

Let's modify testcase 5 a bit more, to also delete A/file on our side:

  original:   A/file exists
  our side:   rename B/file -> C/file
              delete A/file
  their side: rename C/     -> A/
              rename A/file -> D/file

Now the "in the way" logic doesn't detect that there's an A/file in
the way (neither side has an A/file anymore), so it's fine to
transitively rename C/file further to A/file...except that we end up
with A/file being both the source of one rename, and the target of a
different rename.  Each rename pair tries to handle the resolution of
the source and target paths of its own rename.  But when we go to
process the second rename pair in process_renames(), we do not expect
either the source or the destination to be marked as handled already;
so, when we hit the sanity checks that these are not handled:

    VERIFY_CI(oldinfo);
    VERIFY_CI(newinfo);

then one of these is going to throw an assertion failure since the
previous rename pair already marked both of its paths as handled.
This will give us an error of the form:

    git: merge-ort.c:3007: process_renames: Assertion `newinfo && !newinfo->merged.clean' failed.

This is the failure we're currently triggering, and it fundamentally
depends on:
  * a path existing in the original
  * that original path being removed or renamed on *both* sides
  * some kind of directory rename moving some *other* path into that
    original path

This was added as testcase 12q in t6423.

=== Testcase 7 ===

Bonus bug found while investigating!

Let's go back to the comparison between testcases 2 & 3, and set up a
file present on their side that we need to consider:

  Original:   A/file exists
  our side:   rename B/file -> C/file
              rename A/file -> D/file
  their side: rename C/     -> A/

Here, there is no A/file in the way on our side like testcase 4.
There is an A/file present on their side like testcase 3, which was an
add/add conflict, but that's associated with the file be renamed to
D/file.  So, that really shouldn't be an add/add conflict because we
instead want all modes of the original A/file to be transported to
D/file.

Unfortunately, the current code kind of treats it like an add/add
conflict instead...but even worse.  There is also a valid mode for
A/file in the original, which normally goes to stage 1.  However, an
add/add conflict should be represented in the index with no mode at
stage 1 (for the original side), only modes at stages 2 and 3 (for our
and their side), so for an add/add we'd expect that mode for A/file in
the original version to be cleared out (or be transported to D/file).

Unfortunately, the code currently leaves not only the stage 3 entry
for A/file intact, it also leaves the stage 1 entry for A/file.  This
results in `git ls-files -u A/file` output of the form:

    100644 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 1	A/file
    100644 0cfbf08886fca9a91cb753ec8734c84fcbe52c9f 2	A/file
    100644 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 3	A/file

This would likely cause users to believe this isn't an add/add
conflict; rather, this would lead them to believe that A/file was only
modified on our side and that therefore it should not have been a
conflict in the first place.  And while resolving the conflict in
favor of our side is the correct resolution (because stages 1 and 3
should have been cleared out in the first place), this is certainly
likely to cause confusion for anyone attempting to investigate why
this path was marked as conflicted.

This was added as testcase 12p in t6423.

== Attempted solutions that I discarded ==

1) For each side of history, create a strset of the sources of each
   rename on the other side of history.  Then when using directory
   renames to modify existing renames, verify that we aren't renaming
   to a source of another rename.

   Unfortunately, the "relevant renames" optimization in merge-ort
   means we often don't detect renames -- we just see a delete and an
   add -- which is easy to forget and makes debugging testcases harder,
   but it also turns out that this solution in insufficient to solve
   the related problems in the area (more on that below).

2) Modify the code to be aware of the possibility of renaming to
   the source of another side's rename, and make all the conflict
   resolution logic for each case (including existing
   rename/rename(2to1) and rename/rename(1to2) cases) handle the
   additional complexity.  It turns out there was much more code to
   audit than I wanted, for a really niche case.  I didn't like how
   many changes were needed, and aborted.

== Solution ==

We do not want the stages of unrelated files appearing at the same path
in the index except when dealing with an add/add conflict.  While we
previously handled this for stages 2 & 3, we also need to worry about
stage 1.  So check for a stage 1 index entry being in the way of a
directory rename.

However, if we can detect that the stage 1 index entry is actually from
a related file due to a directory-rename-causes-rename-to-self
situation, then we can allow the stage 1 entry to remain.

From this wording, you may note that it's not just rename cases that
are a problem; bugs could be triggered with directory renames vs simple
adds.  That leads us to...

== Testcases 8+ ==

Another bonus bug, found via understanding our final solutions (and the
failure of our first attempted solution)!

Let's tweak testcase 7 a bit:

  Original:   A/file exists
  our side:   delete A/file
              add -> C/file
  their side: delete A/file
              rename C/     -> A/

Here, there doesn't seem to be a big problem.  Sure C/file gets modified
via the directory rename of C/ -> A/ so that it becomes A/file, but
there's no file in the way, right?  Actually, here we have a problem
that the stage 1 entry of A/file would be combined with the stage 2
entry of C/file, and make it look like a modify/delete conflict.
Perhaps there is some extra checking that could be added to the code to
make it attempt to clear out the stage 1 entry of A/file, but the
various rename-to-self-via-directory-rename testcases make that a bit
more difficult.  For now, it's easier to just treat this as a
path-in-the-way situation and not allow the directory rename to modify
C/file.

That sounds all well and good, but it does have an interesting side
effect.  Due to the "relevant renames" optimizations in merge-ort (i.e.
only detect the renames you need), 100% renames whose files weren't
modified on the other side often go undetected.  This means that if we
modify this testcase slightly to:

  Original:   A/file exists
  our side:   A/file -> C/file
  their side: rename C/ -> A/

Then although this looks like where the directory rename just moves
C/file back to A/file and there's no problem, we may not detect the
A/file -> C/file rename.  Instead it will look like a deletion of A/file
and an addition of C/file.  The directory rename then appears to be
moving C/file to A/file, which is on top of an "unrelated" file (or at
least a file it doesn't know is related).  So, we will report
path-in-the-way conflicts now in cases where we didn't before.  That's
better than silently and accidentally combining stages of unrelated
files and making them look like a modify/delete; users can investigate
the reported conflict and simply resolve it.

This means we tweak the expected solution for testcases 12i, 12j, and
12k.  (Those three tests are basically the same test repeated three
times, but I was worried when I added those that subtle differences in
parent/child, sibling/sibling, and toplevel directories might mess up
how rename-to-self testcases actually get handled.)

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-08-07 13:24:00 -07:00