{"thread":{"id":"45198","subject":"[PATCH 1/4] t7003: ensure --prune-empty can prune root commit","startedAt":"2017-02-23T08:27:30Z","lastAt":"2017-03-03T20:48:43Z","messageCount":15,"participants":["Devin J. Pohly","Junio C Hamano","Jacob Keller","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"312382","messageId":"20170223082736.31283-1-djpohly@gmail.com","threadId":"45198","inReplyTo":null,"subject":"[PATCH 1/4] t7003: ensure --prune-empty can prune root commit","fromName":"Devin J. Pohly","fromEmail":"djpohly@gmail.com","sentAt":"2017-02-23T08:27:33Z","receivedAt":"2017-02-23T08:27:30Z","isPatch":true,"sender":{"key":"djpohly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/521294?v=4"},"body":"New test to expose a bug in filter-branch whereby the root commit is\nnever pruned, even though its tree is empty and --prune-empty is given.\n\nThe setup isn't exactly pretty, but I couldn't think of a simpler way to\ncreate a parallel commit graph sans the first commit.\n\nSigned-off-by: Devin J. Pohly <djpohly@gmail.com>\n---\n t/t7003-filter-branch.sh | 30 ++++++++++++++++++++++++++++++\n 1 file changed, 30 insertions(+)\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex cb8fbd8e5..2dfe46250 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -313,6 +313,27 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '\n \tgit cat-file tag X/2 > actual &&\n \ttest_cmp expect actual\n '\n+test_expect_success 'setup --prune-empty comparisons' '\n+\tgit checkout --orphan master-no-a &&\n+\tgit rm -rf . &&\n+\tunset test_tick &&\n+\ttest_tick &&\n+\tGIT_COMMITTER_DATE=\"@0 +0000\" GIT_AUTHOR_DATE=\"@0 +0000\" &&\n+\ttest_commit --notick B B.t B Bx &&\n+\tgit checkout -b branch-no-a Bx &&\n+\ttest_commit D D.t D Dx &&\n+\tmkdir dir &&\n+\ttest_commit dir/D dir/D.t dir/D dir/Dx &&\n+\ttest_commit E E.t E Ex &&\n+\tgit checkout master-no-a &&\n+\ttest_commit C C.t C Cx &&\n+\tgit checkout branch-no-a &&\n+\tgit merge Cx -m \"Merge tag '\\''C'\\'' into branch\" &&\n+\tgit tag Fx &&\n+\ttest_commit G G.t G Gx &&\n+\ttest_commit H H.t H Hx &&\n+\tgit checkout branch\n+'\n \n test_expect_success 'Prune empty commits' '\n \tgit rev-list HEAD > expect &&\n@@ -341,6 +362,15 @@ test_expect_success 'prune empty works even without index/tree filters' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--prune-empty is able to prune root commit' '\n+\tgit rev-list branch-no-a >expect &&\n+\tgit branch testing H &&\n+\tgit filter-branch -f --prune-empty --index-filter \"git update-index --remove A.t\" testing &&\n+\tgit rev-list testing >actual &&\n+\tgit branch -D testing &&\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-- \n2.11.1\n\n"},{"id":"312383","messageId":"20170223082736.31283-2-djpohly@gmail.com","threadId":"45198","inReplyTo":"20170223082736.31283-1-djpohly@gmail.com","subject":"[PATCH 2/4] t7003: ensure --prune-empty removes entire branch when applicable","fromName":"Devin J. Pohly","fromEmail":"djpohly@gmail.com","sentAt":"2017-02-23T08:27:34Z","receivedAt":"2017-02-23T08:27:34Z","isPatch":true,"sender":{"key":"djpohly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/521294?v=4"},"body":"Sanity check before changing the logic in git_commit_non_empty_tree.\n\nSigned-off-by: Devin J. Pohly <djpohly@gmail.com>\n---\n t/t7003-filter-branch.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 2dfe46250..7cb60799b 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -371,6 +371,13 @@ test_expect_success '--prune-empty is able to prune root commit' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--prune-empty is able to prune entire branch' '\n+\tgit branch prune-entire B &&\n+\tgit filter-branch -f --prune-empty --index-filter \"git update-index --remove A.t B.t\" prune-entire &&\n+\ttest_path_is_missing .git/refs/heads/prune-entire &&\n+\ttest_must_fail git reflog exists refs/heads/prune-entire\n+'\n+\n test_expect_success '--remap-to-ancestor with filename filters' '\n \tgit checkout master &&\n \tgit reset --hard A &&\n-- \n2.11.1\n\n"},{"id":"312384","messageId":"20170223082736.31283-3-djpohly@gmail.com","threadId":"45198","inReplyTo":"20170223082736.31283-1-djpohly@gmail.com","subject":"[PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Devin J. Pohly","fromEmail":"djpohly@gmail.com","sentAt":"2017-02-23T08:27:35Z","receivedAt":"2017-02-23T08:27:39Z","isPatch":true,"sender":{"key":"djpohly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/521294?v=4"},"body":"Previously, the git_commit_non_empty_tree function would always pass any\ncommit with no parents to git-commit-tree, regardless of whether the\ntree was nonempty.  The new commit would then be recorded in the\nfilter-branch revision map, and subsequent commits which leave the tree\nuntouched would be correctly filtered.\n\nWith this change, parentless commits with an empty tree are correctly\npruned, and an empty file is recorded in the revision map, signifying\nthat it was rewritten to \"no commits.\"  This works naturally with the\nparent mapping for subsequent commits.\n\nSigned-off-by: Devin J. Pohly <djpohly@gmail.com>\n---\n Documentation/git-filter-branch.txt | 14 ++++++--------\n git-filter-branch.sh                |  2 ++\n 2 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt\nindex 0a09698c0..6e4bb0220 100644\n--- a/Documentation/git-filter-branch.txt\n+++ b/Documentation/git-filter-branch.txt\n@@ -167,14 +167,12 @@ to other tags will be rewritten to point to the underlying commit.\n \tproject root. Implies <<Remap_to_ancestor>>.\n \n --prune-empty::\n-\tSome kind of filters will generate empty commits, that left the tree\n-\tuntouched.  This switch allow git-filter-branch to ignore such\n-\tcommits.  Though, this switch only applies for commits that have one\n-\tand only one parent, it will hence keep merges points. Also, this\n-\toption is not compatible with the use of `--commit-filter`. Though you\n-\tjust need to use the function 'git_commit_non_empty_tree \"$@\"' instead\n-\tof the `git commit-tree \"$@\"` idiom in your commit filter to make that\n-\thappen.\n+\tSome filters will generate empty commits that leave the tree untouched.\n+\tThis option instructs git-filter-branch to remove such commits if they\n+\thave exactly one or zero non-pruned parents; merge commits will\n+\ttherefore remain intact.  This option cannot be used together with\n+\t`--commit-filter`, though the same effect can be achieved by using the\n+\tprovided `git_commit_non_empty_tree` function in a commit filter.\n \n --original <namespace>::\n \tUse this option to set the namespace where the original commits\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 86b2ff1e0..2b8cdba15 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -46,6 +46,8 @@ git_commit_non_empty_tree()\n {\n \tif test $# = 3 && test \"$1\" = $(git rev-parse \"$3^{tree}\"); then\n \t\tmap \"$3\"\n+\telif test $# = 1 && test \"$1\" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904; then\n+\t\t:\n \telse\n \t\tgit commit-tree \"$@\"\n \tfi\n-- \n2.11.1\n\n"},{"id":"312385","messageId":"20170223082736.31283-4-djpohly@gmail.com","threadId":"45198","inReplyTo":"20170223082736.31283-1-djpohly@gmail.com","subject":"[PATCH 4/4] p7000: add test for filter-branch with --prune-empty","fromName":"Devin J. Pohly","fromEmail":"djpohly@gmail.com","sentAt":"2017-02-23T08:27:36Z","receivedAt":"2017-02-23T08:27:41Z","isPatch":true,"sender":{"key":"djpohly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/521294?v=4"},"body":"Signed-off-by: Devin J. Pohly <djpohly@gmail.com>\n---\n t/perf/p7000-filter-branch.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/perf/p7000-filter-branch.sh b/t/perf/p7000-filter-branch.sh\nindex 15ee5d1d5..b029586cc 100755\n--- a/t/perf/p7000-filter-branch.sh\n+++ b/t/perf/p7000-filter-branch.sh\n@@ -16,4 +16,9 @@ test_perf 'noop filter' '\n \tgit filter-branch -f base..HEAD\n '\n \n+test_perf 'noop prune-empty' '\n+\tgit checkout --detach tip &&\n+\tgit filter-branch -f --prune-empty base..HEAD\n+'\n+\n test_done\n-- \n2.11.1\n\n"},{"id":"312441","messageId":"xmqqbmts7gjm.fsf@gitster.mtv.corp.google.com","threadId":"45198","inReplyTo":"20170223082736.31283-3-djpohly@gmail.com","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-23T21:17:49Z","receivedAt":"2017-02-23T21:18:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Devin J. Pohly\" <djpohly@gmail.com> writes:\n\n> Previously, the git_commit_non_empty_tree function would always pass any\n> commit with no parents to git-commit-tree, regardless of whether the\n> tree was nonempty.  The new commit would then be recorded in the\n> filter-branch revision map, and subsequent commits which leave the tree\n> untouched would be correctly filtered.\n>\n> With this change, parentless commits with an empty tree are correctly\n> pruned, and an empty file is recorded in the revision map, signifying\n> that it was rewritten to \"no commits.\"  This works naturally with the\n> parent mapping for subsequent commits.\n>\n> Signed-off-by: Devin J. Pohly <djpohly@gmail.com>\n> ---\n\nI am not sure if a root that records an empty tree should be pruned\nwith --prune-empty to begin with.\n\nWhen we are pruning consecutive commits in the other parts of the\nhistory because they have identical (presumably non-empty) trees,\nshould an empty root that the original history wanted to create be\npruned because before the commit it was void, after the commit it is\nempty?  Should \"void\" (lack of any tree) and \"empty\" (the tree is\nthere, but it does not have anything in it) be treated the same?\nShouldn't root be treated as a bit more special thing?\n\nI myself do not have a good answer to the above questions.\n\nI think the updated code makes sense, provided if we decide that\nvoid to empty is just like transitioning between two identical\n(presumably non-empty) trees.  The updated documentation is a lot\nmore readable as well.\n\nComments from those who have been involved in filter-branch?\n\n>  Documentation/git-filter-branch.txt | 14 ++++++--------\n>  git-filter-branch.sh                |  2 ++\n>  2 files changed, 8 insertions(+), 8 deletions(-)\n>\n> diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt\n> index 0a09698c0..6e4bb0220 100644\n> --- a/Documentation/git-filter-branch.txt\n> +++ b/Documentation/git-filter-branch.txt\n> @@ -167,14 +167,12 @@ to other tags will be rewritten to point to the underlying commit.\n>  \tproject root. Implies <<Remap_to_ancestor>>.\n>  \n>  --prune-empty::\n> -\tSome kind of filters will generate empty commits, that left the tree\n> -\tuntouched.  This switch allow git-filter-branch to ignore such\n> -\tcommits.  Though, this switch only applies for commits that have one\n> -\tand only one parent, it will hence keep merges points. Also, this\n> -\toption is not compatible with the use of `--commit-filter`. Though you\n> -\tjust need to use the function 'git_commit_non_empty_tree \"$@\"' instead\n> -\tof the `git commit-tree \"$@\"` idiom in your commit filter to make that\n> -\thappen.\n> +\tSome filters will generate empty commits that leave the tree untouched.\n> +\tThis option instructs git-filter-branch to remove such commits if they\n> +\thave exactly one or zero non-pruned parents; merge commits will\n> +\ttherefore remain intact.  This option cannot be used together with\n> +\t`--commit-filter`, though the same effect can be achieved by using the\n> +\tprovided `git_commit_non_empty_tree` function in a commit filter.\n>  \n>  --original <namespace>::\n>  \tUse this option to set the namespace where the original commits\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 86b2ff1e0..2b8cdba15 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -46,6 +46,8 @@ git_commit_non_empty_tree()\n>  {\n>  \tif test $# = 3 && test \"$1\" = $(git rev-parse \"$3^{tree}\"); then\n>  \t\tmap \"$3\"\n> +\telif test $# = 1 && test \"$1\" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904; then\n> +\t\t:\n>  \telse\n>  \t\tgit commit-tree \"$@\"\n>  \tfi\n"},{"id":"312443","messageId":"20170223213333.GA3490@prospect.localdomain","threadId":"45198","inReplyTo":"xmqqbmts7gjm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Devin J. Pohly","fromEmail":"djpohly@gmail.com","sentAt":"2017-02-23T21:33:33Z","receivedAt":"2017-02-23T21:33:20Z","isPatch":true,"sender":{"key":"djpohly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/521294?v=4"},"body":"On Thu, Feb 23, 2017 at 01:17:49PM -0800, Junio C Hamano wrote:\n> \"Devin J. Pohly\" <djpohly@gmail.com> writes:\n> \n> > Previously, the git_commit_non_empty_tree function would always pass any\n> > commit with no parents to git-commit-tree, regardless of whether the\n> > tree was nonempty.  The new commit would then be recorded in the\n> > filter-branch revision map, and subsequent commits which leave the tree\n> > untouched would be correctly filtered.\n> >\n> > With this change, parentless commits with an empty tree are correctly\n> > pruned, and an empty file is recorded in the revision map, signifying\n> > that it was rewritten to \"no commits.\"  This works naturally with the\n> > parent mapping for subsequent commits.\n> >\n> > Signed-off-by: Devin J. Pohly <djpohly@gmail.com>\n> > ---\n> \n> I am not sure if a root that records an empty tree should be pruned\n> with --prune-empty to begin with.\n> \n> When we are pruning consecutive commits in the other parts of the\n> history because they have identical (presumably non-empty) trees,\n> should an empty root that the original history wanted to create be\n> pruned because before the commit it was void, after the commit it is\n> empty?  Should \"void\" (lack of any tree) and \"empty\" (the tree is\n> there, but it does not have anything in it) be treated the same?\n> Shouldn't root be treated as a bit more special thing?\n>\n\nThe case I had in mind was a filter which happened to remove all changes\nfrom any parentless commit (see the testcase added to t7003).  It would\nnot necessarily have been an empty commit in the original history.\n\nUse case/motivation: I am splitting my dotfiles repo to migrate to vcsh,\nand the original first commit (which only touches a few files) appears\nin every branch.  In the branches which do not include those files, the\ncommit is empty but still present.\n\nI think your point is interesting too, though.  If a commit is also\nTREESAME to its parent(s?) in the _pre-filtered_ branch, it seems\nreasonable that someone might want to leave it in the filtered branch as\nan empty commit while pruning empt*ied* commits.  I would imagine that\nas another option (--prune-newly-empty?).\n\n> \n> I myself do not have a good answer to the above questions.\n> \n> I think the updated code makes sense, provided if we decide that\n> void to empty is just like transitioning between two identical\n> (presumably non-empty) trees.  The updated documentation is a lot\n> more readable as well.\n> \n> Comments from those who have been involved in filter-branch?\n> \n> >  Documentation/git-filter-branch.txt | 14 ++++++--------\n> >  git-filter-branch.sh                |  2 ++\n> >  2 files changed, 8 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt\n> > index 0a09698c0..6e4bb0220 100644\n> > --- a/Documentation/git-filter-branch.txt\n> > +++ b/Documentation/git-filter-branch.txt\n> > @@ -167,14 +167,12 @@ to other tags will be rewritten to point to the underlying commit.\n> >  \tproject root. Implies <<Remap_to_ancestor>>.\n> >  \n> >  --prune-empty::\n> > -\tSome kind of filters will generate empty commits, that left the tree\n> > -\tuntouched.  This switch allow git-filter-branch to ignore such\n> > -\tcommits.  Though, this switch only applies for commits that have one\n> > -\tand only one parent, it will hence keep merges points. Also, this\n> > -\toption is not compatible with the use of `--commit-filter`. Though you\n> > -\tjust need to use the function 'git_commit_non_empty_tree \"$@\"' instead\n> > -\tof the `git commit-tree \"$@\"` idiom in your commit filter to make that\n> > -\thappen.\n> > +\tSome filters will generate empty commits that leave the tree untouched.\n> > +\tThis option instructs git-filter-branch to remove such commits if they\n> > +\thave exactly one or zero non-pruned parents; merge commits will\n> > +\ttherefore remain intact.  This option cannot be used together with\n> > +\t`--commit-filter`, though the same effect can be achieved by using the\n> > +\tprovided `git_commit_non_empty_tree` function in a commit filter.\n> >  \n> >  --original <namespace>::\n> >  \tUse this option to set the namespace where the original commits\n> > diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> > index 86b2ff1e0..2b8cdba15 100755\n> > --- a/git-filter-branch.sh\n> > +++ b/git-filter-branch.sh\n> > @@ -46,6 +46,8 @@ git_commit_non_empty_tree()\n> >  {\n> >  \tif test $# = 3 && test \"$1\" = $(git rev-parse \"$3^{tree}\"); then\n> >  \t\tmap \"$3\"\n> > +\telif test $# = 1 && test \"$1\" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904; then\n> > +\t\t:\n> >  \telse\n> >  \t\tgit commit-tree \"$@\"\n> >  \tfi\n\n-- \n<><\n"},{"id":"313092","messageId":"xmqqshmvbhe5.fsf@gitster.mtv.corp.google.com","threadId":"45198","inReplyTo":"20170223213333.GA3490@prospect.localdomain","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-02T19:36:18Z","receivedAt":"2017-03-02T19:36:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Devin J. Pohly\" <djpohly@gmail.com> writes:\n\n> I think your point is interesting too, though.  If a commit is also\n> TREESAME to its parent(s?) in the _pre-filtered_ branch, it seems\n> reasonable that someone might want to leave it in the filtered branch as\n> an empty commit while pruning empt*ied* commits.  I would imagine that\n> as another option (--prune-newly-empty?).\n\nI was hoping to hear from others who may care about filter-branch to\ncomment on this topic to help me decide, but I haven't heard\nanything, so here is my tentative thinking.\n\nI am leaning to:\n\n * Take your series as-is, which would mean --prune-empty will\n   change the behaviour to unconditionally lose the empty root.\n\n * Then, people who care deeply about it can add a new option that\n   prunes commits that become empty while keeping the originally\n   empty ones.\n\nThoughts?\n"},{"id":"313103","messageId":"20170302211804.GB3490@prospect.localdomain","threadId":"45198","inReplyTo":"xmqqshmvbhe5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Devin J. Pohly","fromEmail":"djpohly@gmail.com","sentAt":"2017-03-02T21:18:04Z","receivedAt":"2017-03-02T21:21:36Z","isPatch":true,"sender":{"key":"djpohly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/521294?v=4"},"body":"On Thu, Mar 02, 2017 at 11:36:18AM -0800, Junio C Hamano wrote:\n> \"Devin J. Pohly\" <djpohly@gmail.com> writes:\n> \n> > I think your point is interesting too, though.  If a commit is also\n> > TREESAME to its parent(s?) in the _pre-filtered_ branch, it seems\n> > reasonable that someone might want to leave it in the filtered branch as\n> > an empty commit while pruning empt*ied* commits.  I would imagine that\n> > as another option (--prune-newly-empty?).\n> \n> I was hoping to hear from others who may care about filter-branch to\n> comment on this topic to help me decide, but I haven't heard\n> anything, so here is my tentative thinking.\n> \n> I am leaning to:\n> \n>  * Take your series as-is, which would mean --prune-empty will\n>    change the behaviour to unconditionally lose the empty root.\n> \n>  * Then, people who care deeply about it can add a new option that\n>    prunes commits that become empty while keeping the originally\n>    empty ones.\n> \n> Thoughts?\n\nSounds good to me.  I would be willing to work on a new option if needed\n(to \"atone\" for changing existing behavior), so you can loop me in if\nthere are any complaints.\n\n-- \n<><\n"},{"id":"313105","messageId":"xmqqbmtjbbp4.fsf@gitster.mtv.corp.google.com","threadId":"45198","inReplyTo":"20170302211804.GB3490@prospect.localdomain","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-02T21:39:19Z","receivedAt":"2017-03-02T22:34:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Devin J. Pohly\" <djpohly@gmail.com> writes:\n\n> On Thu, Mar 02, 2017 at 11:36:18AM -0800, Junio C Hamano wrote:\n>> \"Devin J. Pohly\" <djpohly@gmail.com> writes:\n>> \n>> > I think your point is interesting too, though.  If a commit is also\n>> > TREESAME to its parent(s?) in the _pre-filtered_ branch, it seems\n>> > reasonable that someone might want to leave it in the filtered branch as\n>> > an empty commit while pruning empt*ied* commits.  I would imagine that\n>> > as another option (--prune-newly-empty?).\n>> \n>> I was hoping to hear from others who may care about filter-branch to\n>> comment on this topic to help me decide, but I haven't heard\n>> anything, so here is my tentative thinking.\n>> \n>> I am leaning to:\n>> \n>>  * Take your series as-is, which would mean --prune-empty will\n>>    change the behaviour to unconditionally lose the empty root.\n>> \n>>  * Then, people who care deeply about it can add a new option that\n>>    prunes commits that become empty while keeping the originally\n>>    empty ones.\n>> \n>> Thoughts?\n>\n> Sounds good to me.  I would be willing to work on a new option if needed\n> (to \"atone\" for changing existing behavior), so you can loop me in if\n> there are any complaints.\n\nThanks.  I'll wait for others who know filter-branch better than me\nto say something for a few days before doing anything, though.\n"},{"id":"313107","messageId":"CA+P7+xpBA+JQbH1=yKL01-rL9z0GKtY1pxBa8feXV41ebOpAPQ@mail.gmail.com","threadId":"45198","inReplyTo":"xmqqshmvbhe5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-03-02T23:28:33Z","receivedAt":"2017-03-02T23:38:01Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Mar 2, 2017 at 11:36 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Devin J. Pohly\" <djpohly@gmail.com> writes:\n>\n>> I think your point is interesting too, though.  If a commit is also\n>> TREESAME to its parent(s?) in the _pre-filtered_ branch, it seems\n>> reasonable that someone might want to leave it in the filtered branch as\n>> an empty commit while pruning empt*ied* commits.  I would imagine that\n>> as another option (--prune-newly-empty?).\n>\n> I was hoping to hear from others who may care about filter-branch to\n> comment on this topic to help me decide, but I haven't heard\n> anything, so here is my tentative thinking.\n>\n> I am leaning to:\n>\n>  * Take your series as-is, which would mean --prune-empty will\n>    change the behaviour to unconditionally lose the empty root.\n>\n\nThis new behavior is how I expected prune-empty to behave, so seeing\nthat it did not already behave this way was surprising.\n\nThanks,\nJake\n"},{"id":"313142","messageId":"20170303075535.rxij4knd7d4ayfev@sigill.intra.peff.net","threadId":"45198","inReplyTo":"xmqqshmvbhe5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-03T07:55:35Z","receivedAt":"2017-03-03T08:56:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 02, 2017 at 11:36:18AM -0800, Junio C Hamano wrote:\n\n> \"Devin J. Pohly\" <djpohly@gmail.com> writes:\n> \n> > I think your point is interesting too, though.  If a commit is also\n> > TREESAME to its parent(s?) in the _pre-filtered_ branch, it seems\n> > reasonable that someone might want to leave it in the filtered branch as\n> > an empty commit while pruning empt*ied* commits.  I would imagine that\n> > as another option (--prune-newly-empty?).\n> \n> I was hoping to hear from others who may care about filter-branch to\n> comment on this topic to help me decide, but I haven't heard\n> anything, so here is my tentative thinking.\n> \n> I am leaning to:\n> \n>  * Take your series as-is, which would mean --prune-empty will\n>    change the behaviour to unconditionally lose the empty root.\n> \n>  * Then, people who care deeply about it can add a new option that\n>    prunes commits that become empty while keeping the originally\n>    empty ones.\n> \n> Thoughts?\n\nSorry, this was on my to-review list but the sha1 stuff has been much\nmore exciting. :)\n\nThe behavior that Devin proposes is what I would have expected to\nhappen. Between \"prune-existing-empty\" and \"prune-became-empty\", I've\nnever had a use for the distinction. But as I think this is similar to\nthe way cherry-pick distinguished between \"redundant\" and \"empty\", I\nguess some people have.\n\nI agree that it should be a new, separate option, as it's orthogonal to\ndealing with the root commit (i.e., somebody is equally likely to want\nto preserve an already-empty commit from the middle of history).\n\nThe change to filter-branch itself looks obviously correct. The only\nobjectionable thing I noticed in the test additions is that the early\nones should be marked test_expect_failure until the fix from 3/4 flips\nthem to \"success\". Otherwise it breaks bisectability.\n\n-Peff\n"},{"id":"313146","messageId":"20170303075605.3pqqzmjcqdtv2bjd@sigill.intra.peff.net","threadId":"45198","inReplyTo":"20170223082736.31283-4-djpohly@gmail.com","subject":"Re: [PATCH 4/4] p7000: add test for filter-branch with --prune-empty","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-03T07:56:05Z","receivedAt":"2017-03-03T09:19:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 23, 2017 at 02:27:36AM -0600, Devin J. Pohly wrote:\n\n> +test_perf 'noop prune-empty' '\n> +\tgit checkout --detach tip &&\n> +\tgit filter-branch -f --prune-empty base..HEAD\n> +'\n\nI don't mind adding this, but of curiosity, does it show anything\ninteresting?\n\n-Peff\n"},{"id":"313230","messageId":"20170303203025.GA4763@prospect.localdomain","threadId":"45198","inReplyTo":"20170303075535.rxij4knd7d4ayfev@sigill.intra.peff.net","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Devin J. Pohly","fromEmail":"djpohly@gmail.com","sentAt":"2017-03-03T20:30:25Z","receivedAt":"2017-03-03T20:31:10Z","isPatch":true,"sender":{"key":"djpohly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/521294?v=4"},"body":"On Fri, Mar 03, 2017 at 02:55:35AM -0500, Jeff King wrote:\n> The only objectionable thing I noticed in the test additions is that\n> the early ones should be marked test_expect_failure until the fix from\n> 3/4 flips them to \"success\". Otherwise it breaks bisectability.\n> \n> -Peff\n\nGood point.  Will fix in a v2 set then.\n\n-- \n<><\n"},{"id":"313231","messageId":"20170303203449.GB4763@prospect.localdomain","threadId":"45198","inReplyTo":"20170303075605.3pqqzmjcqdtv2bjd@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] p7000: add test for filter-branch with --prune-empty","fromName":"Devin J. Pohly","fromEmail":"djpohly@gmail.com","sentAt":"2017-03-03T20:34:49Z","receivedAt":"2017-03-03T20:35:15Z","isPatch":true,"sender":{"key":"djpohly@gmail.com","avatar":"https://avatars.githubusercontent.com/u/521294?v=4"},"body":"On Fri, Mar 03, 2017 at 02:56:05AM -0500, Jeff King wrote:\n> On Thu, Feb 23, 2017 at 02:27:36AM -0600, Devin J. Pohly wrote:\n> \n> > +test_perf 'noop prune-empty' '\n> > +\tgit checkout --detach tip &&\n> > +\tgit filter-branch -f --prune-empty base..HEAD\n> > +'\n> \n> I don't mind adding this, but of curiosity, does it show anything\n> interesting?\n> \n> -Peff\n\nNothing surprising; the overhead for the change was minimal.  I wasn't\nsure what the practice is for adding unit/perf tests, so I erred on the\nside of covering everything.\n\nI will leave this as the last commit in the series so that it can be\ndropped or merged as you see fit.\n\n-- \n<><\n"},{"id":"313232","messageId":"xmqq8tom8526.fsf@gitster.mtv.corp.google.com","threadId":"45198","inReplyTo":"20170303075535.rxij4knd7d4ayfev@sigill.intra.peff.net","subject":"Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-03T20:43:13Z","receivedAt":"2017-03-03T20:48:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> The change to filter-branch itself looks obviously correct. The only\n> objectionable thing I noticed in the test additions is that the early\n> ones should be marked test_expect_failure until the fix from 3/4 flips\n> them to \"success\". Otherwise it breaks bisectability.\n\nI'll squash in the necessary changes to flip between expect_success\nand expect_failure in the appropriate places and re-queue on 'pu'.\n\nThanks.\n\n\ncommit 07cac4a5fdfeeb3c1c8385e222100d575a4460b0\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Fri Mar 3 11:41:36 2017 -0800\n\n    fixup! t7003: ensure --prune-empty can prune root commit\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 2dfe462501..45372a1cba 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -362,7 +362,7 @@ test_expect_success 'prune empty works even without index/tree filters' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--prune-empty is able to prune root commit' '\n+test_expect_failure '--prune-empty is able to prune root commit' '\n \tgit rev-list branch-no-a >expect &&\n \tgit branch testing H &&\n \tgit filter-branch -f --prune-empty --index-filter \"git update-index --remove A.t\" testing &&\n\n\ncommit 562ed048c681686426ca95e0e550378b48aa4852\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Fri Mar 3 12:11:25 2017 -0800\n\n    fixup! t7003: ensure --prune-empty removes entire branch when applicable\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex a774a8e4b3..40526d1716 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -371,7 +371,7 @@ test_expect_failure '--prune-empty is able to prune root commit' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--prune-empty is able to prune entire branch' '\n+test_expect_failure '--prune-empty is able to prune entire branch' '\n \tgit branch prune-entire B &&\n \tgit filter-branch -f --prune-empty --index-filter \"git update-index --remove A.t B.t\" prune-entire &&\n \ttest_path_is_missing .git/refs/heads/prune-entire &&\n\n\ncommit 520534c4035a13c54229dab0320e745d18635ef3\nAuthor: Junio C Hamano <gitster@pobox.com>\nDate:   Fri Mar 3 12:39:58 2017 -0800\n\n    fixup! filter-branch: fix --prune-empty on parentless commits\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 40526d1716..7cb60799be 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -362,7 +362,7 @@ test_expect_success 'prune empty works even without index/tree filters' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure '--prune-empty is able to prune root commit' '\n+test_expect_success '--prune-empty is able to prune root commit' '\n \tgit rev-list branch-no-a >expect &&\n \tgit branch testing H &&\n \tgit filter-branch -f --prune-empty --index-filter \"git update-index --remove A.t\" testing &&\n@@ -371,7 +371,7 @@ test_expect_failure '--prune-empty is able to prune root commit' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure '--prune-empty is able to prune entire branch' '\n+test_expect_success '--prune-empty is able to prune entire branch' '\n \tgit branch prune-entire B &&\n \tgit filter-branch -f --prune-empty --index-filter \"git update-index --remove A.t B.t\" prune-entire &&\n \ttest_path_is_missing .git/refs/heads/prune-entire &&\n"}]}