{"thread":{"id":"37030","subject":"[PATCH] Fix filter-branch to eliminate duplicate mapped parents","startedAt":"2014-06-30T21:20:27Z","lastAt":"2014-07-01T15:03:46Z","messageCount":3,"participants":["Charles Bailey","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"245215","messageId":"1404163227-30962-1-git-send-email-charles@hashpling.org","threadId":"37030","inReplyTo":null,"subject":"[PATCH] Fix filter-branch to eliminate duplicate mapped parents","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2014-06-30T21:20:27Z","receivedAt":"2014-06-30T21:20:27Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"From: Charles Bailey <cbailey32@bloomberg.net>\n\nWhen multiple parents of a merge commit get mapped to the same commit,\nfilter-branch used to pass all instances of the parent commit to the\nparent and commit filters and to \"git commit-tree\" or\n\"git_commit_non_empty_tree\".\n\nThis can often happen when extracting a small project from a large\nrepository; merges can join history with no commits on any branch which\naffect the paths being retained. Once the intermediate commits have been\nfiltered out, all the immediate parents of the merge commit can end up\nbeing mapped to the same commit - either the original merge-base or an\nancestor of it.\n\n\"git commit-tree\" would display an error but write the commit with the\nnormalized parents in any case. \"git_commit_non_empty_tree\" would fail\nto notice that the commit being made was in fact a non-merge commit and\nwould retain it even if a further pass with --prune-empty would discard\nthe commit as empty.\n\nThis change ensure that duplicate parents are pruned before the parent\nfilter and ensures that --prune-empty is idempotent, removing all\nempty non-merge commits in a singe pass.\n\nSigned-off-by: Charles Bailey <cbailey32@bloomberg.net>\n---\n\nI worked on this after discovering that --prune-empty often left some\napparently empty commits that I was wasn't expecting it to leave and\nthat running filter-branch --prune-empty in a loop would often do many\npasses where it was still pruning empty former merge commits.\n\nThe test is a simple example of such a case. A non-ff merge of a commit\nthat only changes a file that is to be pruned gets squashed into an\nempty non-merge commit that should be pruned.\n\n git-filter-branch.sh     |  8 +++++++-\n t/t7003-filter-branch.sh | 11 +++++++++++\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 86d6994..c5b82a8 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -332,7 +332,13 @@ while read commit parents; do\n \tparentstr=\n \tfor parent in $parents; do\n \t\tfor reparent in $(map \"$parent\"); do\n-\t\t\tparentstr=\"$parentstr -p $reparent\"\n+\t\t\tcase \"$parentstr\" in\n+\t\t\t\t*\" -p $reparent\"*)\n+\t\t\t\t\t;;\n+\t\t\t\t*)\n+\t\t\t\t\tparentstr=\"$parentstr -p $reparent\"\n+\t\t\t\t\t;;\n+\t\t\tesac\n \t\tdone\n \tdone\n \tif [ \"$filter_parent\" ]; then\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 9496736..3741f51 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -308,6 +308,17 @@ test_expect_success 'Prune empty commits' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'Prune empty collapsed merges' '\n+\ttest_config merge.ff false &&\n+\tgit rev-list HEAD > expect &&\n+\ttest_commit to_remove_2 &&\n+\tgit reset --hard HEAD^ &&\n+\ttest_merge non-ff to_remove_2 &&\n+\tgit filter-branch -f --index-filter \"git update-index --remove to_remove_2.t\" --prune-empty HEAD &&\n+\tgit rev-list HEAD > actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success '--remap-to-ancestor with filename filters' '\n \tgit checkout master &&\n \tgit reset --hard A &&\n-- \n1.9.0\n"},{"id":"245229","messageId":"20140701121110.GA6656@hashpling.org","threadId":"37030","inReplyTo":"1404163227-30962-1-git-send-email-charles@hashpling.org","subject":"Re: [PATCH] Fix filter-branch to eliminate duplicate mapped parents","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2014-07-01T12:11:10Z","receivedAt":"2014-07-01T12:11:10Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Mon, Jun 30, 2014 at 10:20:27PM +0100, Charles Bailey wrote:\n> From: Charles Bailey <cbailey32@bloomberg.net>\n> \n> This change ensure that duplicate parents are pruned before the parent\n> filter and ensures that --prune-empty is idempotent, removing all\n> empty non-merge commits in a singe pass.\n\ns/change ensure/change ensures/\n"},{"id":"245234","messageId":"xmqqmwctxgrx.fsf@gitster.dls.corp.google.com","threadId":"37030","inReplyTo":"1404163227-30962-1-git-send-email-charles@hashpling.org","subject":"Re: [PATCH] Fix filter-branch to eliminate duplicate mapped parents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-01T15:03:46Z","receivedAt":"2014-07-01T15:03:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Charles Bailey <charles@hashpling.org> writes:\n\n> I worked on this after discovering that --prune-empty often left some\n> apparently empty commits that I was wasn't expecting it to leave and\n> that running filter-branch --prune-empty in a loop would often do many\n> passes where it was still pruning empty former merge commits.\n\nGood find.\n\n>\n> The test is a simple example of such a case. A non-ff merge of a commit\n> that only changes a file that is to be pruned gets squashed into an\n> empty non-merge commit that should be pruned.\n>\n>  git-filter-branch.sh     |  8 +++++++-\n>  t/t7003-filter-branch.sh | 11 +++++++++++\n>  2 files changed, 18 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 86d6994..c5b82a8 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -332,7 +332,13 @@ while read commit parents; do\n>  \tparentstr=\n>  \tfor parent in $parents; do\n>  \t\tfor reparent in $(map \"$parent\"); do\n> -\t\t\tparentstr=\"$parentstr -p $reparent\"\n> +\t\t\tcase \"$parentstr\" in\n> +\t\t\t\t*\" -p $reparent\"*)\n> +\t\t\t\t\t;;\n> +\t\t\t\t*)\n> +\t\t\t\t\tparentstr=\"$parentstr -p $reparent\"\n> +\t\t\t\t\t;;\n> +\t\t\tesac\n\nThe case arms seem to be indented one level too deep; I'll fix it up\nlocally when queuing, so no need to resend.\n\n\nThanks.\n"}]}