{"thread":{"id":"18968","subject":"Bug in test-lib.sh: test_create_repo() / RFC","startedAt":"2009-04-20T14:51:18Z","lastAt":"2009-04-21T09:29:10Z","messageCount":7,"participants":["Michael J Gruber","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"111745","messageId":"49EC8BE6.2030709@drmicha.warpmail.net","threadId":"18968","inReplyTo":null,"subject":"Bug in test-lib.sh: test_create_repo() / RFC","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-04-20T14:51:18Z","receivedAt":"2009-04-20T14:51:18Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Hi there,\n\nrunning the test suite with -v for the upcoming release exposed a\ncertain problem with test_create_repo() whose consequences I can't quite\nfathom at the moment. That means: I don't know whether it's maint\nmaterial or forbidden fruits during rc-cycle...\n\nProblem:\nSince a6d63b7 (test-lib: avoid assuming that templates/ are in the\nGIT_EXEC_PATH, 2009-02-04), test_create_repo() assumes to be called from\na directory such that `pwd`/../templates/blt/ contains templates for\ngit-init.\n\nSeveral tests (see below) call test_create_repo() from a different\ndirectory, which means the repo is created without any of the default\nfiles (and that a mv .git/hooks .git/hooks-disabled later in the\nfunction errors out). Now, for most tests this probably doesn't matter\nat all but it's not nice.\n\nRFC:\nI see several possible solutions:\n\n- Make sure all tests use test_create_repo() from t/. Cumbersome and\nfragile.\n\n- Simply use $(TEST_DIRECTORY)/../templates/blt/. Nice and easy. But\nuses the templates from the git repo containing t/ even when testing\nagainst and installed git (just like now, for most of the tests).\n\n- Teach git a \"--templates-dir\" option similar to \"--html-path\" and use\nthat (from the git actually being tested). Means we use the templates\nbelonging to the tested git; but also means we can test only git\nversions containing that new option.\n\nWhat do you think?\n\nMichael\n\nAffected tests:\nt0050-filesystem.sh\nt1007-hash-object.sh\nt1302-repo-version.sh\nt2103-update-index-ignore-missing.sh\nt4027-diff-submodule.sh\nt5300-pack-object.sh\nt5513-fetch-track.sh\nt5600-clone-fail-cleanup.sh\nt5601-clone.sh\nt5700-clone-reference.sh\nt5710-info-alternate.sh\nt6026-merge-attr.sh\nt7001-mv.sh\nt7010-setup.sh\nt7401-submodule-summary.sh\nt7506-status-submodule.sh\nt7508-status.sh\n"},{"id":"111828","messageId":"20090421030638.GE14479@coredump.intra.peff.net","threadId":"18968","inReplyTo":"49EC8BE6.2030709@drmicha.warpmail.net","subject":"Re: Bug in test-lib.sh: test_create_repo() / RFC","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-21T03:06:38Z","receivedAt":"2009-04-21T03:06:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 20, 2009 at 04:51:18PM +0200, Michael J Gruber wrote:\n\n> Problem:\n> Since a6d63b7 (test-lib: avoid assuming that templates/ are in the\n> GIT_EXEC_PATH, 2009-02-04), test_create_repo() assumes to be called from\n> a directory such that `pwd`/../templates/blt/ contains templates for\n> git-init.\n> \n> Several tests (see below) call test_create_repo() from a different\n> directory, which means the repo is created without any of the default\n> files (and that a mv .git/hooks .git/hooks-disabled later in the\n> function errors out). Now, for most tests this probably doesn't matter\n> at all but it's not nice.\n\nIf I am understanding it correctly, I think this is simply a bug\nintroduced by a6d63b7, and the fix is maint-worthy.\n\nAnd I think the right fix is:\n\n> - Simply use $(TEST_DIRECTORY)/../templates/blt/. Nice and easy. But\n> uses the templates from the git repo containing t/ even when testing\n> against and installed git (just like now, for most of the tests).\n\nThe original code (before a6d63b7) used $(GIT_EXEC_PATH), since it\nhandily pointed to \"$(TEST_DIRECTORY)/..\" (and I didn't check, but I\nsuspect that line predates $(TEST_DIRECTORY) entirely). So I think this\nis just doing what the a6d63b7 should have done in the first place,\ninstead of using `pwd` to guess at the top-level location.\n\nOne thing to consider: we now have code to test an installed version of\ngit. Should it be using the vanilla hooks from the source directory, or\nshould it be using the regular templates? I think that question is\northogonal to the bug you mention, though -- the current code is trying\nto use the hooks from the source directory, but it is just failing to do\nso; by fixing it, you certainly won't be breaking the\ntest-installed-version case.\n\n-Peff\n"},{"id":"111837","messageId":"7vocuqbmgu.fsf@gitster.siamese.dyndns.org","threadId":"18968","inReplyTo":"20090421030638.GE14479@coredump.intra.peff.net","subject":"Re: Bug in test-lib.sh: test_create_repo() / RFC","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-21T05:32:49Z","receivedAt":"2009-04-21T05:32:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Apr 20, 2009 at 04:51:18PM +0200, Michael J Gruber wrote:\n>\n>> Problem:\n>> Since a6d63b7 (test-lib: avoid assuming that templates/ are in the\n>> GIT_EXEC_PATH, 2009-02-04), test_create_repo() assumes to be called from\n>> a directory such that `pwd`/../templates/blt/ contains templates for\n>> git-init.\n>> \n>> Several tests (see below) call test_create_repo() from a different\n>> directory, which means the repo is created without any of the default\n>> files (and that a mv .git/hooks .git/hooks-disabled later in the\n>> function errors out). Now, for most tests this probably doesn't matter\n>> at all but it's not nice.\n>\n> If I am understanding it correctly, I think this is simply a bug\n> introduced by a6d63b7, and the fix is maint-worthy.\n>\n> And I think the right fix is:\n>\n>> - Simply use $(TEST_DIRECTORY)/../templates/blt/. Nice and easy. But\n>> uses the templates from the git repo containing t/ even when testing\n>> against and installed git (just like now, for most of the tests).\n>\n> The original code (before a6d63b7) used $(GIT_EXEC_PATH), since it\n> handily pointed to \"$(TEST_DIRECTORY)/..\" (and I didn't check, but I\n> suspect that line predates $(TEST_DIRECTORY) entirely). So I think this\n> is just doing what the a6d63b7 should have done in the first place,\n> instead of using `pwd` to guess at the top-level location.\n>\n> One thing to consider: we now have code to test an installed version of\n> git. Should it be using the vanilla hooks from the source directory, or\n> should it be using the regular templates? I think that question is\n> orthogonal to the bug you mention, though -- the current code is trying\n> to use the hooks from the source directory, but it is just failing to do\n> so; by fixing it, you certainly won't be breaking the\n> test-installed-version case.\n\nYup, I think that is the right approach.\n"},{"id":"111862","messageId":"1240305543-27163-1-git-send-email-git@drmicha.warpmail.net","threadId":"18968","inReplyTo":"7vocuqbmgu.fsf@gitster.siamese.dyndns.org","subject":"[PATCH-maint] test-lib.sh: Help test_create_repo() find the templates dir","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-04-21T09:19:03Z","receivedAt":"2009-04-21T09:19:03Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, test_create_repo() expects that templates can be found below\n$GIT_EXEC_DIR. This assumption fails when tests are run against a git\ninstalled somewhere else.\nTherefore, use $TEST_DIRECTORY as introduced in 2d84e9fb and expect\ntemplates to be present in $TEST_DIRECTORY/.. which should be the root\ndir of the git checkout.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nThis is a patch against maint as discussed in\nhttp://article.gmane.org/gmane.comp.version-control.git/11699 although\nmaint does not (yet) contain a6d63b7 which actually exposes the problem.\n\nThe patch does not apply cleanly to master and needs a different commit\nmessage there anyways. Therefore, I will send a follow-up with a patch\nagainst master. I suggest considering this \"maint for master\", i.e.\napplying to master where the bug is actually exposed.\n\n t/test-lib.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 59d82d2..dbce26a 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -434,7 +434,7 @@ test_create_repo () {\n \trepo=\"$1\"\n \tmkdir -p \"$repo\"\n \tcd \"$repo\" || error \"Cannot setup test environment\"\n-\t\"$GIT_EXEC_PATH/git\" init \"--template=$GIT_EXEC_PATH/templates/blt/\" >&3 2>&4 ||\n+\t\"$GIT_EXEC_PATH/git\" init \"--template=$TEST_DIRECTORY/../templates/blt/\" >&3 2>&4 ||\n \terror \"cannot run git init -- have you built things yet?\"\n \tmv .git/hooks .git/hooks-disabled\n \tcd \"$owd\"\n-- \n1.6.3.rc0.201.g3759d\n"},{"id":"111863","messageId":"1240305719-27224-1-git-send-email-git@drmicha.warpmail.net","threadId":"18968","inReplyTo":"1240305543-27163-1-git-send-email-git@drmicha.warpmail.net","subject":"[PATCH-master] test-lib.sh: Help test_create_repo() find the templates dir","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-04-21T09:21:59Z","receivedAt":"2009-04-21T09:21:59Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, test_create_repo() expects that templates can be found below\n`pwd`/.. This assumption fails when tests are run against a git\ninstalled somewhere else or test_create_repo() is called from\nsubdirectiories (several tests do this).\nTherefore, use $TEST_DIRECTORY as introduced in 2d84e9fb and expect\ntemplates to be present in $TEST_DIRECTORY/.. which should be the root\ndir of the git checkout.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n t/test-lib.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 4bd986f..dad1437 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -491,7 +491,7 @@ test_create_repo () {\n \trepo=\"$1\"\n \tmkdir -p \"$repo\"\n \tcd \"$repo\" || error \"Cannot setup test environment\"\n-\t\"$GIT_EXEC_PATH/git-init\" \"--template=$owd/../templates/blt/\" >&3 2>&4 ||\n+\t\"$GIT_EXEC_PATH/git-init\" \"--template=$TEST_DIRECTORY/../templates/blt/\" >&3 2>&4 ||\n \terror \"cannot run git init -- have you built things yet?\"\n \tmv .git/hooks .git/hooks-disabled\n \tcd \"$owd\"\n-- \n1.6.3.rc0.201.g3759d\n"},{"id":"111864","messageId":"alpine.DEB.1.00.0904211126180.10279@pacific.mpi-cbg.de","threadId":"18968","inReplyTo":"1240305543-27163-1-git-send-email-git@drmicha.warpmail.net","subject":"Re: [PATCH-maint] test-lib.sh: Help test_create_repo() find the templates dir","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-04-21T09:26:38Z","receivedAt":"2009-04-21T09:26:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 21 Apr 2009, Michael J Gruber wrote:\n\n> This is a patch against maint as discussed in \n> http://article.gmane.org/gmane.comp.version-control.git/11699 although \n> maint does not (yet) contain a6d63b7 which actually exposes the problem.\n\nSo it is a fix in need of a problem?\n\nCiao,\nDscho\n"},{"id":"111865","messageId":"49ED91E6.5060303@drmicha.warpmail.net","threadId":"18968","inReplyTo":"alpine.DEB.1.00.0904211126180.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH-maint] test-lib.sh: Help test_create_repo() find the templates dir","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-04-21T09:29:10Z","receivedAt":"2009-04-21T09:29:10Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Johannes Schindelin venit, vidit, dixit 21.04.2009 11:26:\n> Hi,\n> \n> On Tue, 21 Apr 2009, Michael J Gruber wrote:\n> \n>> This is a patch against maint as discussed in \n>> http://article.gmane.org/gmane.comp.version-control.git/11699 although \n>> maint does not (yet) contain a6d63b7 which actually exposes the problem.\n> \n> So it is a fix in need of a problem?\n\nYour question is answered by the second paragraph which I wrote and you\ncut... I suggested applying to master only. (Admittedly, the word \"only\"\ndoes not appear there.)\n\nCheers,\nMichael\n"}]}