threads / patch / 27820

patchref namespaces: tests

Subject: [PATCH] ref namespaces: tests

## tl;dr

8 messages between Jul 14, 2011 and Jul 22, 2011. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Josh Triplett· Jul 14, 2011, 20:50 UTC · lore

Test pushing, pulling, and mirroring of repositories with ref namespaces.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
---

The most recent "What's cooking" suggested that the ref namespaces patches needed tests. This test works with PATCHv10, currently in pu. Please append this patch to the js/ref-namespaces branch.

 t/t5502-fetch-push-namespaces.sh |   77 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 77 insertions(+), 0 deletions(-)
 create mode 100755 t/t5502-fetch-push-namespaces.sh
Show changes to t/t5502-fetch-push-namespaces.sh +77 −0
diff --git a/t/t5502-fetch-push-namespaces.sh b/t/t5502-fetch-push-namespaces.sh
new file mode 100755
index 0000000..85720b6
--- /dev/null
+++ b/t/t5502-fetch-push-namespaces.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+
+test_description='fetch/push involving ref namespaces'
+. ./test-lib.sh
+
+test_expect_success setup '
+	test_tick &&
+	git init original &&
+	(
+		cd original &&
+		i=0 &&
+		while [ "$i" -lt 2 ]
+		do
+			echo "$i" > count &&
+			git add count &&
+			test_commit "$i" &&
+			i=$(($i + 1))
+		done &&
+		git remote add pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&
+		git remote add pushee-unnamespaced ../pushee
+	) &&
+	git init pushee &&
+	git init puller
+'
+
+test_expect_success 'pushing into a repository using a ref namespace' '
+	(
+		cd original &&
+		git push pushee-namespaced master &&
+		git ls-remote pushee-namespaced > actual &&
+		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba\trefs/heads/master\n" > expected &&
+		test_cmp expected actual &&
+		git push pushee-namespaced --tags &&
+		git ls-remote pushee-namespaced > actual &&
+		printf "fbdf4310c71b916568f04753f603fb24a0544227\trefs/tags/0\n" >> expected &&
+		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba\trefs/tags/1\n" >> expected &&
+		test_cmp expected actual &&
+		# Verify that the GIT_NAMESPACE environment variable works as well
+		GIT_NAMESPACE=namespace git ls-remote "ext::git %s ../pushee" > actual &&
+		test_cmp expected actual &&
+		# Verify that --namespace overrides GIT_NAMESPACE
+		GIT_NAMESPACE=garbage git ls-remote pushee-namespaced > actual &&
+		test_cmp expected actual &&
+		# Try a namespace with no content
+		git ls-remote "ext::git --namespace=garbage %s ../pushee" > actual &&
+		test_cmp /dev/null actual &&
+		git ls-remote pushee-unnamespaced > actual &&
+		sed -e "s|refs/|refs/namespaces/namespace/refs/|" expected > expected.unnamespaced &&
+		test_cmp expected.unnamespaced actual
+	)
+'
+
+test_expect_success 'pulling from a repository using a ref namespace' '
+	(
+		cd puller &&
+		git remote add -f pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&
+		git for-each-ref refs/ > actual &&
+		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/remotes/pushee-namespaced/master\n" > expected &&
+		printf "fbdf4310c71b916568f04753f603fb24a0544227 commit\trefs/tags/0\n" >> expected &&
+		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/tags/1\n" >> expected &&
+		test_cmp expected actual
+	)
+'
+
+test_expect_success 'mirroring a repository using a ref namespace' '
+	git clone --mirror pushee mirror &&
+	(
+		cd mirror &&
+		git for-each-ref refs/ > actual &&
+		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/namespaces/namespace/refs/heads/master\n" > expected &&
+		printf "fbdf4310c71b916568f04753f603fb24a0544227 commit\trefs/namespaces/namespace/refs/tags/0\n" >> expected &&
+		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/namespaces/namespace/refs/tags/1\n" >> expected &&
+		test_cmp expected actual
+	)
+'
+
+test_done

-- 
1.7.5.4
Junio C Hamano· Jul 14, 2011, 23:13 UTC · re: Josh Triplett · lore

Re: [PATCH] ref namespaces: tests

Josh Triplett <josh@joshtriplett.org> writes:
Show 5 quoted lines
> Test pushing, pulling, and mirroring of repositories with ref
> namespaces.
>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> Signed-off-by: Jamey Sharp <jamey@minilop.net>
Thanks.
>  create mode 100755 t/t5502-fetch-push-namespaces.sh
Isn't 5502 used already?
Show 9 quoted lines
> +test_expect_success setup '
> +	test_tick &&
> +	git init original &&
> +	(
> +		cd original &&
> +		i=0 &&
> +		while [ "$i" -lt 2 ]
> +		do
> +			echo "$i" > count &&
This is just style, but the test scripts prefer to spell these like this:
	while test "$i" -lt 2
	do
        	echo "$i" >count &&
                ...

to favor "test" over "[ ... ]", and omit SP between ">" redirection (or "<" for that matter) and the filename.

> +		git remote add pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&
Nice ;-).
Show 6 quoted lines
> +test_expect_success 'pushing into a repository using a ref namespace' '
> +	(
> +		cd original &&
> +		git push pushee-namespaced master &&
> +		git ls-remote pushee-namespaced > actual &&
> +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba\trefs/heads/master\n" > expected &&

Could you avoid hardcoding the exact object names here? Your script knows what object should appear at refs/heads/master at "pushee-namespaced" (as you have pushed from the repository "original" you are in), so it may be something like:

	printf "%s\trefs/heads/mater\n" $(git rev-parse master) >expect
Same comment applies for all the other hardcoded object names.
Show 11 quoted lines
> +test_expect_success 'mirroring a repository using a ref namespace' '
> +	git clone --mirror pushee mirror &&
> +	(
> +		cd mirror &&
> +		git for-each-ref refs/ > actual &&
> +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/namespaces/namespace/refs/heads/master\n" > expected &&
> +		printf "fbdf4310c71b916568f04753f603fb24a0544227 commit\trefs/namespaces/namespace/refs/tags/0\n" >> expected &&
> +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/namespaces/namespace/refs/tags/1\n" >> expected &&
> +		test_cmp expected actual
> +	)
> +'

I am not sure what you are trying to test. "pushee" is pretending to be a hosting site that uses the namespace feature to house refs pushed from original in refs/namespaces/namespace/ so it is expected to have these refs under there. You didn't make any "git remote" configuration in either mirror nor pushee, so it is natural with or without the namespace feature that "git clone --mirror" would find them at the same place.

What hasn't been tested in the above is to see what actual refs pushee has with (cd pushee && git for-each-ref), and you could argue that this test is a proxy for that, but then you are assuming that "clone --mirror" is not broken, which means it would make debugging harder when this test does start failing---is it the basic namespace feature, or is it mirror cloning that acquired a bug to break this test?

Also I would have expected that some configuration that affects the central repository side (i.e. "pushee") to cause unsuspecting clients (e.g. "original" that pushes into it) to see only a subset of refs "pushee" has, which would be the real motivation for this feature. To show that off, we may also need tests that house two or more namespaces hosted in the same repository.

Josh Triplett· Jul 15, 2011, 03:45 UTC · re: Junio C Hamano · lore

Re: [PATCH] ref namespaces: tests

On Thu, Jul 14, 2011 at 04:13:48PM -0700, Junio C Hamano wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
> >  create mode 100755 t/t5502-fetch-push-namespaces.sh
> 
> Isn't 5502 used already?

Argh. Yes; I put it right after t5501-fetch-push-alternates.sh without noticing that 5502 already existed. I'll fix it to use t5507. Thanks for catching that.

Show 19 quoted lines
> > +test_expect_success setup '
> > +	test_tick &&
> > +	git init original &&
> > +	(
> > +		cd original &&
> > +		i=0 &&
> > +		while [ "$i" -lt 2 ]
> > +		do
> > +			echo "$i" > count &&
> 
> This is just style, but the test scripts prefer to spell these like this:
> 
> 	while test "$i" -lt 2
> 	do
>         	echo "$i" >count &&
>                 ...
> 
> to favor "test" over "[ ... ]", and omit SP between ">" redirection (or
> "<" for that matter) and the filename.

Will do. I had done a quick grep-survey of the tests to check usage of test versus [, and saw enough of both to assume it didn't matter, but it hadn't occurred to me to check CodingGuidelines for shell scripts; I now see that it has a section specifically on shell scripting. I'll fix this in the next version.

Actually, I plan to unroll this two-iteration loop in the next version, so that I can capture the two object hashes I need for use later in the script.

Out of curiosity, what's the rationale for the use of test rather than '['? Just uniformity, or does test have some particular advantage over '['?

> > +		git remote add pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&
> 
> Nice ;-).
Thanks. :)
Show 15 quoted lines
> > +test_expect_success 'pushing into a repository using a ref namespace' '
> > +	(
> > +		cd original &&
> > +		git push pushee-namespaced master &&
> > +		git ls-remote pushee-namespaced > actual &&
> > +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba\trefs/heads/master\n" > expected &&
> 
> Could you avoid hardcoding the exact object names here?  Your script knows
> what object should appear at refs/heads/master at "pushee-namespaced" (as
> you have pushed from the repository "original" you are in), so it may be
> something like:
> 
> 	printf "%s\trefs/heads/mater\n" $(git rev-parse master) >expect
> 
> Same comment applies for all the other hardcoded object names.

I can do that; since the same two object hashes recur throughout the script, I'll record them in shell variables up at the top.

Show 25 quoted lines
> > +test_expect_success 'mirroring a repository using a ref namespace' '
> > +	git clone --mirror pushee mirror &&
> > +	(
> > +		cd mirror &&
> > +		git for-each-ref refs/ > actual &&
> > +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/namespaces/namespace/refs/heads/master\n" > expected &&
> > +		printf "fbdf4310c71b916568f04753f603fb24a0544227 commit\trefs/namespaces/namespace/refs/tags/0\n" >> expected &&
> > +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba commit\trefs/namespaces/namespace/refs/tags/1\n" >> expected &&
> > +		test_cmp expected actual
> > +	)
> > +'
> 
> I am not sure what you are trying to test. "pushee" is pretending to be a
> hosting site that uses the namespace feature to house refs pushed from
> original in refs/namespaces/namespace/ so it is expected to have these
> refs under there.  You didn't make any "git remote" configuration in
> either mirror nor pushee, so it is natural with or without the namespace
> feature that "git clone --mirror" would find them at the same place.
> 
> What hasn't been tested in the above is to see what actual refs pushee has
> with (cd pushee && git for-each-ref), and you could argue that this test
> is a proxy for that, but then you are assuming that "clone --mirror" is
> not broken, which means it would make debugging harder when this test does
> start failing---is it the basic namespace feature, or is it mirror cloning
> that acquired a bug to break this test?

I wrote this test specifically to check for possible regressions in clone or the machinery underneath it. I wanted to ensure that no future change caused clone to ignore refs in refs/namespaces/*. In particular, I want to protect against a regression caused by any future change to the refs machinery that might cause it to ignore refs outside of refs/heads/* or refs/tags/*, which might otherwise go un-noticed (as they almost did during the development of this patchset, if not for an incidental side effect of t5501).

If this test failed, I would expect that it would fail because clone --mirror produced a mirrored repository which didn't actually contain any refs, even though pushee contained the correctly namespaced refs; thus, for-each-ref doesn't seem like the right test.

More generally, I also added this test because it tests a specific high-level feature I care about: the ability to mirror a repository containing namespaces using clone --mirror, and preserve those namespaces. I plan to use that as a backup mechanism, and I want it to continue working. :)

- Josh Triplett
Josh Triplett· Jul 15, 2011, 18:40 UTC · re: Josh Triplett · lore

[PATCHv2] ref namespaces: tests

Test pushing, pulling, and mirroring of repositories with ref namespaces.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
---

v2: Incorporate feedback from Junio: change test number to 5507 to avoid conflicting with existing tests, make style consistent with other test scripts, avoid hardcoding commit hashes, add rationale for mirror test.

 t/t5507-fetch-push-namespaces.sh |   85 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 85 insertions(+), 0 deletions(-)
 create mode 100755 t/t5507-fetch-push-namespaces.sh
Show changes to t/t5507-fetch-push-namespaces.sh +85 −0
diff --git a/t/t5507-fetch-push-namespaces.sh b/t/t5507-fetch-push-namespaces.sh
new file mode 100755
index 0000000..cc0b31f
--- /dev/null
+++ b/t/t5507-fetch-push-namespaces.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+
+test_description='fetch/push involving ref namespaces'
+. ./test-lib.sh
+
+test_expect_success setup '
+	test_tick &&
+	git init original &&
+	(
+		cd original &&
+		echo 0 >count &&
+		git add count &&
+		test_commit 0 &&
+		echo 1 >count &&
+		git add count &&
+		test_commit 1 &&
+		git remote add pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&
+		git remote add pushee-unnamespaced ../pushee
+	) &&
+	commit0=$(cd original && git rev-parse HEAD^) &&
+	commit1=$(cd original && git rev-parse HEAD) &&
+	git init pushee &&
+	git init puller
+'
+
+test_expect_success 'pushing into a repository using a ref namespace' '
+	(
+		cd original &&
+		git push pushee-namespaced master &&
+		git ls-remote pushee-namespaced >actual &&
+		printf "$commit1\trefs/heads/master\n" >expected &&
+		test_cmp expected actual &&
+		git push pushee-namespaced --tags &&
+		git ls-remote pushee-namespaced >actual &&
+		printf "$commit0\trefs/tags/0\n" >>expected &&
+		printf "$commit1\trefs/tags/1\n" >>expected &&
+		test_cmp expected actual &&
+		# Verify that the GIT_NAMESPACE environment variable works as well
+		GIT_NAMESPACE=namespace git ls-remote "ext::git %s ../pushee" >actual &&
+		test_cmp expected actual &&
+		# Verify that --namespace overrides GIT_NAMESPACE
+		GIT_NAMESPACE=garbage git ls-remote pushee-namespaced >actual &&
+		test_cmp expected actual &&
+		# Try a namespace with no content
+		git ls-remote "ext::git --namespace=garbage %s ../pushee" >actual &&
+		test_cmp /dev/null actual &&
+		git ls-remote pushee-unnamespaced >actual &&
+		sed -e "s|refs/|refs/namespaces/namespace/refs/|" expected >expected.unnamespaced &&
+		test_cmp expected.unnamespaced actual
+	)
+'
+
+test_expect_success 'pulling from a repository using a ref namespace' '
+	(
+		cd puller &&
+		git remote add -f pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&
+		git for-each-ref refs/ >actual &&
+		printf "$commit1 commit\trefs/remotes/pushee-namespaced/master\n" >expected &&
+		printf "$commit0 commit\trefs/tags/0\n" >>expected &&
+		printf "$commit1 commit\trefs/tags/1\n" >>expected &&
+		test_cmp expected actual
+	)
+'
+
+# This test with clone --mirror checks for possible regressions in clone
+# or the machinery underneath it. It ensures that no future change
+# causes clone to ignore refs in refs/namespaces/*. In particular, it
+# protects against a regression caused by any future change to the refs
+# machinery that might cause it to ignore refs outside of refs/heads/*
+# or refs/tags/*. More generally, this test also checks the high-level
+# functionality of using clone --mirror to back up a set of repos hosted
+# in the namespaces of a single repo.
+test_expect_success 'mirroring a repository using a ref namespace' '
+	git clone --mirror pushee mirror &&
+	(
+		cd mirror &&
+		git for-each-ref refs/ >actual &&
+		printf "$commit1 commit\trefs/namespaces/namespace/refs/heads/master\n" >expected &&
+		printf "$commit0 commit\trefs/namespaces/namespace/refs/tags/0\n" >>expected &&
+		printf "$commit1 commit\trefs/namespaces/namespace/refs/tags/1\n" >>expected &&
+		test_cmp expected actual
+	)
+'
+
+test_done

-- 
1.7.5.4
Josh Triplett· Jul 21, 2011, 20:10 UTC · re: Josh Triplett · lore

[PATCHv3] ref namespaces: tests

Test pushing, pulling, and mirroring of repositories with ref namespaces.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
---

v3: Change test number to 5509 to avoid conflicts with new tests 5507 and 5508 in pu.

At this point I think we've incorporated all the outstanding feedback. Does this test need any further changes to allow the ref-namespaces branch to graduate to next?

(Also, for future reference, do you prefer to see later versions of patches as replies to the previous version, as I've made this mail a reply to PATCHv2, or do you prefer to see them as new threads?)

v2: Incorporate feedback from Junio: change test number to 5507 to avoid conflicting with existing tests, make style consistent with other test scripts, avoid hardcoding commit hashes, add rationale for mirror test.

 t/t5509-fetch-push-namespaces.sh |   85 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 85 insertions(+), 0 deletions(-)
 create mode 100755 t/t5509-fetch-push-namespaces.sh
Show changes to t/t5509-fetch-push-namespaces.sh +85 −0
diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh
new file mode 100755
index 0000000..cc0b31f
--- /dev/null
+++ b/t/t5509-fetch-push-namespaces.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+
+test_description='fetch/push involving ref namespaces'
+. ./test-lib.sh
+
+test_expect_success setup '
+	test_tick &&
+	git init original &&
+	(
+		cd original &&
+		echo 0 >count &&
+		git add count &&
+		test_commit 0 &&
+		echo 1 >count &&
+		git add count &&
+		test_commit 1 &&
+		git remote add pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&
+		git remote add pushee-unnamespaced ../pushee
+	) &&
+	commit0=$(cd original && git rev-parse HEAD^) &&
+	commit1=$(cd original && git rev-parse HEAD) &&
+	git init pushee &&
+	git init puller
+'
+
+test_expect_success 'pushing into a repository using a ref namespace' '
+	(
+		cd original &&
+		git push pushee-namespaced master &&
+		git ls-remote pushee-namespaced >actual &&
+		printf "$commit1\trefs/heads/master\n" >expected &&
+		test_cmp expected actual &&
+		git push pushee-namespaced --tags &&
+		git ls-remote pushee-namespaced >actual &&
+		printf "$commit0\trefs/tags/0\n" >>expected &&
+		printf "$commit1\trefs/tags/1\n" >>expected &&
+		test_cmp expected actual &&
+		# Verify that the GIT_NAMESPACE environment variable works as well
+		GIT_NAMESPACE=namespace git ls-remote "ext::git %s ../pushee" >actual &&
+		test_cmp expected actual &&
+		# Verify that --namespace overrides GIT_NAMESPACE
+		GIT_NAMESPACE=garbage git ls-remote pushee-namespaced >actual &&
+		test_cmp expected actual &&
+		# Try a namespace with no content
+		git ls-remote "ext::git --namespace=garbage %s ../pushee" >actual &&
+		test_cmp /dev/null actual &&
+		git ls-remote pushee-unnamespaced >actual &&
+		sed -e "s|refs/|refs/namespaces/namespace/refs/|" expected >expected.unnamespaced &&
+		test_cmp expected.unnamespaced actual
+	)
+'
+
+test_expect_success 'pulling from a repository using a ref namespace' '
+	(
+		cd puller &&
+		git remote add -f pushee-namespaced "ext::git --namespace=namespace %s ../pushee" &&
+		git for-each-ref refs/ >actual &&
+		printf "$commit1 commit\trefs/remotes/pushee-namespaced/master\n" >expected &&
+		printf "$commit0 commit\trefs/tags/0\n" >>expected &&
+		printf "$commit1 commit\trefs/tags/1\n" >>expected &&
+		test_cmp expected actual
+	)
+'
+
+# This test with clone --mirror checks for possible regressions in clone
+# or the machinery underneath it. It ensures that no future change
+# causes clone to ignore refs in refs/namespaces/*. In particular, it
+# protects against a regression caused by any future change to the refs
+# machinery that might cause it to ignore refs outside of refs/heads/*
+# or refs/tags/*. More generally, this test also checks the high-level
+# functionality of using clone --mirror to back up a set of repos hosted
+# in the namespaces of a single repo.
+test_expect_success 'mirroring a repository using a ref namespace' '
+	git clone --mirror pushee mirror &&
+	(
+		cd mirror &&
+		git for-each-ref refs/ >actual &&
+		printf "$commit1 commit\trefs/namespaces/namespace/refs/heads/master\n" >expected &&
+		printf "$commit0 commit\trefs/namespaces/namespace/refs/tags/0\n" >>expected &&
+		printf "$commit1 commit\trefs/namespaces/namespace/refs/tags/1\n" >>expected &&
+		test_cmp expected actual
+	)
+'
+
+test_done

-- 
1.7.5.4
Junio C Hamano· Jul 21, 2011, 21:56 UTC · re: Josh Triplett · lore

Re: [PATCHv3] ref namespaces: tests

Josh Triplett <josh@joshtriplett.org> writes:
> At this point I think we've incorporated all the outstanding feedback.
> Does this test need any further changes to allow the ref-namespaces
> branch to graduate to next?
No more nitpicks from me on this patch at least for now.

Are people who expressed concern during the review on the previous round of the series happy with the second round? I recall there was a strong sentiment that it is regrettable that the series specifically changes fetch and push and is not a more general mechanism. Personally I am OK with the approach taken by this series, as I do not offhand think of other ways to serve a modified namespace. You have to view the unaltered reality when interacting with your own refs to enumerate the objects you have, while giving the altered view to your clients that is limited to the "virtual" space.

> (Also, for future reference, do you prefer to see later versions of
> patches as replies to the previous version, as I've made this mail a
> reply to PATCHv2, or do you prefer to see them as new threads?)

Often it is very convenient to be able to go "up" in the thread to re-read the discussions in the previous round. On the other hand it sometimes gets inconvenient when viewing tons of threads to have a beginning of a new round buried deep in other threads. Referring to the messages in the previous round by their message-id (or thread.gmane.org/ URL) in the body of the patch message below "---" lines may solve both issues, but I can go either way.

Jeff King· Jul 22, 2011, 22:32 UTC · re: Junio C Hamano · lore

Re: [PATCHv3] ref namespaces: tests

On Thu, Jul 21, 2011 at 02:56:16PM -0700, Junio C Hamano wrote:
Show 17 quoted lines
> Josh Triplett <josh@joshtriplett.org> writes:
> 
> > At this point I think we've incorporated all the outstanding feedback.
> > Does this test need any further changes to allow the ref-namespaces
> > branch to graduate to next?
> 
> No more nitpicks from me on this patch at least for now.
> 
> Are people who expressed concern during the review on the previous round
> of the series happy with the second round? I recall there was a strong
> sentiment that it is regrettable that the series specifically changes
> fetch and push and is not a more general mechanism. Personally I am OK
> with the approach taken by this series, as I do not offhand think of other
> ways to serve a modified namespace. You have to view the unaltered reality
> when interacting with your own refs to enumerate the objects you have,
> while giving the altered view to your clients that is limited to the
> "virtual" space.

I was one of the people who wanted to have namespaces or virtual repos at a more fundamental level. However, I tried to do a relatively simple patch when the discussion started, and ended up getting mired in corner cases. And it sounds like Josh and Jamey made a good faith effort in that direction, but still ended up where they are now. So I'm willing to accept that it is not as simple as we hoped, and the more practical approach from their series is acceptable.

As for the code itself, I admit I haven't been paying all that close attention. I can try to give a more careful review if we want another set of eyes.

-Peff
Junio C Hamano· Jul 15, 2011, 19:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] ref namespaces: tests

Junio C Hamano <gitster@pobox.com> writes:
Show 15 quoted lines
>> +test_expect_success 'pushing into a repository using a ref namespace' '
>> +	(
>> +		cd original &&
>> +		git push pushee-namespaced master &&
>> +		git ls-remote pushee-namespaced > actual &&
>> +		printf "dc65a2e0f299dcc7efddbbe01641a28ee84329ba\trefs/heads/master\n" > expected &&
>
> Could you avoid hardcoding the exact object names here?  Your script knows
> what object should appear at refs/heads/master at "pushee-namespaced" (as
> you have pushed from the repository "original" you are in), so it may be
> something like:
>
> 	printf "%s\trefs/heads/mater\n" $(git rev-parse master) >expect
>
> Same comment applies for all the other hardcoded object names.

Just FYI, I did a sample fix-up for earlier tests in this patch and pushed it out in 'pu'; hopefully it may save your time.

Thanks.

← back to recent threads