git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* filter-branch fix and tests
@ 2010-01-25 13:06 Michal Sojka
  2010-01-25 13:06 ` [PATCH 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Michal Sojka @ 2010-01-25 13:06 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin

Hi,

this is resend of the patch I sent two weeks ago. I've extended the commit
message to address Johannes' question and in the second patch I've added tests
for the fix.

Michal

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/2] filter-branch: Fix to allow replacing submodules with another content
  2010-01-25 13:06 filter-branch fix and tests Michal Sojka
@ 2010-01-25 13:06 ` Michal Sojka
  2010-01-25 13:06 ` [PATCH 2/2] filter-branch: Add tests for submodules Michal Sojka
       [not found] ` <alpine.DEB.1.00.1001261939420.4641@intel-tinevez-2-302>
  2 siblings, 0 replies; 15+ messages in thread
From: Michal Sojka @ 2010-01-25 13:06 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, Michal Sojka

When git filter-branch is used to replace a submodule with another
content, it always fails on the first commit. Consider a repository with
submod directory containing a submodule. If I want to remove the
submodule and replace it with a file, the following command fails.

git filter-branch --tree-filter 'rm -rf submod &&
				 git rm -q submod &&
				 mkdir submod &&
				 touch submod/file'

The error message is:
error: submod: is a directory - add files inside instead

The reason is that git diff-index, which generates the first part of the
list of files updated by the tree filter, emits also the removed
submodule even if it was replaced by a real directory.

Adding --ignored-submodules solves the problem for me and
tests in t7003-filter-branch.sh pass correctly.

If somebody wants to replace one revision of the module with another, it
can be done with --index-filter. Using --tree-filter for this has no
sense since --tree-filter "git submodule update --init" fails with
  Clone of '/tmp/submod' into submodule path 'submod' failed
so that the revision must be replaced only in index.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 git-filter-branch.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 195b5ef..d4ac7fb 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -331,7 +331,7 @@ while read commit parents; do
 			die "tree filter failed: $filter_tree"
 
 		(
-			git diff-index -r --name-only $commit &&
+			git diff-index -r --name-only --ignore-submodules $commit && 
 			git ls-files --others
 		) > "$tempdir"/tree-state || exit
 		git update-index --add --replace --remove --stdin \
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/2] filter-branch: Add tests for submodules
  2010-01-25 13:06 filter-branch fix and tests Michal Sojka
  2010-01-25 13:06 ` [PATCH 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
@ 2010-01-25 13:06 ` Michal Sojka
       [not found] ` <alpine.DEB.1.00.1001261939420.4641@intel-tinevez-2-302>
  2 siblings, 0 replies; 15+ messages in thread
From: Michal Sojka @ 2010-01-25 13:06 UTC (permalink / raw)
  To: git; +Cc: Johannes.Schindelin, Michal Sojka

The test 'rewrite submodule with another content' passes only with the
previous patch.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 t/t7003-filter-branch.sh |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 9503875..39d4153 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -306,4 +306,30 @@ test_expect_success '--remap-to-ancestor with filename filters' '
 	test $orig_invariant = $(git rev-parse invariant)
 '
 
+test_expect_success 'setup submodule' '
+	rm -rf * .*
+	git init &&
+	test_commit file &&
+	mkdir submod &&
+	submodurl="$PWD/submod"
+	( cd submod &&
+	  git init &&
+	  test_commit file-in-submod ) &&
+	git submodule add "$submodurl"
+	git commit -m "added submodule" &&
+	test_commit add-file &&
+	( cd submod && test_commit add-in-submodule ) &&
+	git add submod &&
+	git commit -m "changed submodule"
+'
+
+test_expect_success 'rewrite submodule with another content' '
+	git filter-branch --tree-filter "test -d submod && {
+					 rm -rf submod &&
+					 git rm -rf --quiet submod &&
+					 mkdir submod &&
+					 : > submod/file
+					 } || :"
+'
+
 test_done
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] filter-branch fix and tests
       [not found] ` <alpine.DEB.1.00.1001261939420.4641@intel-tinevez-2-302>
@ 2010-01-27 15:49   ` Michal Sojka
  2010-01-27 16:18     ` Johannes Sixt
  0 siblings, 1 reply; 15+ messages in thread
From: Michal Sojka @ 2010-01-27 15:49 UTC (permalink / raw)
  To: Johannes Schindelin, git

Hi Johannes,

On Tuesday 26 of January 2010 19:40:47 Johannes Schindelin wrote:
> On Mon, 25 Jan 2010, Michal Sojka wrote:
> > this is resend of the patch I sent two weeks ago. I've extended the
> > commit message to address Johannes' question and in the second patch
> > I've added tests for the fix.
> 
> From a quick look, it does not seem that I will be happy with your "fix";
> you changed the commit message, but did not address the issue.
> 
> Unfortunately, I do not have time to review anything that takes longer
> than 30 minutes in total, and today not even that, so that will have to
> wait.

Thanks for your reply. I do not want to steal your time, but I may not
understand what is the issue. I extended my previous patch with tests
(see bellow) so that all the use cases which might be IMHO affected by
my "fix" are covered. If you can think of another use case let me
know.

The fastest way to test this patch is:
GIT_SKIP_TESTS='t7003.[12]? t7003.[2-9]' ./t7003-filter-branch.sh -d

>From 849b105541ba4b5e3592de6769922b1264be0c77 Mon Sep 17 00:00:00 2001
From: Michal Sojka <sojkam1@fel.cvut.cz>
Date: Wed, 27 Jan 2010 15:57:17 +0100
Subject: [PATCH] filter-branch: Add tests for submodules

There are three important tests:
1) 'rewrite submodule with another content' passes only with the
   previous patch applied.

2) 'checkout submodule during rewrite' demonstrates that it is not
   possible to replace a submodule revision in tree-filter by checking
   the submodule out and reseting the submodule's HEAD. Fails both
   with and without the previous patch. This is because filter-branch
   sets GIT_WORKING_TREE to "." which causes clone (called from
   git-submodule) to fail.

3) 'replace submodule revision' shows that replacing submodule
   revision is possible by direct index manipulation. Succeeds both
   with and without the previous patch.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 t/t7003-filter-branch.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 9503875..a218d7a 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -306,4 +306,52 @@ test_expect_success '--remap-to-ancestor with filename filters' '
 	test $orig_invariant = $(git rev-parse invariant)
 '
 
+test_expect_success 'setup submodule' '
+	rm -rf * .*
+	git init &&
+	test_commit file &&
+	mkdir submod &&
+	submodurl="$PWD/submod"
+	( cd submod &&
+	  git init &&
+	  test_commit file-in-submod ) &&
+	git submodule add "$submodurl"
+	git commit -m "added submodule" &&
+	test_commit add-file &&
+	( cd submod && test_commit add-in-submodule ) &&
+	git add submod &&
+	git commit -m "changed submodule"
+	git branch original HEAD
+'
+
+orig_head=`git show-ref --hash HEAD`
+export orig_head
+
+test_expect_success 'rewrite submodule with another content' '
+	git filter-branch --tree-filter "test -d submod && {
+					 rm -rf submod &&
+					 git rm -rf --quiet submod &&
+					 mkdir submod &&
+					 : > submod/file
+					 } || : &&
+					 test $orig_head != `git show-ref --hash HEAD`"
+'
+
+test_expect_failure 'checkout submodule during rewrite' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "git submodule update --init &&
+	     cd submod &&
+	     git reset --hard origin/master" HEAD
+'
+
+test_expect_success 'replace submodule revision' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "git ls-files --error-unmatch -- submod > /dev/null 2>&1 &&
+	     git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submod ||
+	     : &&
+	     test $orig_head != `git show-ref --hash HEAD`"
+'
+
 test_done
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] filter-branch fix and tests
  2010-01-27 15:49   ` [PATCH] filter-branch fix and tests Michal Sojka
@ 2010-01-27 16:18     ` Johannes Sixt
  2010-01-27 23:41       ` Michal Sojka
  0 siblings, 1 reply; 15+ messages in thread
From: Johannes Sixt @ 2010-01-27 16:18 UTC (permalink / raw)
  To: Michal Sojka; +Cc: Johannes Schindelin, git

Michal Sojka schrieb:
> +orig_head=`git show-ref --hash HEAD`
> +export orig_head

You place the tree filter in double-quotes. Hence, orig_head will be
interpolated on the filter-branch command line. You don't need to export it.

> +test_expect_success 'rewrite submodule with another content' '
> +	git filter-branch --tree-filter "test -d submod && {
> +					 rm -rf submod &&
> +					 git rm -rf --quiet submod &&
> +					 mkdir submod &&
> +					 : > submod/file
> +					 } || : &&
> +					 test $orig_head != `git show-ref --hash HEAD`"

What is the purpose of the check in the last line?

As long as you have another command after the "} || : &&", you can just
write "}" instead.

> +test_expect_failure 'checkout submodule during rewrite' '
> +	git reset --hard original &&
> +	git filter-branch -f --tree-filter \
> +	    "git submodule update --init &&
> +	     cd submod &&
> +	     git reset --hard origin/master" HEAD

You must not change the directory without changing back. Use a sub-shell.

I'm not sure whether it's worth catering for this use-case anyway.
Replacing a submodule commit should really be done only in the
--index-filter. The tree that --tree-filter checks out is intended only as
a temporary scratch area. It is not intended as a full worktree. In
particular, since 'submodule update --init' changes the configuration, it
is extremly dangerous to call from a filter.

-- Hannes

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] filter-branch fix and tests
  2010-01-27 16:18     ` Johannes Sixt
@ 2010-01-27 23:41       ` Michal Sojka
  2010-01-27 23:55         ` [PATCHv3] filter-branch: Add tests for submodules Michal Sojka
  0 siblings, 1 reply; 15+ messages in thread
From: Michal Sojka @ 2010-01-27 23:41 UTC (permalink / raw)
  To: Johannes Sixt, Johannes Schindelin; +Cc: git

On Wednesday 27 of January 2010 17:18:16 Johannes Sixt wrote:
> Michal Sojka schrieb:
> > +orig_head=`git show-ref --hash HEAD`
> > +export orig_head
> 
> You place the tree filter in double-quotes. Hence, orig_head will be
> interpolated on the filter-branch command line. You don't need to export
>  it.

You're right. Fixed.
 
> > +test_expect_success 'rewrite submodule with another content' '
> > +	git filter-branch --tree-filter "test -d submod && {
> > +					 rm -rf submod &&
> > +					 git rm -rf --quiet submod &&
> > +					 mkdir submod &&
> > +					 : > submod/file
> > +					 } || : &&
> > +					 test $orig_head != `git show-ref --hash HEAD`"
> 
> What is the purpose of the check in the last line?

It should check that something was rewritten, but it was incorrectly put into 
the filed. Fixed.
 
> As long as you have another command after the "} || : &&", you can just
> write "}" instead.

OK.

> > +test_expect_failure 'checkout submodule during rewrite' '
> > +	git reset --hard original &&
> > +	git filter-branch -f --tree-filter \
> > +	    "git submodule update --init &&
> > +	     cd submod &&
> > +	     git reset --hard origin/master" HEAD
> 
> You must not change the directory without changing back. Use a sub-shell.
> 
> I'm not sure whether it's worth catering for this use-case anyway.
> Replacing a submodule commit should really be done only in the
> --index-filter. The tree that --tree-filter checks out is intended only as
> a temporary scratch area. It is not intended as a full worktree. In
> particular, since 'submodule update --init' changes the configuration, it
> is extremly dangerous to call from a filter.

I fully agree. I don't plan to put this test in the final version of the 
patch. I wrote this test because I didn't exactly know which issue has Dscho 
in mind. If it was this one, I wanted to show that this is not relevant.

I'm sending corrected version of the patch with tests.

Thanks
Michal

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv3] filter-branch: Add tests for submodules
  2010-01-27 23:41       ` Michal Sojka
@ 2010-01-27 23:55         ` Michal Sojka
  2010-01-28  0:14           ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Michal Sojka @ 2010-01-27 23:55 UTC (permalink / raw)
  To: git; +Cc: j.sixt, Johannes.Schindelin, Michal Sojka

There are three important tests:
1) 'rewrite submodule with another content' passes only with the
   previous patch applied.

2) 'checkout submodule during rewrite' demonstrates that it is not
   possible to replace a submodule revision in tree-filter by checking
   the submodule out and reseting the submodule's HEAD. Fails both
   with and without the previous patch. This is because filter-branch
   sets GIT_WORKING_TREE to "." which causes clone (called from
   git-submodule) to fail.

3) 'replace submodule revision' shows that replacing submodule
   revision is possible by direct index manipulation. Succeeds both
   with and without the previous patch.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 t/t7003-filter-branch.sh |   47 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 9503875..fabe038 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -306,4 +306,51 @@ test_expect_success '--remap-to-ancestor with filename filters' '
 	test $orig_invariant = $(git rev-parse invariant)
 '
 
+test_expect_success 'setup submodule' '
+	rm -rf * .*
+	git init &&
+	test_commit file &&
+	mkdir submod &&
+	submodurl="$PWD/submod"
+	( cd submod &&
+	  git init &&
+	  test_commit file-in-submod ) &&
+	git submodule add "$submodurl"
+	git commit -m "added submodule" &&
+	test_commit add-file &&
+	( cd submod && test_commit add-in-submodule ) &&
+	git add submod &&
+	git commit -m "changed submodule" &&
+	git branch original HEAD
+'
+
+orig_head=`git show-ref --hash --head HEAD`
+
+test_expect_success 'rewrite submodule with another content' '
+	git filter-branch --tree-filter "test -d submod && {
+					 rm -rf submod &&
+					 git rm -rf --quiet submod &&
+					 mkdir submod &&
+					 : > submod/file
+					 } || :" HEAD &&
+	test $orig_head != `git show-ref --hash --head HEAD`
+'
+
+test_expect_failure 'checkout submodule during rewrite' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "git submodule update --init &&
+	     ( test -d submod && cd submod &&
+	       git reset --hard origin/master )" HEAD
+'
+
+test_expect_success 'replace submodule revision' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "if git ls-files --error-unmatch -- submod > /dev/null 2>&1
+	     then git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submod
+	     fi" HEAD &&
+	test $orig_head != `git show-ref --hash --head HEAD`
+'
+
 test_done
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCHv3] filter-branch: Add tests for submodules
  2010-01-27 23:55         ` [PATCHv3] filter-branch: Add tests for submodules Michal Sojka
@ 2010-01-28  0:14           ` Junio C Hamano
  2010-01-28  9:02             ` Michal Sojka
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2010-01-28  0:14 UTC (permalink / raw)
  To: Michal Sojka; +Cc: git, j.sixt, Johannes.Schindelin

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> There are three important tests:

It is unnecessary and counterproductive to self-proclaim the importance of
a patch or new tests.  If anything, what are important are not tests
themselves but the conditions that they check, so "Add tests to check
three important cases:" is slightly more palatable.

I'd suggest to just start with "Add three tests to make sure:".

> 1) 'rewrite submodule with another content' passes only with the
>    previous patch applied.

Sorry, but I think I am missing some context here to understand this
sentence.  Which previous patch?

> 2) 'checkout submodule during rewrite' demonstrates that it is not
>    possible to replace a submodule revision in tree-filter by checking
>    the submodule out and reseting the submodule's HEAD. Fails both
>    with and without the previous patch. This is because filter-branch
>    sets GIT_WORKING_TREE to "." which causes clone (called from
>    git-submodule) to fail.

I thought you agreed with Hannes that this is not something we would even
want to support?

> 3) 'replace submodule revision' shows that replacing submodule
>    revision is possible by direct index manipulation. Succeeds both
>    with and without the previous patch.
>
> Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
> ---

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv3] filter-branch: Add tests for submodules
  2010-01-28  0:14           ` Junio C Hamano
@ 2010-01-28  9:02             ` Michal Sojka
  2010-01-28  9:08               ` [PATCHv4 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
  2010-01-28  9:08               ` [PATCHv4 2/2] filter-branch: Add tests for submodules in tree-filter Michal Sojka
  0 siblings, 2 replies; 15+ messages in thread
From: Michal Sojka @ 2010-01-28  9:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, j.sixt, Johannes.Schindelin

On Thursday 28 of January 2010 01:14:07 Junio C Hamano wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
> > There are three important tests:
> 
> It is unnecessary and counterproductive to self-proclaim the importance of
> a patch or new tests.  If anything, what are important are not tests
> themselves but the conditions that they check, so "Add tests to check
> three important cases:" is slightly more palatable.
> 
> I'd suggest to just start with "Add three tests to make sure:".

OK

> > 1) 'rewrite submodule with another content' passes only with the
> >    previous patch applied.
> 
> Sorry, but I think I am missing some context here to understand this
> sentence.  Which previous patch?

I should have sent both patches. Sorry for that. I'll send them now.
 
> > 2) 'checkout submodule during rewrite' demonstrates that it is not
> >    possible to replace a submodule revision in tree-filter by checking
> >    the submodule out and reseting the submodule's HEAD. Fails both
> >    with and without the previous patch. This is because filter-branch
> >    sets GIT_WORKING_TREE to "." which causes clone (called from
> >    git-submodule) to fail.
> 
> I thought you agreed with Hannes that this is not something we would even
> want to support?

Yes. I'm removing this test.

Cheers,
Michal

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv4 1/2] filter-branch: Fix to allow replacing submodules with another content
  2010-01-28  9:02             ` Michal Sojka
@ 2010-01-28  9:08               ` Michal Sojka
  2010-01-28  9:08               ` [PATCHv4 2/2] filter-branch: Add tests for submodules in tree-filter Michal Sojka
  1 sibling, 0 replies; 15+ messages in thread
From: Michal Sojka @ 2010-01-28  9:08 UTC (permalink / raw)
  To: git; +Cc: j.sixt, Johannes.Schindelin, gitster, Michal Sojka

When git filter-branch is used to replace a submodule with another
content, it always fails on the first commit. Consider a repository with
submod directory containing a submodule. If I want to remove the
submodule and replace it with a file, the following command fails.

git filter-branch --tree-filter 'rm -rf submod &&
				 git rm -q submod &&
				 mkdir submod &&
				 touch submod/file'

The error message is:
error: submod: is a directory - add files inside instead

The reason is that git diff-index, which generates the first part of the
list of files updated by the tree filter, emits also the removed
submodule even if it was replaced by a real directory.

Adding --ignored-submodules solves the problem for me and
tests in t7003-filter-branch.sh pass correctly.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 git-filter-branch.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 195b5ef..7c4ad7d 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -331,7 +331,7 @@ while read commit parents; do
 			die "tree filter failed: $filter_tree"
 
 		(
-			git diff-index -r --name-only $commit &&
+			git diff-index -r --name-only --ignore-submodules $commit &&
 			git ls-files --others
 		) > "$tempdir"/tree-state || exit
 		git update-index --add --replace --remove --stdin \
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv4 2/2] filter-branch: Add tests for submodules in tree-filter
  2010-01-28  9:02             ` Michal Sojka
  2010-01-28  9:08               ` [PATCHv4 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
@ 2010-01-28  9:08               ` Michal Sojka
  2010-01-28 21:57                 ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Michal Sojka @ 2010-01-28  9:08 UTC (permalink / raw)
  To: git; +Cc: j.sixt, Johannes.Schindelin, gitster, Michal Sojka

Add tests to make sure:
1) a submodule can be removed and its content replaced with regular
   files ('rewrite submodule with another content'). This test passes
   only with the previous patch applied.

2) it is possible to replace submodule revision by direct index
   manipulation ('replace submodule revision'). Although it would be
   better to run such a filter in --index-filter, this test shows that
   this functionality is not broken by the previous patch. This
   succeeds both with and without the previous patch.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 t/t7003-filter-branch.sh |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 9503875..4ee8237 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -306,4 +306,43 @@ test_expect_success '--remap-to-ancestor with filename filters' '
 	test $orig_invariant = $(git rev-parse invariant)
 '
 
+test_expect_success 'setup submodule' '
+	rm -rf * .*
+	git init &&
+	test_commit file &&
+	mkdir submod &&
+	submodurl="$PWD/submod"
+	( cd submod &&
+	  git init &&
+	  test_commit file-in-submod ) &&
+	git submodule add "$submodurl"
+	git commit -m "added submodule" &&
+	test_commit add-file &&
+	( cd submod && test_commit add-in-submodule ) &&
+	git add submod &&
+	git commit -m "changed submodule" &&
+	git branch original HEAD
+'
+
+orig_head=`git show-ref --hash --head HEAD`
+
+test_expect_success 'rewrite submodule with another content' '
+	git filter-branch --tree-filter "test -d submod && {
+					 rm -rf submod &&
+					 git rm -rf --quiet submod &&
+					 mkdir submod &&
+					 : > submod/file
+					 } || :" HEAD &&
+	test $orig_head != `git show-ref --hash --head HEAD`
+'
+
+test_expect_success 'replace submodule revision' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "if git ls-files --error-unmatch -- submod > /dev/null 2>&1
+	     then git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submod
+	     fi" HEAD &&
+	test $orig_head != `git show-ref --hash --head HEAD`
+'
+
 test_done
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 2/2] filter-branch: Add tests for submodules in tree-filter
  2010-01-28  9:08               ` [PATCHv4 2/2] filter-branch: Add tests for submodules in tree-filter Michal Sojka
@ 2010-01-28 21:57                 ` Junio C Hamano
  2010-01-29 15:20                   ` Michal Sojka
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2010-01-28 21:57 UTC (permalink / raw)
  To: Michal Sojka; +Cc: git, j.sixt, Johannes.Schindelin, gitster

Michal Sojka <sojkam1@fel.cvut.cz> writes:

> @@ -306,4 +306,43 @@ test_expect_success '--remap-to-ancestor with filename filters' '
>  	test $orig_invariant = $(git rev-parse invariant)
>  '
>  
> +test_expect_success 'setup submodule' '
> +	rm -rf * .*

Yikes.  Please don't do this.

If you cannot structure your tests following what has already been done by
the previous tests, at least name the things that you want to remove a bit
more explicitly to avoid mistakes.  The loosest form that is reasonable
would probably be (to catch a, actual, backup-refs, ... and .git):

	rm -fr ?* .?* &&

but it would be preferable to be even more explicit "rm -fr ?* .git".

Also make sure you don't break the chain of "&&" unnecessarily.

> +	git init &&
> +	test_commit file &&
> +	mkdir submod &&
> +	submodurl="$PWD/submod"
> +	( cd submod &&
> +	  git init &&
> +	  test_commit file-in-submod ) &&
> +	git submodule add "$submodurl"

"&&"?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCHv4 2/2] filter-branch: Add tests for submodules in tree-filter
  2010-01-28 21:57                 ` Junio C Hamano
@ 2010-01-29 15:20                   ` Michal Sojka
  2010-01-29 15:27                     ` [PATCHv5 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
  2010-01-29 15:27                     ` [PATCHv5 2/2] filter-branch: Add tests for submodules in tree-filter Michal Sojka
  0 siblings, 2 replies; 15+ messages in thread
From: Michal Sojka @ 2010-01-29 15:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, j.sixt, Johannes.Schindelin

On Thursday 28 of January 2010 22:57:16 Junio C Hamano wrote:
> Michal Sojka <sojkam1@fel.cvut.cz> writes:
> > +test_expect_success 'setup submodule' '
> > +	rm -rf * .*
> 
> Yikes.  Please don't do this.
> 
> [...]
> 
> but it would be preferable to be even more explicit "rm -fr ?* .git".

Fixed. I did it like you suggested.

> Also make sure you don't break the chain of "&&" unnecessarily.
> 

Fixed.

I've also split each my test into two separate tests. In the first one I 
perform the rewrite and in the second I test that the result is what was 
expected. I did it because the other tests in this file are structured the 
same way.

Cheers,
Michal

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCHv5 1/2] filter-branch: Fix to allow replacing submodules with another content
  2010-01-29 15:20                   ` Michal Sojka
@ 2010-01-29 15:27                     ` Michal Sojka
  2010-01-29 15:27                     ` [PATCHv5 2/2] filter-branch: Add tests for submodules in tree-filter Michal Sojka
  1 sibling, 0 replies; 15+ messages in thread
From: Michal Sojka @ 2010-01-29 15:27 UTC (permalink / raw)
  To: git; +Cc: j.sixt, Johannes.Schindelin, gitster, Michal Sojka

When git filter-branch is used to replace a submodule with another
content, it always fails on the first commit. Consider a repository with
submod directory containing a submodule. If I want to remove the
submodule and replace it with a file, the following command fails.

git filter-branch --tree-filter 'rm -rf submod &&
				 git rm -q submod &&
				 mkdir submod &&
				 touch submod/file'

The error message is:
error: submod: is a directory - add files inside instead

The reason is that git diff-index, which generates the first part of the
list of files updated by the tree filter, emits also the removed
submodule even if it was replaced by a real directory.

Adding --ignored-submodules solves the problem for me and
tests in t7003-filter-branch.sh pass correctly.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 git-filter-branch.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 195b5ef..7c4ad7d 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -331,7 +331,7 @@ while read commit parents; do
 			die "tree filter failed: $filter_tree"
 
 		(
-			git diff-index -r --name-only $commit &&
+			git diff-index -r --name-only --ignore-submodules $commit &&
 			git ls-files --others
 		) > "$tempdir"/tree-state || exit
 		git update-index --add --replace --remove --stdin \
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCHv5 2/2] filter-branch: Add tests for submodules in tree-filter
  2010-01-29 15:20                   ` Michal Sojka
  2010-01-29 15:27                     ` [PATCHv5 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
@ 2010-01-29 15:27                     ` Michal Sojka
  1 sibling, 0 replies; 15+ messages in thread
From: Michal Sojka @ 2010-01-29 15:27 UTC (permalink / raw)
  To: git; +Cc: j.sixt, Johannes.Schindelin, gitster, Michal Sojka

Add tests to make sure:
1) a submodule can be removed and its content replaced with regular
   files ('rewrite submodule with another content'). This test passes
   only with the previous patch applied.

2) it is possible to replace submodule revision by direct index
   manipulation ('replace submodule revision'). Although it would be
   better to run such a filter in --index-filter, this test shows that
   this functionality is not broken by the previous patch. This
   succeeds both with and without the previous patch.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 t/t7003-filter-branch.sh |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 9503875..a7f0791 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -306,4 +306,49 @@ test_expect_success '--remap-to-ancestor with filename filters' '
 	test $orig_invariant = $(git rev-parse invariant)
 '
 
+test_expect_success 'setup submodule' '
+	rm -rf ?* .git &&
+	git init &&
+	test_commit file &&
+	mkdir submod &&
+	submodurl="$PWD/submod" &&
+	( cd submod &&
+	  git init &&
+	  test_commit file-in-submod ) &&
+	git submodule add "$submodurl" &&
+	git commit -m "added submodule" &&
+	test_commit add-file &&
+	( cd submod && test_commit add-in-submodule ) &&
+	git add submod &&
+	git commit -m "changed submodule" &&
+	git branch original HEAD
+'
+
+orig_head=`git rev-parse HEAD`
+
+test_expect_success 'rewrite submodule with another content' '
+	git filter-branch --tree-filter "test -d submod && {
+					 rm -rf submod &&
+					 git rm -rf --quiet submod &&
+					 mkdir submod &&
+					 : > submod/file
+					 } || :" HEAD
+'
+test_expect_success 'test that submodule was rewritten' '
+	test -f submod/file &&
+	test $orig_head != `git rev-parse HEAD`
+'
+
+test_expect_success 'replace submodule revision' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "if git ls-files --error-unmatch -- submod > /dev/null 2>&1
+	     then git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submod
+	     fi" HEAD
+'
+test_expect_success 'test that revision was replaced' '
+	test "`git ls-files --stage submod`" = "160000 0123456789012345678901234567890123456789 0	submod" &&
+	test $orig_head != `git rev-parse HEAD`
+'
+
 test_done
-- 
1.6.6

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2010-01-29 15:27 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-25 13:06 filter-branch fix and tests Michal Sojka
2010-01-25 13:06 ` [PATCH 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
2010-01-25 13:06 ` [PATCH 2/2] filter-branch: Add tests for submodules Michal Sojka
     [not found] ` <alpine.DEB.1.00.1001261939420.4641@intel-tinevez-2-302>
2010-01-27 15:49   ` [PATCH] filter-branch fix and tests Michal Sojka
2010-01-27 16:18     ` Johannes Sixt
2010-01-27 23:41       ` Michal Sojka
2010-01-27 23:55         ` [PATCHv3] filter-branch: Add tests for submodules Michal Sojka
2010-01-28  0:14           ` Junio C Hamano
2010-01-28  9:02             ` Michal Sojka
2010-01-28  9:08               ` [PATCHv4 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
2010-01-28  9:08               ` [PATCHv4 2/2] filter-branch: Add tests for submodules in tree-filter Michal Sojka
2010-01-28 21:57                 ` Junio C Hamano
2010-01-29 15:20                   ` Michal Sojka
2010-01-29 15:27                     ` [PATCHv5 1/2] filter-branch: Fix to allow replacing submodules with another content Michal Sojka
2010-01-29 15:27                     ` [PATCHv5 2/2] filter-branch: Add tests for submodules in tree-filter Michal Sojka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).