[PATCH] Configure test for FREAD_READS_DIRECTORIES

[PATCH] Configure test for FREAD_READS_DIRECTORIES

[Michal Rokos]

Hello,

this patch adds missing tests for FREAD_READS_DIRECTORIES.

Signed-off-by: Michal Rokos <michal.rokos@nextsoft.cz>

diff --git a/Makefile b/Makefile
index ca5aad9..344ab49 100644
--- a/Makefile
+++ b/Makefile
@@ -526,6 +526,7 @@ ifeq ($(uname_S),HP-UX)
 	NO_UNSETENV = YesPlease
 	NO_HSTRERROR = YesPlease
 	NO_SYS_SELECT_H = YesPlease
+	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 endif
 ifneq (,$(findstring arm,$(uname_M)))
 	ARM_SHA1 = YesPlease
diff --git a/config.mak.in b/config.mak.in
index ee6c33d..516c468 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -46,3 +46,4 @@ NO_MKDTEMP=@NO_MKDTEMP@
 NO_ICONV=@NO_ICONV@
 OLD_ICONV=@OLD_ICONV@
 NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
+FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@
diff --git a/configure.ac b/configure.ac
index 85d7ef5..0ac28f6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -326,6 +326,27 @@ else
 	NO_C99_FORMAT=
 fi
 AC_SUBST(NO_C99_FORMAT)
+#
+# Define FREAD_READS_DIRECTORIES if your are on a system which succeeds
+# when attempting to read from an fopen'ed directory.
+AC_CACHE_CHECK([whether system succeeds to read fopen'ed directory],
+ [ac_cv_fread_reads_directories],
+[
+AC_RUN_IFELSE(
+	[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],
+		[[char c;
+		FILE *f = fopen("/etc", "r");
+		if (! f) return 0;
+		if (f && fread(&c, 1, 1, f) > 0) return 1]])],
+	[ac_cv_fread_reads_directories=no],
+	[ac_cv_fread_reads_directories=yes])
+])
+if test $ac_cv_fread_reads_directories = yes; then
+	FREAD_READS_DIRECTORIES=UnfortunatelyYes
+else
+	FREAD_READS_DIRECTORIES=
+fi
+AC_SUBST(FREAD_READS_DIRECTORIES)
 
 
 ## Checks for library functions.

Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES

[Junio C Hamano]

Michal Rokos <michal.rokos@nextsoft.cz> writes:

> Hello,

"Hello," not wanted in the commit log message.

> this patch adds missing tests for FREAD_READS_DIRECTORIES.
>
> Signed-off-by: Michal Rokos <michal.rokos@nextsoft.cz>

> +#
> +# Define FREAD_READS_DIRECTORIES if your are on a system which succeeds
> +# when attempting to read from an fopen'ed directory.
> +AC_CACHE_CHECK([whether system succeeds to read fopen'ed directory],
> + [ac_cv_fread_reads_directories],
> +[
> +AC_RUN_IFELSE(
> +	[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],
> +		[[char c;
> +		FILE *f = fopen("/etc", "r");

Why "/etc" and not "." I have to wonder...

On how many different platforms was this configure check tested on?

Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES

[Michal Rokos]

Hello,

On Tuesday 04 March 2008 12:03:12 Junio C Hamano wrote:
> > +#
> > +# Define FREAD_READS_DIRECTORIES if your are on a system which succeeds
> > +# when attempting to read from an fopen'ed directory.
> > +AC_CACHE_CHECK([whether system succeeds to read fopen'ed directory],
> > + [ac_cv_fread_reads_directories],
> > +[
> > +AC_RUN_IFELSE(
> > +	[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],
> > +		[[char c;
> > +		FILE *f = fopen("/etc", "r");
>
> Why "/etc" and not "." I have to wonder...

I think "." is brilliant - ie no reason for "/etc" apart that I'm dumb.

> On how many different platforms was this configure check tested on?

Test works on Linux (no FREAD_READS_DIRECTORIES) and HPUXes 
(FREAD_READS_DIRECTORIES): HP-UX B.11.23 ia64 (Itanium) and HP-UX B.11.11 
9000/800 (PaRisc)

Do you want me to resend with "."?

MR
-- 
Michal Rokos

NextSoft s.r.o.
Vyskočilova 1/1410
140 21 Praha 4
phone:  +420 267 224 311
fax:    +420 267 224 307
mobile: +420 736 646 591
e-mail: michal.rokos@nextsoft.cz

Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES

[Junio C Hamano]

Michal Rokos <michal.rokos@nextsoft.cz> writes:

>> On how many different platforms was this configure check tested on?
>
> Test works on Linux (no FREAD_READS_DIRECTORIES) and HPUXes 
> (FREAD_READS_DIRECTORIES): HP-UX B.11.23 ia64 (Itanium) and HP-UX B.11.11 
> 9000/800 (PaRisc)
>
> Do you want me to resend with "."?

Probably resending with "." and asking the list audiences for help in
testing would help you gather success reports on different platforms.

Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES

[Michal Rokos]

Hello,

On Tuesday 04 March 2008 12:32:57 Junio C Hamano wrote:
> Michal Rokos <michal.rokos@nextsoft.cz> writes:
> >> On how many different platforms was this configure check tested on?
> >
> > Test works on Linux (no FREAD_READS_DIRECTORIES) and HPUXes
> > (FREAD_READS_DIRECTORIES): HP-UX B.11.23 ia64 (Itanium) and HP-UX B.11.11
> > 9000/800 (PaRisc)
> >
> > Do you want me to resend with "."?
>
> Probably resending with "." and asking the list audiences for help in
> testing would help you gather success reports on different platforms.

Will do... Did that.

Do you think that there's some reason not-to merge it? I mean if fopen(".") 
throws an error, FREAD_READS_DIRECTORIES will NOT be defined - as is it now.

I don't know how many people cares about configure script since there are 
missing bits in it again and again. I believe it could receive good amount of 
testing only when it's merged in.

I'm trying to make GIT working on HPUX - next patch in my queue is about 
broken vsnprintf() that returns -1 on maxsize overrun. Do you think that it's 
more likely that patch will be accepted when I omit "broken vsnprintf()" 
detection code from configure.ac?

MR

-- 
Michal Rokos

NextSoft s.r.o.
Vyskočilova 1/1410
140 21 Praha 4
phone:  +420 267 224 311
fax:    +420 267 224 307
mobile: +420 736 646 591
e-mail: michal.rokos@nextsoft.cz

Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES

[Jakub Narebski]

Michal Rokos <michal.rokos@nextsoft.cz> writes:
 
> I don't know how many people care about configure script since
> there are missing bits in it again and again. I believe it could
> receive good amount of testing only when it's merged in.

Because configure script is optional, people do tend to forget to add
test to it, when adding new compile configuration option.
Configuration is mainly done by guessing based on uname.

Unfortunately we don't have maintainer for configure script, who would
catch new make configuration options, and add appropriate tests to
./configure.

> I'm trying to make GIT working on HPUX - next patch in my queue is
> about broken vsnprintf() that returns -1 on maxsize overrun. Do you
> think that it's more likely that patch will be accepted when I omit
> "broken vsnprintf()" detection code from configure.ac?

I think it would be better to split patch into two: one adding build
option, or setting it for given operating system or operating system
version, and one adding test to ./configure script.  It is much
simplier to test first patch; the patch to configure needs more
review, as it should work correctly on all operating systems.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] Configure test for FREAD_READS_DIRECTORIES

[Junio C Hamano]

Michal Rokos <michal.rokos@nextsoft.cz> writes:

> Will do... Did that.
>
> Do you think that there's some reason not-to merge it?

Yes, if you meant "apply as-is" by "merge it".  No, if you meant "apply
after an initial round of sanity checks, even if it is not perfect".

I was hoping that with this approach, in a week after you sent
out your call-for-help-in-testing, you could send a version for
inclusion with a commit log message that says "tested on X (by
Foo), Y (by Bar),...", with the patch text that is exactly the
same as what people tested.  The point is not to make that list
of platforms exhaustive, but at least make it a bit more than
"works for me".

And I think that plan has worked well.