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 johan...@kyriasis.com 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 johan...@kyriasis.com 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 johan...@kyriasis.com 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 johan...@kyriasis.com 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 johan...@kyriasis.com 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
Johannes Löthberg johan...@kyriasis.com writes: Test that the master ref is set up properly when cloning from a ref namespace Signed-off-by: Johannes Löthberg johan...@kyriasis.com --- 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
