Re: [PATCH] fiter-branch: fix commit map init from state branch

From: Patrick Steinhardt <ps@pks.im>
Date: Sep 30, 2026, 14:40 UTC
Message-ID: <ar0fRtN8XMG-fyis@pks.im>
In-Reply-To: <20260801033127.10606-1-dev@grantmoyer.com>

On Fri, Jul 31, 2026 at 11:31:27PM -0400, Grant Moyer wrote:

Sorry, this slipped may radar. Thanks for bumping this thread, Michele.

Nit: pointed out by Michele: the subject has a typo in "fiter-branch".

Show 12 quoted lines

> 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:
> 
> > /usr/lib/git-core/git-filter-branch: line 305: ../map/: Is a directory
> 
> This change corrects the population of the commit map dir to match the
> "from_commit:to_commit" format.

So... does this mean that we don't have test coverage for this case at all?

It might make sense to also mention f6d855091e (filter-branch: stop depending on Perl, 2025-04-16) for context, as this is where the issue was introduced.

Show 15 quoted lines

> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index 24fa317aaa..9aa07be6e1 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -302,7 +302,9 @@ then
>  		do
>  			case "$line" in
>  			*:*)
> -				echo "${line%:*}" >../map/"${line#*:}";;
> +				from_commit=${line%:*}
> +				to_commit=${line#*:}
> +				echo "$to_commit" >../map/"$from_commit";;
>  			*)
>  				die "Unable to load state from $state_branch:filter.map";;
>  			esac

Okay. A simpler fix could've been the following diff:

- echo "${line%:*}" >../map/"${line#*:}";; + echo "${line#*:}" >../map/"${line%:*}";;

But I guess it doesn't hurt to have proper naming here.

Show 12 quoted lines

> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index 86011e7b1f..3934cc4a11 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -121,7 +121,7 @@ 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
>  '

So does this now detect the issue? If so, it feels somewhat roundabout.

Michele, it seems like you have already invested some time into reproducing the issue. Can this maybe be put into a proper test case that directly exercises your issues (that is, the swapped entries and such)?

Thanks!

Patrick