{"thread":{"id":"38192","subject":"Introducing a test_create_repo_bare (was Re: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes)","startedAt":"2014-12-17T23:20:22Z","lastAt":"2014-12-18T17:06:28Z","messageCount":5,"participants":["Stefan Beller","Junio C Hamano","Jonathan Nieder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"253786","messageId":"CAGZ79kY=TP31VJxPZnjb04og-vHU+-c4d+AgAkis2Q7yeDeXbg@mail.gmail.com","threadId":"38192","inReplyTo":null,"subject":"Introducing a test_create_repo_bare (was Re: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes)","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-17T23:20:22Z","receivedAt":"2014-12-17T23:20:22Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Dec 16, 2014 at 11:46 AM, Junio C Hamano wrote:\n\n>> + (\n>> + cd upstream && git config receive.denyCurrentBranch warn\n>> + ) &&\n>\n> I was wondering how you would do this part after suggesting use of\n> test_create_repo, knowing very well that one of them was a bare one\n> ;-).\n>\n> We might want to extend test_create_repo to allow creating a bare\n> repository, but this is also OK.\n\nSo I searching through all the tests, where it would make sense to do that.\nI searched for \"denyCurrentBranch\" and came up with this list where I think\nit makes sense to replace (git init | test_create_repo | or alike) by a\ntest_create_repo_bare or add the --bare option to test_create_repo\n\nplaces where test_create_repo_bare is easily introducable:\nt5517-push-mirror.sh # setup an upstream repo\nt5543-atomic-push.sh # setup an upstream repo\nt5701-clone-local.sh # Test 'clone empty repository'\n\nnot as easy:\nt5400-send-pack.sh # we commit to that repo while being inside\nt5405-send-pack-rewind.sh # we commit to that repo while being inside\nt5516-fetch-push.sh # we test the various denyCurrentBranch options\n\nunsure:\nt5522-pull-symlink.sh # just cloning the repo\n\nSo I think we don't need the test_create_repo_bare yet.\n\nThanks,\nStefan\n"},{"id":"253788","messageId":"xmqq61d996oc.fsf@gitster.dls.corp.google.com","threadId":"38192","inReplyTo":"CAGZ79kY=TP31VJxPZnjb04og-vHU+-c4d+AgAkis2Q7yeDeXbg@mail.gmail.com","subject":"Re: Introducing a test_create_repo_bare (was Re: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-17T23:51:15Z","receivedAt":"2014-12-17T23:51:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Tue, Dec 16, 2014 at 11:46 AM, Junio C Hamano wrote:\n>\n>>> + (\n>>> + cd upstream && git config receive.denyCurrentBranch warn\n>>> + ) &&\n>>\n>> I was wondering how you would do this part after suggesting use of\n>> test_create_repo, knowing very well that one of them was a bare one\n>> ;-).\n>>\n>> We might want to extend test_create_repo to allow creating a bare\n>> repository, but this is also OK.\n>\n> So I searching through all the tests, where it would make sense to do that.\n> I searched for \"denyCurrentBranch\" and came up with this list where I think\n> it makes sense to replace (git init | test_create_repo | or alike) by a\n> test_create_repo_bare or add the --bare option to test_create_repo\n>\n> places where test_create_repo_bare is easily introducable:\n> t5517-push-mirror.sh # setup an upstream repo\n> t5543-atomic-push.sh # setup an upstream repo\n> t5701-clone-local.sh # Test 'clone empty repository'\n>\n> not as easy:\n> t5400-send-pack.sh # we commit to that repo while being inside\n> t5405-send-pack-rewind.sh # we commit to that repo while being inside\n> t5516-fetch-push.sh # we test the various denyCurrentBranch options\n>\n> unsure:\n> t5522-pull-symlink.sh # just cloning the repo\n>\n> So I think we don't need the test_create_repo_bare yet.\n\nThanks for digging.\n\nWe already knew we do not *NEED* it. We have been surviving without\none.\n\nYou need to remember that adding and using a new helper is *NOT* the\nultimate goal; categorizing those that do not want bare repositories\nas \"not as easy\" is misguided. They truly do not want bare, so they\nare not our target audience in the first place. For the same reason,\n\"easily introduceable\" is not a good criteria to look for.\n\nThe issue is if some existing tests will be helped, if we had such a\nhelper. That is, do we \"git init --bare\" by hand in some test? Are\nthese tests in such a hand-crafted repositories more susceptible to\nfuture breakages because they do not use the template from the built\nlocation or they do not disable hooks? If we had such tests, then\nthey would benefit by having a \"bare\" mode of test_create_repo.\n"},{"id":"253792","messageId":"CAGZ79kYiYiErT-fuiQLCW_0zByiJJt_pfJ5RGUiFAP4jgxLuVA@mail.gmail.com","threadId":"38192","inReplyTo":"xmqq61d996oc.fsf@gitster.dls.corp.google.com","subject":"Re: Introducing a test_create_repo_bare (was Re: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes)","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-12-18T00:14:27Z","receivedAt":"2014-12-18T00:14:27Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Dec 17, 2014 at 3:51 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> On Tue, Dec 16, 2014 at 11:46 AM, Junio C Hamano wrote:\n>>\n>>>> + (\n>>>> + cd upstream && git config receive.denyCurrentBranch warn\n>>>> + ) &&\n>>>\n>>> I was wondering how you would do this part after suggesting use of\n>>> test_create_repo, knowing very well that one of them was a bare one\n>>> ;-).\n>>>\n>>> We might want to extend test_create_repo to allow creating a bare\n>>> repository, but this is also OK.\n>>\n>> So I searching through all the tests, where it would make sense to do that.\n>> I searched for \"denyCurrentBranch\" and came up with this list where I think\n>> it makes sense to replace (git init | test_create_repo | or alike) by a\n>> test_create_repo_bare or add the --bare option to test_create_repo\n>>\n>> places where test_create_repo_bare is easily introducable:\n>> t5517-push-mirror.sh # setup an upstream repo\n>> t5543-atomic-push.sh # setup an upstream repo\n>> t5701-clone-local.sh # Test 'clone empty repository'\n>>\n>> not as easy:\n>> t5400-send-pack.sh # we commit to that repo while being inside\n>> t5405-send-pack-rewind.sh # we commit to that repo while being inside\n>> t5516-fetch-push.sh # we test the various denyCurrentBranch options\n>>\n>> unsure:\n>> t5522-pull-symlink.sh # just cloning the repo\n>>\n>> So I think we don't need the test_create_repo_bare yet.\n>\n> Thanks for digging.\n>\n> We already knew we do not *NEED* it. We have been surviving without\n> one.\n>\n> You need to remember that adding and using a new helper is *NOT* the\n> ultimate goal; categorizing those that do not want bare repositories\n> as \"not as easy\" is misguided. They truly do not want bare, so they\n> are not our target audience in the first place. For the same reason,\n> \"easily introduceable\" is not a good criteria to look for.\n>\n> The issue is if some existing tests will be helped, if we had such a\n> helper. That is, do we \"git init --bare\" by hand in some test? Are\n> these tests in such a hand-crafted repositories more susceptible to\n> future breakages because they do not use the template from the built\n> location or they do not disable hooks? If we had such tests, then\n> they would benefit by having a \"bare\" mode of test_create_repo.\n>\n>\n\nYou're right, I was looking at the wrong criteria. There are rougly 125\n\" --bare\" outside outside of the t0001-init.sh and t5601-clone.sh\nwhich are not relying on the template nor the $GIT_EXEC_PATH/git-init\n\nThe occurrences outside these two tests are mostly init and clone with --bare\nalthough we also have a few \"git --bare foo\" commands to operate on\nthe bare repos.\n\nLooking at it the other way round it seems to be more work to fix it. :/\n"},{"id":"253793","messageId":"20141218002825.GQ29365@google.com","threadId":"38192","inReplyTo":"xmqq61d996oc.fsf@gitster.dls.corp.google.com","subject":"Re: Introducing a test_create_repo_bare (was Re: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2014-12-18T00:28:25Z","receivedAt":"2014-12-18T00:28:25Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> The issue is if some existing tests will be helped, if we had such a\n> helper.\n\nSince both bin-wrappers/git and test-lib.sh set GIT_TEMPLATE_DIR and\ntemplates/blt doesn't contain any enabled hooks, I don't see how such\na helper would be useful.\n\nIf making things more consistent were worth the churn, then if\nanything it would make sense to make test_create_repo private to the\nsetup code in test-lib.sh and to use plain 'git init ' in\ntests.\n\nJonathan\n"},{"id":"253809","messageId":"xmqqlhm47uqz.fsf@gitster.dls.corp.google.com","threadId":"38192","inReplyTo":"20141218002825.GQ29365@google.com","subject":"Re: Introducing a test_create_repo_bare (was Re: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-18T17:06:28Z","receivedAt":"2014-12-18T17:06:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>\n>> The issue is if some existing tests will be helped, if we had such a\n>> helper.\n>\n> Since both bin-wrappers/git and test-lib.sh set GIT_TEMPLATE_DIR and\n> templates/blt doesn't contain any enabled hooks, I don't see how such\n> a helper would be useful.\n\nProbably bin-wrappers/git has gained it after test_setup_repo\nprotected itself manually, and I obviously forgot about it. I agree\nwith your conclusion below, if test_create_repo has become a no-op\nthese days. Thanks.\n\n> If making things more consistent were worth the churn, then if\n> anything it would make sense to make test_create_repo private to the\n> setup code in test-lib.sh and to use plain 'git init ' in\n> tests.\n"}]}