Re: [PATCH 3/4] filter-branch: fix --prune-empty on parentless commits

"Devin J. Pohly" [off-list ref] writes:

Previously, the git_commit_non_empty_tree function would always pass any
commit with no parents to git-commit-tree, regardless of whether the
tree was nonempty.  The new commit would then be recorded in the
filter-branch revision map, and subsequent commits which leave the tree
untouched would be correctly filtered.

With this change, parentless commits with an empty tree are correctly
pruned, and an empty file is recorded in the revision map, signifying
that it was rewritten to "no commits."  This works naturally with the
parent mapping for subsequent commits.

Signed-off-by: Devin J. Pohly <redacted>
---

I am not sure if a root that records an empty tree should be pruned
with --prune-empty to begin with.

When we are pruning consecutive commits in the other parts of the
history because they have identical (presumably non-empty) trees,
should an empty root that the original history wanted to create be
pruned because before the commit it was void, after the commit it is
empty?  Should "void" (lack of any tree) and "empty" (the tree is
there, but it does not have anything in it) be treated the same?
Shouldn't root be treated as a bit more special thing?

I myself do not have a good answer to the above questions.

I think the updated code makes sense, provided if we decide that
void to empty is just like transitioning between two identical
(presumably non-empty) trees.  The updated documentation is a lot
more readable as well.

Comments from those who have been involved in filter-branch?

On Thu, Feb 23, 2017 at 01:17:49PM -0800, Junio C Hamano wrote:

"Devin J. Pohly" [off-list ref] writes:

quoted

Previously, the git_commit_non_empty_tree function would always pass any
commit with no parents to git-commit-tree, regardless of whether the
tree was nonempty.  The new commit would then be recorded in the
filter-branch revision map, and subsequent commits which leave the tree
untouched would be correctly filtered.

With this change, parentless commits with an empty tree are correctly
pruned, and an empty file is recorded in the revision map, signifying
that it was rewritten to "no commits."  This works naturally with the
parent mapping for subsequent commits.

Signed-off-by: Devin J. Pohly <redacted>
---

I am not sure if a root that records an empty tree should be pruned
with --prune-empty to begin with.

When we are pruning consecutive commits in the other parts of the
history because they have identical (presumably non-empty) trees,
should an empty root that the original history wanted to create be
pruned because before the commit it was void, after the commit it is
empty?  Should "void" (lack of any tree) and "empty" (the tree is
there, but it does not have anything in it) be treated the same?
Shouldn't root be treated as a bit more special thing?

The case I had in mind was a filter which happened to remove all changes
from any parentless commit (see the testcase added to t7003).  It would
not necessarily have been an empty commit in the original history.

Use case/motivation: I am splitting my dotfiles repo to migrate to vcsh,
and the original first commit (which only touches a few files) appears
in every branch.  In the branches which do not include those files, the
commit is empty but still present.

I think your point is interesting too, though.  If a commit is also
TREESAME to its parent(s?) in the _pre-filtered_ branch, it seems
reasonable that someone might want to leave it in the filtered branch as
an empty commit while pruning empt*ied* commits.  I would imagine that
as another option (--prune-newly-empty?).

I myself do not have a good answer to the above questions.

I think the updated code makes sense, provided if we decide that
void to empty is just like transitioning between two identical
(presumably non-empty) trees.  The updated documentation is a lot
more readable as well.

Comments from those who have been involved in filter-branch?

quoted

 Documentation/git-filter-branch.txt | 14 ++++++--------
 git-filter-branch.sh                |  2 ++
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 0a09698c0..6e4bb0220 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt

@@ -167,14 +167,12 @@ to other tags will be rewritten to point to the underlying commit.
 	project root. Implies <<Remap_to_ancestor>>.
 
 --prune-empty::
-	Some kind of filters will generate empty commits, that left the tree
-	untouched.  This switch allow git-filter-branch to ignore such
-	commits.  Though, this switch only applies for commits that have one
-	and only one parent, it will hence keep merges points. Also, this
-	option is not compatible with the use of `--commit-filter`. Though you
-	just need to use the function 'git_commit_non_empty_tree "$@"' instead
-	of the `git commit-tree "$@"` idiom in your commit filter to make that
-	happen.
+	Some filters will generate empty commits that leave the tree untouched.
+	This option instructs git-filter-branch to remove such commits if they
+	have exactly one or zero non-pruned parents; merge commits will
+	therefore remain intact.  This option cannot be used together with
+	`--commit-filter`, though the same effect can be achieved by using the
+	provided `git_commit_non_empty_tree` function in a commit filter.
 
 --original <namespace>::
 	Use this option to set the namespace where the original commits

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 86b2ff1e0..2b8cdba15 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh

@@ -46,6 +46,8 @@ git_commit_non_empty_tree()
 {
 	if test $# = 3 && test "$1" = $(git rev-parse "$3^{tree}"); then
 		map "$3"
+	elif test $# = 1 && test "$1" = 4b825dc642cb6eb9a060e54bf8d69288fbee4904; then
+		:
 	else
 		git commit-tree "$@"
 	fi

-- 
<><