[PATCH] git-filter-branch: Add an example on how to remove empty commits

[PATCH] git-filter-branch: Add an example on how to remove empty commits

[Petr Baudis]

From: Sverre Rabbelier <srabbelier@gmail.com>

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
 Documentation/git-filter-branch.txt |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index fed6de6..2565244 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -246,6 +246,21 @@ git filter-branch --commit-filter '
 	fi' HEAD
 ------------------------------------------------------------------------------
 
+To remove commits that are empty (do not introduce any change):
+
+------------------------------------------------------------------------------
+git rev-list HEAD | while read c; do [ -n "$(git diff-tree --root $c)" ] || echo $c; done > revs
+
+git filter-branch --commit-filter '
+  if grep -q "$GIT_COMMIT" '"$(pwd)/"revs';
+  then
+    skip_commit "$@";
+  else
+    git commit-tree "$@";
+  fi' HEAD
+
+------------------------------------------------------------------------------
+
 The function 'skip_commit' is defined as follows:
 
 --------------------------
-- 
1.5.6.3.536.g61aad

Re: [PATCH] git-filter-branch: Add an example on how to remove empty commits

[Sam Vilain]

On Wed, 2008-10-29 at 17:33 -0700, Petr Baudis wrote:
> +To remove commits that are empty (do not introduce any change):
> +
> +------------------------------------------------------------------------------
> +git rev-list HEAD | while read c; do [ -n "$(git diff-tree --root $c)" ] || echo $c; done > revs
> +
> +git filter-branch --commit-filter '
> +  if grep -q "$GIT_COMMIT" '"$(pwd)/"revs';
> +  then

Why not put the git diff-tree in the commit filter?

Is this tested?  It doesn't look like it does what the comment says...
surely you have to compare with the previous commit, not the null
commit?

Sam.

Re: [PATCH] git-filter-branch: Add an example on how to remove empty commits

[Johannes Schindelin]

Hi,

On Wed, 29 Oct 2008, Petr Baudis wrote:

> +To remove commits that are empty (do not introduce any change):
> +
> +------------------------------------------------------------------------------
> +git rev-list HEAD | while read c; do [ -n "$(git diff-tree --root $c)" ] || echo $c; done > revs
> +
> +git filter-branch --commit-filter '
> +  if grep -q "$GIT_COMMIT" '"$(pwd)/"revs';
> +  then
> +    skip_commit "$@";
> +  else
> +    git commit-tree "$@";
> +  fi' HEAD

You would not need to use the temporary "revs" file by using something 
(totally untested, of course):

git filter-branch --commit-filter '
  if git diff-tree --exit-status -q "$GIT_COMMIT";
  then
    git commit-tree "$@";
  else
    skip_commit "$@";
  fi' HEAD

Of course, you could also mention that you could use

	git log --cherry-pick -p --pretty=format: ..<branch>@{1}

to verify that all skipped commits had empty diffs.  That one is also 
totally untested.

Ciao,
Dscho

Re: [PATCH] git-filter-branch: Add an example on how to remove empty commits

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1675 bytes --]

On Thu, Oct 30, 2008 at 12:33:53AM +0000, Petr Baudis wrote:
> From: Sverre Rabbelier <srabbelier@gmail.com>
> 
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> ---
>  Documentation/git-filter-branch.txt |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
> index fed6de6..2565244 100644
> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -246,6 +246,21 @@ git filter-branch --commit-filter '
>  	fi' HEAD
>  ------------------------------------------------------------------------------
>  
> +To remove commits that are empty (do not introduce any change):
> +
> +------------------------------------------------------------------------------
> +git rev-list HEAD | while read c; do [ -n "$(git diff-tree --root $c)" ] || echo $c; done > revs
> +
> +git filter-branch --commit-filter '
> +  if grep -q "$GIT_COMMIT" '"$(pwd)/"revs';
> +  then
> +    skip_commit "$@";
> +  else
> +    git commit-tree "$@";
> +  fi' HEAD
> +
> +------------------------------------------------------------------------------

Why not add an option to filter-branch that removes a commit if it's
empty ? It's quite useful, it helps the user concentrating on just
keeping what matches *his* criteriums, and not caring about the minor
details of cleansing the result.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] git-filter-branch: Add an example on how to remove empty commits

[Deskin Miller]

On Thu, Oct 30, 2008 at 02:26:23PM +0100, Pierre Habouzit wrote:
> Why not add an option to filter-branch that removes a commit if it's
> empty ? It's quite useful, it helps the user concentrating on just
> keeping what matches *his* criteriums, and not caring about the minor
> details of cleansing the result.

I've thought this would be useful at times myself.  One potential complication,
however, is that the history could come from a SVN repository via git-svn, in
which case it's possible that empty commits exist due to an incomplete mapping
of SVN's changes, e.g. SVN property changes will get their own revision, even
if the file content does not change.

Therefore, if one were to write a patch such as Pierre suggests, I'd strongly
suggest checking the commit message first for any git-svn-id: line, and either
refusing to work without some --force option from the user, or giving a strong
warning to the user that their git-svn setup may not work properly any more,
and clear instructions on how to recover those refs, or update the svn-related
metadata.

On further thought, automatically updating the svn metadata might be useful to
add as an option to filter-branch regardless; I'll think about that some
myself, any thoughts from others?

My $0.02,
Deskin Miller

Re: [PATCH] git-filter-branch: Add an example on how to remove empty commits

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

On Thu, Oct 30, 2008 at 03:06:18PM +0000, Deskin Miller wrote:
> On Thu, Oct 30, 2008 at 02:26:23PM +0100, Pierre Habouzit wrote:
> > Why not add an option to filter-branch that removes a commit if it's
> > empty ? It's quite useful, it helps the user concentrating on just
> > keeping what matches *his* criteriums, and not caring about the minor
> > details of cleansing the result.
> 
> I've thought this would be useful at times myself.  One potential complication,
> however, is that the history could come from a SVN repository via git-svn, in
> which case it's possible that empty commits exist due to an incomplete mapping
> of SVN's changes, e.g. SVN property changes will get their own revision, even
> if the file content does not change.

Well, if you want to migrate your git-svn repository to something else,
it doesn't makes sense to add this limitation. I'd rather see this
"problem" advertized in the manual page, rather than a limitation added.

Note that using git filter-branch on a git-svn repository and still
expecting it to work with git-svn is IMHO wrong in so many ways that we
should not really try that hard to prevent the user doing something
stupid anyways.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

filter-branch enhancements

[Pierre Habouzit]

The first patch is about making git-filter-branch use parse options, and
is independant, but the current state made my eyes bleed, and I wanted
to work on the second patch ;)

The second patch is more a request for comments, and if people think
it's useful, I'll try to check it actually works, and write some tests
for it.

[PATCH] make git-filter-branch use parse-options.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-filter-branch.sh |   93 +++++++++++++++++++++-----------------------------
 1 files changed, 39 insertions(+), 54 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 81392ad..8af3126 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -87,14 +87,26 @@ set_ident () {
 	echo "case \"\$GIT_${uid}_NAME\" in \"\") GIT_${uid}_NAME=\"\${GIT_${uid}_EMAIL%%@*}\" && export GIT_${uid}_NAME;; esac"
 }
 
-USAGE="[--env-filter <command>] [--tree-filter <command>] \
-[--index-filter <command>] [--parent-filter <command>] \
-[--msg-filter <command>] [--commit-filter <command>] \
-[--tag-name-filter <command>] [--subdirectory-filter <directory>] \
-[--original <namespace>] [-d <directory>] [-f | --force] \
-[<rev-list options>...]"
-
-OPTIONS_SPEC=
+OPTIONS_KEEPDASHDASH=
+OPTIONS_SPEC="\
+git filter-branch [options] [<rev-list options>...]
+--
+d=                      temporary path to use for rewriting
+f,force                 force filter-branch to run
+subdirectory-filter=    only look at the history touching that specific subdirectory
+original=               namespace where the original commits will be stored (default: refs/original)
+
+ Filters that you can run:
+
+env-filter=             environment filter to run
+tree-filter=            tree rewriting filter to run
+index-filter=           index rewriting filter to run
+parent-filter=          parent rewriting filter to run
+msg-filter=             commit message rewriting filter to run
+commit-filter=          commit rewriting filter to run
+tag-name-filter=        tag name rewriting filter to run
+"
+
 . git-sh-setup
 
 if [ "$(is_bare_repository)" = false ]; then
@@ -117,63 +129,36 @@ force=
 while :
 do
 	case "$1" in
-	--)
-		shift
-		break
-		;;
 	--force|-f)
-		shift
-		force=t
-		continue
-		;;
-	-*)
-		;;
-	*)
-		break;
-	esac
-
-	# all switches take one argument
-	ARG="$1"
-	case "$#" in 1) usage ;; esac
-	shift
-	OPTARG="$1"
-	shift
-
-	case "$ARG" in
+		force=t;;
 	-d)
-		tempdir="$OPTARG"
-		;;
+		tempdir="$2"; shift;;
+	--subdirectory-filter)
+		filter_subdir="$2"; shift;;
+	--original)
+		orig_namespace=$(expr "$2/" : '\(.*[^/]\)/*$')/
+		shift;;
+
 	--env-filter)
-		filter_env="$OPTARG"
-		;;
+		filter_env="$2"; shift;;
 	--tree-filter)
-		filter_tree="$OPTARG"
-		;;
+		filter_tree="$2"; shift;;
 	--index-filter)
-		filter_index="$OPTARG"
-		;;
+		filter_index="$2"; shift;;
 	--parent-filter)
-		filter_parent="$OPTARG"
-		;;
+		filter_parent="$2"; shift;;
 	--msg-filter)
-		filter_msg="$OPTARG"
-		;;
+		filter_msg="$2"; shift;;
 	--commit-filter)
-		filter_commit="$functions; $OPTARG"
-		;;
+		filter_commit="$functions; $2"; shift;;
 	--tag-name-filter)
-		filter_tag_name="$OPTARG"
-		;;
-	--subdirectory-filter)
-		filter_subdir="$OPTARG"
-		;;
-	--original)
-		orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
-		;;
+		filter_tag_name="$2"; shift;;
+	--)
+		shift; break;;
 	*)
-		usage
-		;;
+		usage;;
 	esac
+	shift
 done
 
 case "$force" in
-- 
1.6.0.3.758.gc29b0

[Proof of concept PATCH] implement --prune-empty switch for filter-branch

[Pierre Habouzit]

This is not a real patch (lacks a test at least) and is absolutely not
tested, though should basically work as expected.

This is only meant as a basis for discussion.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 Documentation/git-filter-branch.txt |   14 ++++++++++++++
 git-filter-branch.sh                |   26 +++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index fed6de6..451950b 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -122,6 +122,10 @@ You can use the 'map' convenience function in this filter, and other
 convenience functions, too.  For example, calling 'skip_commit "$@"'
 will leave out the current commit (but not its changes! If you want
 that, use 'git-rebase' instead).
++
+You can also use the 'git_commit_non_empty_tree "$@"' instead of
+'git commit-tree "$@"' if you don't wish to keep commits with a single parent
+and that makes no change to the tree.
 
 --tag-name-filter <command>::
 	This is the filter for rewriting tag names. When passed,
@@ -151,6 +155,16 @@ to other tags will be rewritten to point to the underlying commit.
 	The result will contain that directory (and only that) as its
 	project root.
 
+--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.
+
 --original <namespace>::
 	Use this option to set the namespace where the original commits
 	will be stored. The default value is 'refs/original'.
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 8af3126..9c83402 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -49,6 +49,15 @@ die()
 	echo "$*" >&2
 	exit 1
 }
+
+git_commit_non_empty_tree()
+{
+	if test $# = 3 && test "$1" = $(git rev-parse "$3^{tree}"); then
+		map "$3"
+	else
+		git commit-tree "$@"
+	fi
+}
 EOF
 )
 
@@ -95,6 +104,7 @@ d=                      temporary path to use for rewriting
 f,force                 force filter-branch to run
 subdirectory-filter=    only look at the history touching that specific subdirectory
 original=               namespace where the original commits will be stored (default: refs/original)
+prune-empty             use this if you want to automatically prune empty commits
 
  Filters that you can run:
 
@@ -121,11 +131,12 @@ filter_tree=
 filter_index=
 filter_parent=
 filter_msg=cat
-filter_commit='git commit-tree "$@"'
+filter_commit=
 filter_tag_name=
 filter_subdir=
 orig_namespace=refs/original/
 force=
+prune_empty=
 while :
 do
 	case "$1" in
@@ -138,6 +149,8 @@ do
 	--original)
 		orig_namespace=$(expr "$2/" : '\(.*[^/]\)/*$')/
 		shift;;
+	--prune-empty)
+		prune_empty=t;;
 
 	--env-filter)
 		filter_env="$2"; shift;;
@@ -161,6 +174,17 @@ do
 	shift
 done
 
+case "$prune_empty,$filter_commit" in
+',')
+	filter_commit='git commit-tree "$@"';;
+'t,')
+	filter_commit='git_commit_non_empty_tree "$@"';;
+','*)
+	;;
+*)
+	die "Cannot set --prune-empty and --filter-commit at the same time"
+esac
+
 case "$force" in
 t)
 	rm -rf "$tempdir"
-- 
1.6.0.3.758.gc29b0

Re: [PATCH] make git-filter-branch use parse-options.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

Okay this patch prevents us to use rev-list options in fact, which is
broken I guess, since it prevents --not and a few similar arguments that
can make sense. Please do not apply, I'll try to rework the next patch
though since nobody complained, it's that it looks fine :p

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Pierre Habouzit]

git_commit_non_empty_tree is added to the functions that can be run from
commit filters. Its effect is to commit only commits actually touching the
tree and that are not merge points either.

The option --prune-empty is added. It defaults the commit-filter to
'git_commit_non_empty_tree "$@"', and can be used with any other
combination of filters, except --commit-hook that must used
'git_commit_non_empty_tree "$@"' where one puts 'git commit-tree "$@"'
usually to achieve the same result.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

  Okay, this one is meant for inclusion, and has a test case.

  Note that this prune-empty option is not 100% perfect, for example, it won't
  prune an empty base (an empty commit whose parent is 0000000) since it has
  no parent, or won't reduce histories this way:

     t0---t1---t1            t0---t1
              /     ---->        /
             t2                t2

  But I don't think it's especially bad since such trees happen when one does
  'git merge -s ours' all the time, and I don't think git-filter-branch should
  erase those (I mean some people may want to, but then they can figure out
  how to do that by themselves, I don't think it's a common wish).

  The goal of --prune-empty is rather meant to be used e.g. when you remove a
  file from your history, and want to get rid of commits that only touched it,
  or that you want to extract the history of a given subdirectory only and
  remove commits that didn't touch that directory.

 Documentation/git-filter-branch.txt |   14 ++++++++++++++
 git-filter-branch.sh                |   29 ++++++++++++++++++++++++++++-
 t/t7003-filter-branch.sh            |    8 ++++++++
 3 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index fed6de6..451950b 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -122,6 +122,10 @@ You can use the 'map' convenience function in this filter, and other
 convenience functions, too.  For example, calling 'skip_commit "$@"'
 will leave out the current commit (but not its changes! If you want
 that, use 'git-rebase' instead).
++
+You can also use the 'git_commit_non_empty_tree "$@"' instead of
+'git commit-tree "$@"' if you don't wish to keep commits with a single parent
+and that makes no change to the tree.
 
 --tag-name-filter <command>::
 	This is the filter for rewriting tag names. When passed,
@@ -151,6 +155,16 @@ to other tags will be rewritten to point to the underlying commit.
 	The result will contain that directory (and only that) as its
 	project root.
 
+--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.
+
 --original <namespace>::
 	Use this option to set the namespace where the original commits
 	will be stored. The default value is 'refs/original'.
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 81392ad..4827934 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -40,6 +40,16 @@ skip_commit()
 	done;
 }
 
+# if you run 'git_commit_non_empty_tree "$@"' in a commit filter,
+# it will skip commits that leave the tree untouched, commit the other.
+git_commit_non_empty_tree()
+{
+	if test $# = 3 && test "$1" = $(git rev-parse "$3^{tree}"); then
+		map "$3"
+	else
+		git commit-tree "$@"
+	fi
+}
 # override die(): this version puts in an extra line break, so that
 # the progress is still visible
 
@@ -109,11 +119,12 @@ filter_tree=
 filter_index=
 filter_parent=
 filter_msg=cat
-filter_commit='git commit-tree "$@"'
+filter_commit=
 filter_tag_name=
 filter_subdir=
 orig_namespace=refs/original/
 force=
+prune_empty=
 while :
 do
 	case "$1" in
@@ -126,6 +137,11 @@ do
 		force=t
 		continue
 		;;
+	--prune-empty)
+		shift
+		prune_empty=t
+		continue
+		;;
 	-*)
 		;;
 	*)
@@ -176,6 +192,17 @@ do
 	esac
 done
 
+case "$prune_empty,$filter_commit" in
+',')
+	filter_commit='git commit-tree "$@"';;
+'t,')
+	filter_commit="$functions;"' git_commit_non_empty_tree "$@"';;
+','*)
+	;;
+*)
+	die "Cannot set --prune-empty and --filter-commit at the same time"
+esac
+
 case "$force" in
 t)
 	rm -rf "$tempdir"
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index b0a9d7d..352b56b 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -262,4 +262,12 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '
 	test_cmp expect actual
 '
 
+test_expect_success 'Prune empty commits' '
+	make_commit to_remove &&
+	(git rev-list HEAD | grep -v $(git rev-parse HEAD)) > expect &&
+	git filter-branch -f --index-filter "git update-index --remove to_remove" --prune-empty HEAD &&
+	git rev-list HEAD > actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.6.0.3.757.gf86e6.dirty

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Johannes Schindelin]

Hi,

On Fri, 31 Oct 2008, Pierre Habouzit wrote:

> git_commit_non_empty_tree is added to the functions that can be run from 
> commit filters. Its effect is to commit only commits actually touching 
> the tree and that are not merge points either.
> 
> The option --prune-empty is added. It defaults the commit-filter to
> 'git_commit_non_empty_tree "$@"', and can be used with any other
> combination of filters, except --commit-hook that must used
> 'git_commit_non_empty_tree "$@"' where one puts 'git commit-tree "$@"'
> usually to achieve the same result.

I think that the example Sverre posted is better.  It might be a bit more 
to write out, but at least people can adapt it to their needs (as opposed 
to only skip "empty" commits).

However, I would _love_ to see your tests being merged with Sverre's patch 
(of course, the tests should use the described procedure, then).

Ciao,
Dscho

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]

On Fri, Oct 31, 2008 at 10:36:58PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 31 Oct 2008, Pierre Habouzit wrote:
> 
> > git_commit_non_empty_tree is added to the functions that can be run from 
> > commit filters. Its effect is to commit only commits actually touching 
> > the tree and that are not merge points either.
> > 
> > The option --prune-empty is added. It defaults the commit-filter to
> > 'git_commit_non_empty_tree "$@"', and can be used with any other
> > combination of filters, except --commit-hook that must used
> > 'git_commit_non_empty_tree "$@"' where one puts 'git commit-tree "$@"'
> > usually to achieve the same result.
> 
> I think that the example Sverre posted is better.  It might be a bit more 
> to write out, but at least people can adapt it to their needs (as opposed 
> to only skip "empty" commits).
> 
> However, I would _love_ to see your tests being merged with Sverre's patch 
> (of course, the tests should use the described procedure, then).

Well Sverre's example is probably the most efficient way to do the task,
though the thing is right now, what one wants is rarely to "just" skip
empty commits, but to do some modifications that does not leave empty
commits.

IOW not doing a modification _then_ a new one, but both at the same
time. Given how slow filter-branch can be, it's better to do one
transformation instead of two.


Note that I don't think we should apply only my patch and not Sverre's,
his proposal just made me think that this was an itch I wanted to
scratch for a long time, and both probably are complementary.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

> +case "$prune_empty,$filter_commit" in
> +',')
> +	filter_commit='git commit-tree "$@"';;
> +'t,')
> +	filter_commit="$functions;"' git_commit_non_empty_tree "$@"';;
> +','*)
> +	;;
> +*)
> +	die "Cannot set --prune-empty and --filter-commit at the same time"
> +esac

This is only style issue, but I find the above extremely difficult to
read.  If it were either:

	case ... in
        ,) do "neither set case" ;;
        t,) do "prune but not filter case" ;;
        *) do "both set case" ;;
        esac

or (rather amateurish but conveys what it wants to do more clearly):
        
	case ... in
        '','') do "neither set case" ;;
        t,'') do "prune but not filter case" ;;
        t,t) do "both set case" ;;
        esac

I wouldn't have to wonder which sq pairs with which one.

> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index b0a9d7d..352b56b 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -262,4 +262,12 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'Prune empty commits' '
> +	make_commit to_remove &&
> +	(git rev-list HEAD | grep -v $(git rev-parse HEAD)) > expect &&

I am not sure what this one is doing.

 - Isn't this the same as "git rev-list HEAD^"?
 - Do you need a subshell?

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]

On Mon, Nov 03, 2008 at 04:58:44AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > +case "$prune_empty,$filter_commit" in
> > +',')
> > +	filter_commit='git commit-tree "$@"';;
> > +'t,')
> > +	filter_commit="$functions;"' git_commit_non_empty_tree "$@"';;
> > +','*)
> > +	;;
> > +*)
> > +	die "Cannot set --prune-empty and --filter-commit at the same time"
> > +esac
> 
> This is only style issue, but I find the above extremely difficult to
> read.  If it were either:
> 
> 	case ... in
>         ,) do "neither set case" ;;
>         t,) do "prune but not filter case" ;;
>         *) do "both set case" ;;
>         esac
> 
> or (rather amateurish but conveys what it wants to do more clearly):
>         
> 	case ... in
>         '','') do "neither set case" ;;
>         t,'') do "prune but not filter case" ;;
>         t,t) do "both set case" ;;
>         esac
> 
> I wouldn't have to wonder which sq pairs with which one.

agreed.

> > diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> > index b0a9d7d..352b56b 100755
> > --- a/t/t7003-filter-branch.sh
> > +++ b/t/t7003-filter-branch.sh
> > @@ -262,4 +262,12 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '
> >  	test_cmp expect actual
> >  '
> >  
> > +test_expect_success 'Prune empty commits' '
> > +	make_commit to_remove &&
> > +	(git rev-list HEAD | grep -v $(git rev-parse HEAD)) > expect &&
> 
> I am not sure what this one is doing.
> 
>  - Isn't this the same as "git rev-list HEAD^"?
>  - Do you need a subshell?

The filter-branch is supposed to prune the last commit done (current
HEAD) from the revision list. So I build the rev-list we're supposed to
have in the end, and remove the matching ref from it. I don't see how to
avoid the subshell though, but if someone knows better please do :)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 6210 bytes --]

On Mon, Nov 03, 2008 at 09:27:29AM +0000, Pierre Habouzit wrote:
> On Mon, Nov 03, 2008 at 04:58:44AM +0000, Junio C Hamano wrote:
> > > diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> > > index b0a9d7d..352b56b 100755
> > > --- a/t/t7003-filter-branch.sh
> > > +++ b/t/t7003-filter-branch.sh
> > > @@ -262,4 +262,12 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '
> > >  	test_cmp expect actual
> > >  '
> > >  
> > > +test_expect_success 'Prune empty commits' '
> > > +	make_commit to_remove &&
> > > +	(git rev-list HEAD | grep -v $(git rev-parse HEAD)) > expect &&
> > 
> > I am not sure what this one is doing.
> > 
> >  - Isn't this the same as "git rev-list HEAD^"?
> >  - Do you need a subshell?
> 
> The filter-branch is supposed to prune the last commit done (current
> HEAD) from the revision list. So I build the rev-list we're supposed to
> have in the end, and remove the matching ref from it. I don't see how to
> avoid the subshell though, but if someone knows better please do :)

Actually one can write the test this way:

test_expect_success 'Prune empty commits' '
       git rev-list HEAD > expect &&
       make_commit to_remove &&
       git filter-branch -f --index-filter "git update-index --remove to_remove" --prune-empty HEAD &&
       git rev-list HEAD > actual &&
       test_cmp expect actual
'

Which basically:
  - remembers the current list of revisions,
  - makes a commit,
  - runs an index-filter that makes that last commit void,
  - checks the last commit has been removed.

below is the updated patch with your comment and this fix
---8<---
From: Pierre Habouzit <madcoder@debian.org>
Date: Fri, 31 Oct 2008 10:12:21 +0100
Subject: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

git_commit_non_empty_tree is added to the functions that can be run from
commit filters. Its effect is to commit only commits actually touching the
tree and that are not merge points either.

The option --prune-empty is added. It defaults the commit-filter to
'git_commit_non_empty_tree "$@"', and can be used with any other
combination of filters, except --commit-hook that must used
'git_commit_non_empty_tree "$@"' where one puts 'git commit-tree "$@"'
usually to achieve the same result.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 Documentation/git-filter-branch.txt |   14 ++++++++++++++
 git-filter-branch.sh                |   29 ++++++++++++++++++++++++++++-
 t/t7003-filter-branch.sh            |    8 ++++++++
 3 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index fed6de6..451950b 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -122,6 +122,10 @@ You can use the 'map' convenience function in this filter, and other
 convenience functions, too.  For example, calling 'skip_commit "$@"'
 will leave out the current commit (but not its changes! If you want
 that, use 'git-rebase' instead).
++
+You can also use the 'git_commit_non_empty_tree "$@"' instead of
+'git commit-tree "$@"' if you don't wish to keep commits with a single parent
+and that makes no change to the tree.
 
 --tag-name-filter <command>::
 	This is the filter for rewriting tag names. When passed,
@@ -151,6 +155,16 @@ to other tags will be rewritten to point to the underlying commit.
 	The result will contain that directory (and only that) as its
 	project root.
 
+--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.
+
 --original <namespace>::
 	Use this option to set the namespace where the original commits
 	will be stored. The default value is 'refs/original'.
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 81392ad..331724d 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -40,6 +40,16 @@ skip_commit()
 	done;
 }
 
+# if you run 'git_commit_non_empty_tree "$@"' in a commit filter,
+# it will skip commits that leave the tree untouched, commit the other.
+git_commit_non_empty_tree()
+{
+	if test $# = 3 && test "$1" = $(git rev-parse "$3^{tree}"); then
+		map "$3"
+	else
+		git commit-tree "$@"
+	fi
+}
 # override die(): this version puts in an extra line break, so that
 # the progress is still visible
 
@@ -109,11 +119,12 @@ filter_tree=
 filter_index=
 filter_parent=
 filter_msg=cat
-filter_commit='git commit-tree "$@"'
+filter_commit=
 filter_tag_name=
 filter_subdir=
 orig_namespace=refs/original/
 force=
+prune_empty=
 while :
 do
 	case "$1" in
@@ -126,6 +137,11 @@ do
 		force=t
 		continue
 		;;
+	--prune-empty)
+		shift
+		prune_empty=t
+		continue
+		;;
 	-*)
 		;;
 	*)
@@ -176,6 +192,17 @@ do
 	esac
 done
 
+case "$prune_empty,$filter_commit" in
+,)
+	filter_commit='git commit-tree "$@"';;
+t,)
+	filter_commit="$functions;"' git_commit_non_empty_tree "$@"';;
+,*)
+	;;
+*)
+	die "Cannot set --prune-empty and --filter-commit at the same time"
+esac
+
 case "$force" in
 t)
 	rm -rf "$tempdir"
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index b0a9d7d..8537bf9 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -262,4 +262,12 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '
 	test_cmp expect actual
 '
 
+test_expect_success 'Prune empty commits' '
+	git rev-list HEAD > expect &&
+	make_commit to_remove &&
+	git filter-branch -f --index-filter "git update-index --remove to_remove" --prune-empty HEAD &&
+	git rev-list HEAD > actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.6.0.3.795.g892be


[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Jay Soffian]

On Mon, Nov 3, 2008 at 10:18 AM, Pierre Habouzit <madcoder@debian.org> wrote:
> On Mon, Nov 03, 2008 at 09:27:29AM +0000, Pierre Habouzit wrote:
>> On Mon, Nov 03, 2008 at 04:58:44AM +0000, Junio C Hamano wrote:

Bump, http://thread.gmane.org/gmane.comp.version-control.git/99440/

(I'd like to see this included. Having a bunch of empty commits after
using filter-branch to remove unwanted files from history is, er,
sub-optimal, so seems like it might even be default behavior?)

j.

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

On Fri, Jan 09, 2009 at 07:29:15PM +0000, Jay Soffian wrote:
> On Mon, Nov 3, 2008 at 10:18 AM, Pierre Habouzit <madcoder@debian.org> wrote:
> > On Mon, Nov 03, 2008 at 09:27:29AM +0000, Pierre Habouzit wrote:
> >> On Mon, Nov 03, 2008 at 04:58:44AM +0000, Junio C Hamano wrote:
> 
> Bump, http://thread.gmane.org/gmane.comp.version-control.git/99440/
> 
> (I'd like to see this included. Having a bunch of empty commits after
> using filter-branch to remove unwanted files from history is, er,
> sub-optimal, so seems like it might even be default behavior?)

Yeah I have that in my own git tree, and I meant to ask if something had
to be fixed for it to be accepted for some time, but always forget about
it.

Junio, do you think this could be accepted, or does it need some work ?

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Johannes Schindelin]

Hi,

On Sun, 11 Jan 2009, Pierre Habouzit wrote:

> On Fri, Jan 09, 2009 at 07:29:15PM +0000, Jay Soffian wrote:
> > On Mon, Nov 3, 2008 at 10:18 AM, Pierre Habouzit <madcoder@debian.org> wrote:
> > > On Mon, Nov 03, 2008 at 09:27:29AM +0000, Pierre Habouzit wrote:
> > >> On Mon, Nov 03, 2008 at 04:58:44AM +0000, Junio C Hamano wrote:
> > 
> > Bump, http://thread.gmane.org/gmane.comp.version-control.git/99440/
> > 
> > (I'd like to see this included. Having a bunch of empty commits after
> > using filter-branch to remove unwanted files from history is, er,
> > sub-optimal, so seems like it might even be default behavior?)
> 
> Yeah I have that in my own git tree, and I meant to ask if something had 
> to be fixed for it to be accepted for some time, but always forget about 
> it.
> 
> Junio, do you think this could be accepted, or does it need some work ?

AFAICT Junio had some style issues which were not addressed.

And I suggested to merge the tests with Sverre's patch.  That suggestion 
also went unaddressed.

Ciao,
Dscho

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

On Sun, Jan 11, 2009 at 01:35:15PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Sun, 11 Jan 2009, Pierre Habouzit wrote:
> 
> > On Fri, Jan 09, 2009 at 07:29:15PM +0000, Jay Soffian wrote:
> > > On Mon, Nov 3, 2008 at 10:18 AM, Pierre Habouzit <madcoder@debian.org> wrote:
> > > > On Mon, Nov 03, 2008 at 09:27:29AM +0000, Pierre Habouzit wrote:
> > > >> On Mon, Nov 03, 2008 at 04:58:44AM +0000, Junio C Hamano wrote:
> > > 
> > > Bump, http://thread.gmane.org/gmane.comp.version-control.git/99440/
> > > 
> > > (I'd like to see this included. Having a bunch of empty commits after
> > > using filter-branch to remove unwanted files from history is, er,
> > > sub-optimal, so seems like it might even be default behavior?)
> > 
> > Yeah I have that in my own git tree, and I meant to ask if something had 
> > to be fixed for it to be accepted for some time, but always forget about 
> > it.
> > 
> > Junio, do you think this could be accepted, or does it need some work ?
> 
> AFAICT Junio had some style issues which were not addressed.

Huh, I did in a latter resend in that very thread.

> And I suggested to merge the tests with Sverre's patch.  That suggestion 
> also went unaddressed.

I can't find any mails from Sverre in the same thread, but maybe I'm not
searching in the proper place...

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Sverre Rabbelier]

On Sun, Jan 11, 2009 at 15:27, Pierre Habouzit <madcoder@debian.org> wrote:
>> And I suggested to merge the tests with Sverre's patch.  That suggestion
>> also went unaddressed.
>
> I can't find any mails from Sverre in the same thread, but maybe I'm not
> searching in the proper place...

I think my patch is in a different thread as it was a
documentation-only patch, no?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

On Sun, Jan 11, 2009 at 02:40:04PM +0000, Sverre Rabbelier wrote:
> On Sun, Jan 11, 2009 at 15:27, Pierre Habouzit <madcoder@debian.org> wrote:
> >> And I suggested to merge the tests with Sverre's patch.  That suggestion
> >> also went unaddressed.
> >
> > I can't find any mails from Sverre in the same thread, but maybe I'm not
> > searching in the proper place...
> 
> I think my patch is in a different thread as it was a
> documentation-only patch, no?

No I've found it, it was sent by Petr which explains why I didn't find a
mail from _you_ :P

Will sent a patch reworked to have a special paragraph the way you
wanted, but using my additions instead.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Sverre Rabbelier]

On Sun, Jan 11, 2009 at 15:55, Pierre Habouzit <madcoder@debian.org> wrote:
> No I've found it, it was sent by Petr which explains why I didn't find a
> mail from _you_ :P

Ah, yes, that's right :).

> Will sent a patch reworked to have a special paragraph the way you
> wanted, but using my additions instead.

Ok, nice!

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

> On Sun, Jan 11, 2009 at 01:35:15PM +0000, Johannes Schindelin wrote:
> ...
>> And I suggested to merge the tests with Sverre's patch.  That suggestion 
>> also went unaddressed.
>
> I can't find any mails from Sverre in the same thread, but maybe I'm not
> searching in the proper place...

This is in 'master' already; if there was anything missing in your patch,
please fill in with updates.

Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Johannes Schindelin]

Hi,

On Sun, 11 Jan 2009, Junio C Hamano wrote:

> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > On Sun, Jan 11, 2009 at 01:35:15PM +0000, Johannes Schindelin wrote:
> > ...
> >> And I suggested to merge the tests with Sverre's patch.  That 
> >> suggestion also went unaddressed.
> >
> > I can't find any mails from Sverre in the same thread, but maybe I'm 
> > not searching in the proper place...
> 
> This is in 'master' already; if there was anything missing in your 
> patch, please fill in with updates.

I _think_ that I asked for test cases to use that workflow, so that we can 
be sure that what's in the man page actually works.

Ciao,
Dscho