Re: [PATCH v2 2/2] t: Add test for cloning from ref namespace
Johannes Löthberg writes: > Would it be acceptable to check against > ../bare/refs/namespaces/new_namespace/HEAD and > ../bare/refs/namespaces/new_namespace/refs/heads/master instead, until > rev-parse is thaught about namespaces? Yes. Because I do not immediately see any legitimate reason for the rest of the system (including rev-parse) to ever start paying attention to GIT_NAMESPACE, I think that is not even "instead, until" but is the right solution. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] t: Add test for cloning from ref namespace
On 05/06, Junio C Hamano wrote: Johannes Löthberg writes: Hmm, it seems that git-rev-parse doesn't handle GIT_NAMESPACE yet, so can't check it for the namespaced push right now. Not sure if I can fix that myself though. I do not see a need for rev-parse to pay attention to GIT_NAMESPACE at all, though. The ref namespace has its own HEAD, so I'd expect GIT_NAMESPACE=foo git rev-parse HEAD to act sensibly The destination that accepts the push with the enviornment variable, i.e. your ../bare repository after this: + git commit -m "commit two" + GIT_NAMESPACE=new_namespace git push ../bare master must be saving the result somewhere in ../bare/, and that is what you want to check (and also no refs are affected outside that hierarchy). So perhaps along the lines of echo $(git rev-parse master) commit \ refs/namespaces/new_namespace/refs/heads/master >expect && git -C ../bare for-each-ref refs/namespaces/ >actual && test_cmp expect actual or something? You would want to also check that other refs are not molested, so ( echo $(git rev-parse master^) commit \ refs/heads/master && echo $(git rev-parse master) commit \ refs/namespaces/new_namespace/refs/heads/master ) >expect && git -C ../bare for-each-ref >actual && test_cmp expect actual might be a more appropriate test. Sounds okay. -- Sincerely, Johannes Löthberg PGP Key ID: 0x50FB9B273A9D0BB5 https://theos.kyriasis.com/~kyrias/ signature.asc Description: PGP signature
Re: [PATCH v2 2/2] t: Add test for cloning from ref namespace
On 05/06, Johannes Löthberg wrote: On 05/06, Junio C Hamano wrote: Johannes Löthberg writes: git -C ../bare symbolic-ref HEAD >actual && echo refs/heads/master >expect && test_cmp expect actual && git -C ../bare rev-parse HEAD >actual && git rev-parse HEAD >expect && test_cmp expect actual && Hmm, it seems that git-rev-parse doesn't handle GIT_NAMESPACE yet, so can't check it for the namespaced push right now. Not sure if I can fix that myself though. Would it be acceptable to check against ../bare/refs/namespaces/new_namespace/HEAD and ../bare/refs/namespaces/new_namespace/refs/heads/master instead, until rev-parse is thaught about namespaces? -- Sincerely, Johannes Löthberg PGP Key ID: 0x50FB9B273A9D0BB5 https://theos.kyriasis.com/~kyrias/ signature.asc Description: PGP signature
Re: [PATCH v2 2/2] t: Add test for cloning from ref namespace
Johannes Löthberg writes: > Hmm, it seems that git-rev-parse doesn't handle GIT_NAMESPACE yet, so > can't check it for the namespaced push right now. Not sure if I can > fix that myself though. I do not see a need for rev-parse to pay attention to GIT_NAMESPACE at all, though. The destination that accepts the push with the enviornment variable, i.e. your ../bare repository after this: + git commit -m "commit two" + GIT_NAMESPACE=new_namespace git push ../bare master must be saving the result somewhere in ../bare/, and that is what you want to check (and also no refs are affected outside that hierarchy). So perhaps along the lines of echo $(git rev-parse master) commit \ refs/namespaces/new_namespace/refs/heads/master >expect && git -C ../bare for-each-ref refs/namespaces/ >actual && test_cmp expect actual or something? You would want to also check that other refs are not molested, so ( echo $(git rev-parse master^) commit \ refs/heads/master && echo $(git rev-parse master) commit \ refs/namespaces/new_namespace/refs/heads/master ) >expect && git -C ../bare for-each-ref >actual && test_cmp expect actual might be a more appropriate test. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] t: Add test for cloning from ref namespace
On 05/06, Junio C Hamano wrote: Johannes Löthberg writes: + It seems that 5509 already has a few tests for namespaced transfer in both directions. Perhaps this new test would fit there better? Missed that, will move it there. Also I think it probably is better to have these as a single patch. As you wish. + git add file && + git commit -m "commit one" && + git push ../bare master && You want to make sure not just "push" does not complain, but that it left ../bare with the right result, i.e. something along the lines of git -C ../bare symbolic-ref HEAD >actual && echo refs/heads/master >expect && test_cmp expect actual && git -C ../bare rev-parse HEAD >actual && git rev-parse HEAD >expect && test_cmp expect actual && Hmm, it seems that git-rev-parse doesn't handle GIT_NAMESPACE yet, so can't check it for the namespaced push right now. Not sure if I can fix that myself though. + ) && + GIT_NAMESPACE=new_namespace git clone bare clone && + ( + cd clone && + git show Likewise on checking the result of the clone; not just it has HEAD to cause "show" to succeed, you would want it shows the right commit (i.e. not "one", but "two"). There may be other things you may want to check, too. -- Sincerely, Johannes Löthberg PGP Key ID: 0x50FB9B273A9D0BB5 https://theos.kyriasis.com/~kyrias/ signature.asc Description: PGP signature
Re: [PATCH v2 2/2] t: Add test for cloning from ref namespace
Johannes Löthberg writes: > Test that the master ref is set up properly when cloning from a ref > namespace > > Signed-off-by: Johannes Löthberg > --- > t/t9904-clone-from-ref-namespace.sh | 33 + It seems that 5509 already has a few tests for namespaced transfer in both directions. Perhaps this new test would fit there better? Also I think it probably is better to have these as a single patch. > diff --git a/t/t9904-clone-from-ref-namespace.sh > b/t/t9904-clone-from-ref-namespace.sh > new file mode 100755 > index 000..60977f8 > --- /dev/null > +++ b/t/t9904-clone-from-ref-namespace.sh > @@ -0,0 +1,33 @@ > +#!/bin/sh > +# > + > +test_description='git clone from ref namespace > + > +This test checks that cloning from a ref namespace works' > + > +. ./test-lib.sh > + > +test_expect_success 'clone from ref namespace' ' > + rm -rf initial bare clone && > + git init initial && > + git init --bare bare && > + ( > + cd initial && > + echo "commit one" >> file && minor style: drop SP between redirection and its target, i.e. echo "commit one" >file && > + git add file && > + git commit -m "commit one" && > + git push ../bare master && You want to make sure not just "push" does not complain, but that it left ../bare with the right result, i.e. something along the lines of git -C ../bare symbolic-ref HEAD >actual && echo refs/heads/master >expect && test_cmp expect actual && git -C ../bare rev-parse HEAD >actual && git rev-parse HEAD >expect && test_cmp expect actual && > + echo "commit two" >> file && Likewise on style. > + git add file && > + git commit -m "commit two" Broken &&-chain. > + GIT_NAMESPACE=new_namespace git push ../bare master Likewise on checking the result of the push. > + ) && > + GIT_NAMESPACE=new_namespace git clone bare clone && > + ( > + cd clone && > + git show Likewise on checking the result of the clone; not just it has HEAD to cause "show" to succeed, you would want it shows the right commit (i.e. not "one", but "two"). There may be other things you may want to check, too. > + ) > +' > + > +test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] t: Add test for cloning from ref namespace
Test that the master ref is set up properly when cloning from a ref namespace Signed-off-by: Johannes Löthberg --- t/t9904-clone-from-ref-namespace.sh | 33 + 1 file changed, 33 insertions(+) create mode 100755 t/t9904-clone-from-ref-namespace.sh diff --git a/t/t9904-clone-from-ref-namespace.sh b/t/t9904-clone-from-ref-namespace.sh new file mode 100755 index 000..60977f8 --- /dev/null +++ b/t/t9904-clone-from-ref-namespace.sh @@ -0,0 +1,33 @@ +#!/bin/sh +# + +test_description='git clone from ref namespace + +This test checks that cloning from a ref namespace works' + +. ./test-lib.sh + +test_expect_success 'clone from ref namespace' ' + rm -rf initial bare clone && + git init initial && + git init --bare bare && + ( + cd initial && + echo "commit one" >> file && + git add file && + git commit -m "commit one" && + git push ../bare master && + + echo "commit two" >> file && + git add file && + git commit -m "commit two" + GIT_NAMESPACE=new_namespace git push ../bare master + ) && + GIT_NAMESPACE=new_namespace git clone bare clone && + ( + cd clone && + git show + ) +' + +test_done -- 2.4.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
