[PATCH/RFC] filter-branch: support skipping of commits more easily

[PATCH/RFC] filter-branch: support skipping of commits more easily

[Johannes Schindelin]

When commit-filter echoes just "skip", just skip that commit by mapping 
its object name to the same (possibly rewritten) object name(s) its 
parent(s) are mapped to.

IOW, given A-B-C, if commit-filter says "skip" upon B, the rewritten 
branch will look like this: A'-C'.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Of course, if you think of "patchsets", this behaviour might
	be unexpected, since the children will still contain everything
	which was changed in the skipped revisions, and not changed in
	_them_.

 git-filter-branch.sh |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 9d61b7f..e6ed7b9 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -102,6 +102,9 @@
 #	multiple commit ids; in that case, all of them will be used
 #	as parents instead of the original commit in further commits.
 #
+#	For the common case, that this commit should be skipped, just
+#	output a single "skip".
+#
 # --tag-name-filter COMMAND:: The filter for rewriting tag names.
 #	If this filter is passed, it will be called for every tag ref
 #	that points to a rewritten object (or to a tag object which
@@ -321,7 +324,7 @@ test $commits -eq 0 && die "Found nothing to rewrite"
 i=0
 while read commit; do
 	i=$(($i+1))
-	printf "$commit ($i/$commits) "
+	printf "\rRewriting commits... ($i/$commits)"
 
 	git-read-tree -i -m $commit
 
@@ -358,8 +361,14 @@ while read commit; do
 
 	sed -e '1,/^$/d' <../commit | \
 		eval "$filter_msg" | \
-		sh -c "$filter_commit" git-commit-tree $(git-write-tree) $parentstr | \
-		tee ../map/$commit
+		sh -c "$filter_commit" \
+			git-commit-tree $(git-write-tree) $parentstr \
+		> ../map/$commit
+
+	test skip = "$(cat ../map/$commit)" &&
+		for parent in $(get_parents $commit); do
+			map "$parent"
+		done > ../map/$commit
 done <../revs
 
 src_head=$(tail -n 1 ../revs)
-- 
1.5.2.1.2689.gaf768-dirty

Re: [PATCH/RFC] filter-branch: support skipping of commits more easily

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When commit-filter echoes just "skip", just skip that commit by mapping 
> its object name to the same (possibly rewritten) object name(s) its 
> parent(s) are mapped to.
>
> IOW, given A-B-C, if commit-filter says "skip" upon B, the rewritten 
> branch will look like this: A'-C'.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	Of course, if you think of "patchsets", this behaviour might
> 	be unexpected, since the children will still contain everything
> 	which was changed in the skipped revisions, and not changed in
> 	_them_.

I think that is fine; in effect, by saying "skip" B, you are
squashing B-C into C'.

Does this mean that, given

          C---D---E
         /   /
	A---B

and if commit-filter says "skip" on D, the written history would
look like this?

          C'------E'
         /       /
	A'--B'--'

The new commit E' would become an evil merge that has difference
between D and E in the original history?

I am not objecting; just trying to get a mental picture.

Re: [PATCH/RFC] filter-branch: support skipping of commits more easily

[linux]

> I think that is fine; in effect, by saying "skip" B, you are
> squashing B-C into C'.
> 
> Does this mean that, given
> 
>           C---D---E
>          /   /
>         A---B
> 
> and if commit-filter says "skip" on D, the written history would
> look like this?
> 
>           C'------E'
>          /       /
>         A'--B'--'
> 
> The new commit E' would become an evil merge that has difference
> between D and E in the original history?
> 
> I am not objecting; just trying to get a mental picture.

I think, for compatibility with the existing git path limiter,
it should delete D from the history only if:
1) Told to skip D, and
2) Told to skip B or C (or both).

So you could get A--B--E' or A--C--E' or even A--E', but D would only
be deleted if it wasn't needed as a merge marker.

That's probably a little more complex to implement, but it feels like
The Right Thing.

Re: [PATCH/RFC] filter-branch: support skipping of commits more easily

[Johannes Schindelin]

Hi,

On Thu, 7 Jun 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When commit-filter echoes just "skip", just skip that commit by mapping 
> > its object name to the same (possibly rewritten) object name(s) its 
> > parent(s) are mapped to.
> >
> > IOW, given A-B-C, if commit-filter says "skip" upon B, the rewritten 
> > branch will look like this: A'-C'.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >
> > 	Of course, if you think of "patchsets", this behaviour might
> > 	be unexpected, since the children will still contain everything
> > 	which was changed in the skipped revisions, and not changed in
> > 	_them_.
> 
> I think that is fine; in effect, by saying "skip" B, you are
> squashing B-C into C'.
> 
> Does this mean that, given
> 
>           C---D---E
>          /   /
> 	A---B
> 
> and if commit-filter says "skip" on D, the written history would
> look like this?
> 
>           C'------E'
>          /       /
> 	A'--B'--'
> 
> The new commit E' would become an evil merge that has difference
> between D and E in the original history?
> 
> I am not objecting; just trying to get a mental picture.

Yeah, but you called it "squash" instead of "skip". So, maybe it should 
accept "squash" to do that operation instead?

Ciao,
Dscho

Re: [PATCH/RFC] filter-branch: support skipping of commits more easily

[Johannes Schindelin]

Hi,

On Fri, 7 Jun 2007, linux@horizon.com wrote:

> > I think that is fine; in effect, by saying "skip" B, you are
> > squashing B-C into C'.
> > 
> > Does this mean that, given
> > 
> >           C---D---E
> >          /   /
> >         A---B
> > 
> > and if commit-filter says "skip" on D, the written history would
> > look like this?
> > 
> >           C'------E'
> >          /       /
> >         A'--B'--'
> > 
> > The new commit E' would become an evil merge that has difference
> > between D and E in the original history?
> > 
> > I am not objecting; just trying to get a mental picture.
> 
> I think, for compatibility with the existing git path limiter,
> it should delete D from the history only if:
> 1) Told to skip D, and
> 2) Told to skip B or C (or both).
> 
> So you could get A--B--E' or A--C--E' or even A--E', but D would only
> be deleted if it wasn't needed as a merge marker.
> 
> That's probably a little more complex to implement, but it feels like
> The Right Thing.

... but if that script should do that, the name "filter"-branch was a 
misnomer.

It filters the _branch_. In the sense that a branch is one or more perls 
of commits, uniting in the tip of that branch.

If you want to skip a commit, that is fine. But a commit is _not_ a patch, 
no sir. It is a revision.

The fact that we actually are able to extract nice patches from any patch 
series, does not mean that the revisions are actually only deltas with 
regard to the previous commit. To the contrary: we actually allow -- and 
encourage -- git-diff between different revisions, be they on the same 
branch or not. That alone should tell everybody that a revision is a 
revision is a revision, and _not_ a delta.

So, when you filter commits, you should not expect a certain _patch_ to be 
skipped when you say "skip" (or maybe "squash", which I actually like 
better, because it is as unambiguous as you get it), but a _commit_ (AKA 
revision) to be skipped.

Ciao,
Dscho

Re: [PATCH/RFC] filter-branch: support skipping of commits more easily

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > IOW, given A-B-C, if commit-filter says "skip" upon B, the rewritten 
>> > branch will look like this: A'-C'.
>> >
>> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> > ---
>> >
>> > 	Of course, if you think of "patchsets", this behaviour might
>> > 	be unexpected, since the children will still contain everything
>> > 	which was changed in the skipped revisions, and not changed in
>> > 	_them_.
>> 
>> I think that is fine; in effect, by saying "skip" B, you are
>> squashing B-C into C'.
>> ... 
>> I am not objecting; just trying to get a mental picture.
>
> Yeah, but you called it "squash" instead of "skip". So, maybe it should 
> accept "squash" to do that operation instead?

Skip is logically correct, but this does sound like squashing to
me.