{"thread":{"id":"2779","subject":"[PATCH] use \"git init-db\" in tests","startedAt":"2005-12-08T20:25:55Z","lastAt":"2005-12-09T20:46:51Z","messageCount":11,"participants":["Alex Riesen","Junio C Hamano","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"13361","messageId":"20051208202555.GA3046@steel.home","threadId":"2779","inReplyTo":null,"subject":"[PATCH] use \"git init-db\" in tests","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-12-08T20:25:55Z","receivedAt":"2005-12-08T20:25:55Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Unless there was a special reason to use git-init-db from $PATH, could\nthe patch below be accepted? It makes the test suite use git-init-db\nfrom the \"the binaries we have just built\".\n\nThe reason, why we need the patch even if the $PATH is set, will be\nbetter explained by glibc's sources: the lookup does not stop if the\nfile exists but has no execute permission.\n\nSee file posix/execvp.c, the loop iterating over elements in $PATH:\n\n switch (errno)\n {\n case EACCES:\n /* Record the we got a `Permission denied' error. If we end\n up finding no executable we can use, we want to diagnose\n that we did find one but were denied access. */\n got_eacces = true;\n ...\n break;\n ...\n }\n ...\n /* We tried every element and none of them worked. */\n if (got_eacces)\n /* At least one failure was due to permissions, so report that\n error. */\n __set_errno (EACCES);\n\nSigned-off-by: Alex Riesen \n\n---\n\n By the same reason, the whole testsuite should be reviewed to use \n \"git \" notation.\n\n t/test-lib.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\n4417503d10870c913cff4f635acf274b7da75864\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0539dac..3704e5f 100755\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -190,8 +190,8 @@ test=trash\n rm -fr \"$test\"\n mkdir \"$test\"\n cd \"$test\"\n-git-init-db --template=../../templates/blt/ 2>/dev/null ||\n-error \"cannot run git-init-db\"\n+git init-db --template=../../templates/blt/ 2>/dev/null ||\n+error \"cannot run git init-db\"\n \n mv .git/hooks .git/hooks-disabled\n \n-- \n0.99.9.GIT\n"},{"id":"13362","messageId":"7vu0dje2oi.fsf@assigned-by-dhcp.cox.net","threadId":"2779","inReplyTo":"20051208202555.GA3046@steel.home","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-08T20:47:41Z","receivedAt":"2005-12-08T20:47:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Unless there was a special reason to use git-init-db from $PATH, could\n> the patch below be accepted? It makes the test suite use git-init-db\n> from the \"the binaries we have just built\".\n\nA few lines above what you patched we do make sure the PATH\nstarts with the top of the build directory where you would find\ngit-init-db we have just built, so unless I am missing something...\n"},{"id":"13363","messageId":"20051208205941.GA19423@steel.home","threadId":"2779","inReplyTo":"20051208202555.GA3046@steel.home","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-12-08T20:59:41Z","receivedAt":"2005-12-08T20:59:41Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Alex Riesen, Thu, Dec 08, 2005 21:25:55 +0100:\n> -git-init-db --template=../../templates/blt/ 2>/dev/null ||\n> -error \"cannot run git-init-db\"\n> +git init-db --template=../../templates/blt/ 2>/dev/null ||\n> +error \"cannot run git init-db\"\n\n\"$GIT_EXEC_PATH/git init-db\", of course.\n\nSigned-off-by: Alex Riesen \n\n---\n\n t/test-lib.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\n33dbb3dacf1b38c40f091dd800ea2dafdfa1fd3f\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 0539dac..777c748 100755\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -190,8 +190,8 @@ test=trash\n rm -fr \"$test\"\n mkdir \"$test\"\n cd \"$test\"\n-git-init-db --template=../../templates/blt/ 2>/dev/null ||\n-error \"cannot run git-init-db\"\n+\"$GIT_EXEC_PATH/git\" init-db --template=../../templates/blt/ 2>/dev/null ||\n+error \"cannot run $GIT_EXEC_PATH/git init-db\"\n \n mv .git/hooks .git/hooks-disabled\n \n-- \n0.99.9.GIT\n"},{"id":"13365","messageId":"20051208210251.GB19423@steel.home","threadId":"2779","inReplyTo":"7vu0dje2oi.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-12-08T21:02:51Z","receivedAt":"2005-12-08T21:02:51Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Thu, Dec 08, 2005 21:47:41 +0100:\n> \n> > Unless there was a special reason to use git-init-db from $PATH, could\n> > the patch below be accepted? It makes the test suite use git-init-db\n> > from the \"the binaries we have just built\".\n> \n> A few lines above what you patched we do make sure the PATH\n> starts with the top of the build directory where you would find\n> git-init-db we have just built, so unless I am missing something...\n> \n\nYou do miss something. glibc will happily continue lookup if\ngit-init-db in the top of the build directory is not executable, and\nit will take the next one it finds (and people _will_ have git-init-db\nin PATH).\n"},{"id":"13366","messageId":"7v7jafcmev.fsf@assigned-by-dhcp.cox.net","threadId":"2779","inReplyTo":"20051208210251.GB19423@steel.home","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-08T21:24:24Z","receivedAt":"2005-12-08T21:24:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> You do miss something. glibc will happily continue lookup if\n> git-init-db in the top of the build directory is not executable, and\n> it will take the next one it finds (and people _will_ have git-init-db\n> in PATH).\n\nAnd the reason git-init-db we just built is not executable\nis...?\n"},{"id":"13406","messageId":"81b0412b0512082336i674932bapd631d559e80cad79@mail.gmail.com","threadId":"2779","inReplyTo":"7v7jafcmev.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-12-09T07:36:33Z","receivedAt":"2005-12-09T07:36:33Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 12/8/05, Junio C Hamano wrote:\n> > You do miss something. glibc will happily continue lookup if\n> > git-init-db in the top of the build directory is not executable, and\n> > it will take the next one it finds (and people _will_ have git-init-db\n> > in PATH).\n>\n> And the reason git-init-db we just built is not executable\n> is...?\n\nAn accident? Like a filesystem not supporting executable permission?\nWhat is the reason to report success from the test run in that conditions?\n"},{"id":"13407","messageId":"7vlkyu7l05.fsf@assigned-by-dhcp.cox.net","threadId":"2779","inReplyTo":"81b0412b0512082336i674932bapd631d559e80cad79@mail.gmail.com","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-09T08:06:02Z","receivedAt":"2005-12-09T08:06:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> An accident? Like a filesystem not supporting executable permission?\n> What is the reason to report success from the test run in that conditions?\n\nLet's be reasonable. I was hoping to hear from you a real-world\nbreakage case that I overlooked due to my lack of access to\nplatforms you may have access to. I am not interested in a\ntheoretical failure case discussion very much. If your\nfilesystem does not support executables, why do you expect\nthings to run from the freshly built directory to begin with?\n\nLinkage error of git-init-db (or git wrapper) may leave the file\ncreated but leave that in unexecutable form, which could be a\nvalid concern, but that would signal an error to the make during\nthe build stage, and \"test\" target depends on \"all\" target.\n\nAnd please do not start arguing that you can cd to 't' directory\nafter such a build failure and manually say \"make\". You can do\nthat without even running make at the top level and cause the\nsame failure. I consider both of them pilot errors.\n"},{"id":"13408","messageId":"7vvexy4ppw.fsf@assigned-by-dhcp.cox.net","threadId":"2779","inReplyTo":"7vlkyu7l05.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-09T08:52:27Z","receivedAt":"2005-12-09T08:52:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Linkage error of git-init-db (or git wrapper) may leave the file\n> created but leave that in unexecutable form, which could be a\n> valid concern, but that would signal an error to the make during\n> the build stage, and \"test\" target depends on \"all\" target.\n\nBTW, I sometimes wished if it were easier to disable that \"test:\nall\" dependency and run tests without building things first,\ni.e. deliberately using the already installed binaries, so that\nI can make sure that updated or new tests reproduce and catch\nproblems with the existing code first and then make sure the\nbinaries built from the updated sources fix the problem.\n\nOf course that is a very specialized application so a makefile\nvariable \"make NO_BUILD_BEFORE_TEST=YesPlease test\" would not\nmake much sense, but I could have done something like this:\n\n---\ndiff --git a/Makefile b/Makefile\nindex 01b6643..8dd7be7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -439,7 +439,13 @@ doc:\n \n ### Testing rules\n \n-test: all\n+ifdef NO_BUILD_BEFORE_TEST\n+test_depends = \n+else\n+test_depends = all\n+endif\n+\n+test: $(test_depends)\n \t$(MAKE) -C t/ all\n \n test-date$X: test-date.c date.o ctype.o\n"},{"id":"13409","messageId":"20051209093024.GR22159@pasky.or.cz","threadId":"2779","inReplyTo":"7vvexy4ppw.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-12-09T09:30:24Z","receivedAt":"2005-12-09T09:30:24Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Fri, Dec 09, 2005 at 09:52:27AM CET, I got a letter\nwhere Junio C Hamano said that...\n> Junio C Hamano writes:\n> \n> > Linkage error of git-init-db (or git wrapper) may leave the file\n> > created but leave that in unexecutable form, which could be a\n> > valid concern, but that would signal an error to the make during\n> > the build stage, and \"test\" target depends on \"all\" target.\n> \n> BTW, I sometimes wished if it were easier to disable that \"test:\n> all\" dependency and run tests without building things first,\n> i.e. deliberately using the already installed binaries, so that\n> I can make sure that updated or new tests reproduce and catch\n> problems with the existing code first and then make sure the\n> binaries built from the updated sources fix the problem.\n> \n> Of course that is a very specialized application so a makefile\n> variable \"make NO_BUILD_BEFORE_TEST=YesPlease test\" would not\n> make much sense, but I could have done something like this:\n\nI think having \"test-standalone\" or \"test-nobuild\" or whatever rule and\n\n\ttest: all test-nobuild\n\n\ttest-nobuild:\n\t\t...\n\nwould be much more elegant.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"},{"id":"13411","messageId":"81b0412b0512090259l5f06f6d6n5416e271af36d6a1@mail.gmail.com","threadId":"2779","inReplyTo":"7vlkyu7l05.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-12-09T10:59:21Z","receivedAt":"2005-12-09T10:59:21Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 12/9/05, Junio C Hamano wrote:\n> > An accident? Like a filesystem not supporting executable permission?\n> > What is the reason to report success from the test run in that conditions?\n>\n> Let's be reasonable. I was hoping to hear from you a real-world\n> breakage case that I overlooked due to my lack of access to\n> platforms you may have access to. I am not interested in a\n> theoretical failure case discussion very much. If your\n> filesystem does not support executables, why do you expect\n> things to run from the freshly built directory to begin with?\n\nIn my case it was the freshly build directory where a chmod 0666 * was\ndone. This directory wont rebuild (the dates are correct), and the\ntests run, as if nothing happened.\nI actually noticed only after a half an hour, that I wasn't running\nthe executables I expected.\n\n> Linkage error of git-init-db (or git wrapper) may leave the file\n> created but leave that in unexecutable form, which could be a\n> valid concern, but that would signal an error to the make during\n> the build stage, and \"test\" target depends on \"all\" target.\n>\n> And please do not start arguing that you can cd to 't' directory\n> after such a build failure and manually say \"make\". You can do\n> that without even running make at the top level and cause the\n> same failure. I consider both of them pilot errors.\n\nYes, but they are not obvious. It is also not obvious what to do,\nbecause everything seams alright.\n"},{"id":"13424","messageId":"7vmzja2e2s.fsf@assigned-by-dhcp.cox.net","threadId":"2779","inReplyTo":"81b0412b0512090259l5f06f6d6n5416e271af36d6a1@mail.gmail.com","subject":"Re: [PATCH] use \"git init-db\" in tests","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-09T20:46:51Z","receivedAt":"2005-12-09T20:46:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> In my case it was the freshly build directory where a chmod 0666 * was\n> done. This directory wont rebuild (the dates are correct), and the\n> tests run, as if nothing happened.\n\nI see. I appreciate your honesty ;-), it is a pilot error\nalright, but surely it would be nicer if we catch it.\n\nAlthough your proposed change makes it harder to implement\nmy desire to be able to run tests with installed binaries than\nit already is, I'd take the patch for now. It would catch this\nparticular pilot error.\n\nI however would think adding a dependency in t/Makefile to\nmake sure the top level is built before starting to run test a\noverkill (I think I've seen some projects to do that), so I'd\nnot go there.\n"}]}