{"thread":{"id":"12488","subject":"[PATCH] Configure test for FREAD_READS_DIRECTORIES","startedAt":"2008-03-04T09:48:53Z","lastAt":"2008-03-05T17:57:22Z","messageCount":7,"participants":["Michal Rokos","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"70914","messageId":"200803041048.53399.michal.rokos@nextsoft.cz","threadId":"12488","inReplyTo":null,"subject":"[PATCH] Configure test for FREAD_READS_DIRECTORIES","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-04T09:48:53Z","receivedAt":"2008-03-04T09:48:53Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hello,\n\nthis patch adds missing tests for FREAD_READS_DIRECTORIES.\n\nSigned-off-by: Michal Rokos <michal.rokos@nextsoft.cz>\n\ndiff --git a/Makefile b/Makefile\nindex ca5aad9..344ab49 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -526,6 +526,7 @@ ifeq ($(uname_S),HP-UX)\n \tNO_UNSETENV = YesPlease\n \tNO_HSTRERROR = YesPlease\n \tNO_SYS_SELECT_H = YesPlease\n+\tFREAD_READS_DIRECTORIES = UnfortunatelyYes\n endif\n ifneq (,$(findstring arm,$(uname_M)))\n \tARM_SHA1 = YesPlease\ndiff --git a/config.mak.in b/config.mak.in\nindex ee6c33d..516c468 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -46,3 +46,4 @@ NO_MKDTEMP=@NO_MKDTEMP@\n NO_ICONV=@NO_ICONV@\n OLD_ICONV=@OLD_ICONV@\n NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@\n+FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@\ndiff --git a/configure.ac b/configure.ac\nindex 85d7ef5..0ac28f6 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -326,6 +326,27 @@ else\n \tNO_C99_FORMAT=\n fi\n AC_SUBST(NO_C99_FORMAT)\n+#\n+# Define FREAD_READS_DIRECTORIES if your are on a system which succeeds\n+# when attempting to read from an fopen'ed directory.\n+AC_CACHE_CHECK([whether system succeeds to read fopen'ed directory],\n+ [ac_cv_fread_reads_directories],\n+[\n+AC_RUN_IFELSE(\n+\t[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],\n+\t\t[[char c;\n+\t\tFILE *f = fopen(\"/etc\", \"r\");\n+\t\tif (! f) return 0;\n+\t\tif (f && fread(&c, 1, 1, f) > 0) return 1]])],\n+\t[ac_cv_fread_reads_directories=no],\n+\t[ac_cv_fread_reads_directories=yes])\n+])\n+if test $ac_cv_fread_reads_directories = yes; then\n+\tFREAD_READS_DIRECTORIES=UnfortunatelyYes\n+else\n+\tFREAD_READS_DIRECTORIES=\n+fi\n+AC_SUBST(FREAD_READS_DIRECTORIES)\n \n \n ## Checks for library functions.\n"},{"id":"70918","messageId":"7vod9u92fj.fsf@gitster.siamese.dyndns.org","threadId":"12488","inReplyTo":"200803041048.53399.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-04T11:03:12Z","receivedAt":"2008-03-04T11:03:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Rokos <michal.rokos@nextsoft.cz> writes:\n\n> Hello,\n\n\"Hello,\" not wanted in the commit log message.\n\n> this patch adds missing tests for FREAD_READS_DIRECTORIES.\n>\n> Signed-off-by: Michal Rokos <michal.rokos@nextsoft.cz>\n\n> +#\n> +# Define FREAD_READS_DIRECTORIES if your are on a system which succeeds\n> +# when attempting to read from an fopen'ed directory.\n> +AC_CACHE_CHECK([whether system succeeds to read fopen'ed directory],\n> + [ac_cv_fread_reads_directories],\n> +[\n> +AC_RUN_IFELSE(\n> +\t[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],\n> +\t\t[[char c;\n> +\t\tFILE *f = fopen(\"/etc\", \"r\");\n\nWhy \"/etc\" and not \".\" I have to wonder...\n\nOn how many different platforms was this configure check tested on?\n"},{"id":"70920","messageId":"200803041217.37027.michal.rokos@nextsoft.cz","threadId":"12488","inReplyTo":"7vod9u92fj.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-04T11:17:36Z","receivedAt":"2008-03-04T11:17:36Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hello,\n\nOn Tuesday 04 March 2008 12:03:12 Junio C Hamano wrote:\n> > +#\n> > +# Define FREAD_READS_DIRECTORIES if your are on a system which succeeds\n> > +# when attempting to read from an fopen'ed directory.\n> > +AC_CACHE_CHECK([whether system succeeds to read fopen'ed directory],\n> > + [ac_cv_fread_reads_directories],\n> > +[\n> > +AC_RUN_IFELSE(\n> > +\t[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],\n> > +\t\t[[char c;\n> > +\t\tFILE *f = fopen(\"/etc\", \"r\");\n>\n> Why \"/etc\" and not \".\" I have to wonder...\n\nI think \".\" is brilliant - ie no reason for \"/etc\" apart that I'm dumb.\n\n> On how many different platforms was this configure check tested on?\n\nTest works on Linux (no FREAD_READS_DIRECTORIES) and HPUXes \n(FREAD_READS_DIRECTORIES): HP-UX B.11.23 ia64 (Itanium) and HP-UX B.11.11 \n9000/800 (PaRisc)\n\nDo you want me to resend with \".\"?\n\nMR\n-- \nMichal Rokos\n\nNextSoft s.r.o.\nVyskočilova 1/1410\n140 21 Praha 4\nphone:  +420 267 224 311\nfax:    +420 267 224 307\nmobile: +420 736 646 591\ne-mail: michal.rokos@nextsoft.cz\n"},{"id":"70924","messageId":"7v7igi911y.fsf@gitster.siamese.dyndns.org","threadId":"12488","inReplyTo":"200803041217.37027.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-04T11:32:57Z","receivedAt":"2008-03-04T11:32:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Rokos <michal.rokos@nextsoft.cz> writes:\n\n>> On how many different platforms was this configure check tested on?\n>\n> Test works on Linux (no FREAD_READS_DIRECTORIES) and HPUXes \n> (FREAD_READS_DIRECTORIES): HP-UX B.11.23 ia64 (Itanium) and HP-UX B.11.11 \n> 9000/800 (PaRisc)\n>\n> Do you want me to resend with \".\"?\n\nProbably resending with \".\" and asking the list audiences for help in\ntesting would help you gather success reports on different platforms.\n"},{"id":"70927","messageId":"200803041248.54197.michal.rokos@nextsoft.cz","threadId":"12488","inReplyTo":"7v7igi911y.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES","fromName":"Michal Rokos","fromEmail":"michal.rokos@nextsoft.cz","sentAt":"2008-03-04T11:48:53Z","receivedAt":"2008-03-04T11:48:53Z","isPatch":true,"sender":{"key":"michal.rokos@nextsoft.cz","avatar":null},"body":"Hello,\n\nOn Tuesday 04 March 2008 12:32:57 Junio C Hamano wrote:\n> Michal Rokos <michal.rokos@nextsoft.cz> writes:\n> >> On how many different platforms was this configure check tested on?\n> >\n> > Test works on Linux (no FREAD_READS_DIRECTORIES) and HPUXes\n> > (FREAD_READS_DIRECTORIES): HP-UX B.11.23 ia64 (Itanium) and HP-UX B.11.11\n> > 9000/800 (PaRisc)\n> >\n> > Do you want me to resend with \".\"?\n>\n> Probably resending with \".\" and asking the list audiences for help in\n> testing would help you gather success reports on different platforms.\n\nWill do... Did that.\n\nDo you think that there's some reason not-to merge it? I mean if fopen(\".\") \nthrows an error, FREAD_READS_DIRECTORIES will NOT be defined - as is it now.\n\nI don't know how many people cares about configure script since there are \nmissing bits in it again and again. I believe it could receive good amount of \ntesting only when it's merged in.\n\nI'm trying to make GIT working on HPUX - next patch in my queue is about \nbroken vsnprintf() that returns -1 on maxsize overrun. Do you think that it's \nmore likely that patch will be accepted when I omit \"broken vsnprintf()\" \ndetection code from configure.ac?\n\nMR\n\n-- \nMichal Rokos\n\nNextSoft s.r.o.\nVyskočilova 1/1410\n140 21 Praha 4\nphone:  +420 267 224 311\nfax:    +420 267 224 307\nmobile: +420 736 646 591\ne-mail: michal.rokos@nextsoft.cz\n"},{"id":"70936","messageId":"m3fxv6isxw.fsf@localhost.localdomain","threadId":"12488","inReplyTo":"200803041248.54197.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-03-04T12:17:50Z","receivedAt":"2008-03-04T12:17:50Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Michal Rokos <michal.rokos@nextsoft.cz> writes:\n \n> I don't know how many people care about configure script since\n> there are missing bits in it again and again. I believe it could\n> receive good amount of testing only when it's merged in.\n\nBecause configure script is optional, people do tend to forget to add\ntest to it, when adding new compile configuration option.\nConfiguration is mainly done by guessing based on uname.\n\nUnfortunately we don't have maintainer for configure script, who would\ncatch new make configuration options, and add appropriate tests to\n./configure.\n\n> I'm trying to make GIT working on HPUX - next patch in my queue is\n> about broken vsnprintf() that returns -1 on maxsize overrun. Do you\n> think that it's more likely that patch will be accepted when I omit\n> \"broken vsnprintf()\" detection code from configure.ac?\n\nI think it would be better to split patch into two: one adding build\noption, or setting it for given operating system or operating system\nversion, and one adding test to ./configure script.  It is much\nsimplier to test first patch; the patch to configure needs more\nreview, as it should work correctly on all operating systems.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"71103","messageId":"7vd4q9njel.fsf@gitster.siamese.dyndns.org","threadId":"12488","inReplyTo":"200803041248.54197.michal.rokos@nextsoft.cz","subject":"Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-05T17:57:22Z","receivedAt":"2008-03-05T17:57:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Rokos <michal.rokos@nextsoft.cz> writes:\n\n> Will do... Did that.\n>\n> Do you think that there's some reason not-to merge it?\n\nYes, if you meant \"apply as-is\" by \"merge it\".  No, if you meant \"apply\nafter an initial round of sanity checks, even if it is not perfect\".\n\nI was hoping that with this approach, in a week after you sent\nout your call-for-help-in-testing, you could send a version for\ninclusion with a commit log message that says \"tested on X (by\nFoo), Y (by Bar),...\", with the patch text that is exactly the\nsame as what people tested.  The point is not to make that list\nof platforms exhaustive, but at least make it a bit more than\n\"works for me\".\n\nAnd I think that plan has worked well.\n"}]}