Re: [PATCH v2] filter-branch: fix commit map init from state branch
From: Patrick Steinhardt <hidden>
Date: 2026-10-01 05:36:16
On Wed, Sep 30, 2026 at 09:23:47PM -0400, Grant Moyer wrote:
The commit map dir is populated from the state branch assuming a "to_commit:from_commit" format, but the state branch is written with a "from_commit:to_commit" format, resulting in an inverted mapping when the map is populated from the state branch. This is especially evident when --prune-empty is used and creates commits which map to nothing; when the map dir is populated from this state on subsequent runs, git-filter-branch outputs many errors while trying to create files with empty names, like:
This still doesn't mention the commit that introduced the regression.
quoted
/usr/lib/git-core/git-filter-branch: line 305: ../map/: Is a directoryThis change corrects the population of the commit map dir to match the "from_commit:to_commit" format and adds/updates tests to check that the state branch is written correctly.
Nit: we typically write commit messages in imperative mood, as if you were instructing the code to change.
Signed-off-by: Grant Moyer <redacted> Tested-by: Michele Locati <redacted> Co-authored-by: Michele Locati [off-list ref]
Your Signed-off-by should always go last because you're signing off on everything that comes before it. How about this message: The "--state-branch" option asks git-filter-branch(1) to write a mapping from old to new objects into the branch. This map is a simple blob stored in "$state_branch:filter.map" with the format "$to_commit:$from_commit". In f6d855091e (filter-branch: stop depending on Perl, 2025-04-16), we have refactored git-filter-branch(1) to no longer require Perl. But as part of that change we accidentally started to interpret the above format in reverse when populating the map directory. This of course breaks incremental rewrites that use the commit map multiple times. But we don't seem to have any tests for this feature, and as a consequence we didn't notice the regression. Fix this bug by interpreting the mappings in the correct order again. Tested-by: Michele Locati [off-list ref] Co-authored-by: Michele Locati [off-list ref] Signed-off-by: Grant Moyer [off-list ref]
quoted hunk ↗ jump to hunk
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index 86011e7b1f..cf225b0f0f 100755 --- a/t/t7003-filter-branch.sh +++ b/t/t7003-filter-branch.sh@@ -121,10 +121,32 @@ W=$(git rev-parse HEAD) test_expect_success 'using --state-branch to skip already rewritten commits' ' test_when_finished git reset --hard $V && git reset --hard $V && - git filter-branch --state-branch state -f --tree-filter "touch file || :" HEAD && + git filter-branch --state-branch state -f --tree-filter "exit 1" HEAD && test_cmp_rev $W HEAD ' +test_expect_success '--state-branch incremental rewrite uses the rewritten parents' '
We could add `test_when_finished rm -rf incremental` here.
quoted hunk ↗ jump to hunk
+ git init incremental && + ( + cd incremental && + mkdir sub && + test_commit first sub/file && + test_commit outside root-file && + git filter-branch --state-branch refs/state \ + --prune-empty --subdirectory-filter sub -- HEAD && + rewritten_first=$(git rev-parse HEAD) && + git reset --hard outside && + test_commit second sub/file && + git filter-branch -f --state-branch refs/state \ + --prune-empty --subdirectory-filter sub -- outside..HEAD && + test_cmp_rev $rewritten_first HEAD^ && + git show refs/state:filter.map >map && + echo "$(git rev-parse second):$(git rev-parse HEAD)" >expect && + grep "^$(git rev-parse second):" map >actual && + test_cmp expect actual + ) +'
Thanks! Patrick