recent commit broke src/dired.c on OSX

recent commit broke src/dired.c on OSX

[Randal L. Schwartz]

This commit

  Author: Juanma Barranquero <lekktu@gmail.com>
  Date:   Sun Jul 25 02:20:51 2010 +0200

broke the build on OSX.  I believe this is the problem:

    --- a/src/dired.c
    +++ b/src/dired.c
    @@ -66,8 +66,8 @@
     #ifndef MSDOS
     #define DIRENTRY struct direct

    -extern DIR *opendir ();
    -extern struct direct *readdir ();
    +extern DIR *opendir (char *);
    +extern struct direct *readdir (DIR *);

     #endif /* not MSDOS */
     #endif /* not SYSV_SYSTEM_DIR */

Needs to be backed out, or made more portable.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
See http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Sun, Jul 25, 2010 at 19:23, Randal L. Schwartz <merlyn@stonehenge.com> wrote:

>    -extern DIR *opendir ();
>    -extern struct direct *readdir ();
>    +extern DIR *opendir (char *);
>    +extern struct direct *readdir (DIR *);

> Needs to be backed out, or made more portable.

Sure. I'd prefer to make it more portable (that already compiles on
Windows / MinGW and Ubuntu / GCC).

What is the right prototype in your system? (You didn't attach the
error output, BTW). Could you send a patch with the relevant #ifdef?

    Juanma

Re: recent commit broke src/dired.c on OSX

[Eli Zaretskii]

> From: merlyn@stonehenge.com (Randal L. Schwartz)
> Date: Sun, 25 Jul 2010 10:23:59 -0700
> 
> 
> This commit
> 
>   Author: Juanma Barranquero <lekktu@gmail.com>
>   Date:   Sun Jul 25 02:20:51 2010 +0200
> 
> broke the build on OSX.  I believe this is the problem:
> 
>     --- a/src/dired.c
>     +++ b/src/dired.c
>     @@ -66,8 +66,8 @@
>      #ifndef MSDOS
>      #define DIRENTRY struct direct
> 
>     -extern DIR *opendir ();
>     -extern struct direct *readdir ();
>     +extern DIR *opendir (char *);
>     +extern struct direct *readdir (DIR *);
> 
>      #endif /* not MSDOS */
>      #endif /* not SYSV_SYSTEM_DIR */
> 
> Needs to be backed out, or made more portable.

Please show the prototypes of these functions on OSX.

Also, do you have <dirent.h> ?

Re: recent commit broke src/dired.c on OSX

[Randal L. Schwartz]

>>>>> "Juanma" == Juanma Barranquero <lekktu@gmail.com> writes:

Juanma> Sure. I'd prefer to make it more portable (that already compiles on
Juanma> Windows / MinGW and Ubuntu / GCC).

Juanma> What is the right prototype in your system? (You didn't attach the
Juanma> error output, BTW). Could you send a patch with the relevant
Juanma> #ifdef?

Sorry, right commit, wrong reason.  Here's the error:

    /Users/merlyn/MIRROR/emacs-GIT/lib-src/emacsclient.c:85: error:
    conflicting types for 'getcwd'
    /usr/include/unistd.h:445: error: previous declaration of 'getcwd' was
    here
    make[2]: *** [emacsclient] Error 1
    make[1]: *** [lib-src] Error 2
    make: *** [bootstrap] Error 2

So it's something with:

  -char *(getcwd) ();
  +char *(getcwd) (char *, int);

On BSD, the prototype is:

     #include <unistd.h>

     char *
     getcwd(char *buf, size_t size);

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
See http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Sun, Jul 25, 2010 at 19:40, Randal L. Schwartz <merlyn@stonehenge.com> wrote:

> Sorry, right commit, wrong reason.  Here's the error:

>     char *
>     getcwd(char *buf, size_t size);

This is already fixed on the trunk, assuming that in your build
environment HAVE_GETCWD is set.

    Juanma

Re: recent commit broke src/dired.c on OSX

[Randal L. Schwartz]

>>>>> "Juanma" == Juanma Barranquero <lekktu@gmail.com> writes:

Juanma> This is already fixed on the trunk, assuming that in your build
Juanma> environment HAVE_GETCWD is set.

I was building from the latest git.  I'll have to wait until it catches up.

Thanks.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
See http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Sun, Jul 25, 2010 at 19:58, Randal L. Schwartz <merlyn@stonehenge.com> wrote:

> I was building from the latest git.  I'll have to wait until it catches up.

Just apply the attached patch.

    Juanma



=== modified file 'lib-src/emacsclient.c'
--- lib-src/emacsclient.c	2010-07-25 00:20:51 +0000
+++ lib-src/emacsclient.c	2010-07-25 03:36:54 +0000
@@ -82,7 +82,9 @@

 \f
 char *getenv (const char *), *getwd (char *);
-char *(getcwd) (char *, int);
+#ifdef HAVE_GETCWD
+char *(getcwd) (char *, size_t);
+#endif

 #ifdef WINDOWSNT
 char *w32_getenv (char *);

Re: recent commit broke src/dired.c on OSX

[Chad Brown]

I think Randal was on to something before; on my system, it's necessary to remove the declaration of opendir() from dired.c (it conflicts with the definition in <dirent.h>).

The resulting build gives warnings:

dired.c: In function ‘Ffile_attributes’:
dired.c:1016: warning: comparison is always false due to limited range of data type
dired.c:1016: warning: comparison is always false due to limited range of data type
dired.c:1020: warning: comparison is always false due to limited range of data type
dired.c:1020: warning: comparison is always false due to limited range of data type
dired.c:1068: warning: comparison is always false due to limited range of data type
dired.c:1068: warning: comparison is always false due to limited range of data type

but seems to run.  I haven't had a chance to test it extensively yet.

It seems that Darwin's dirent.h prototype is:

DIR *opendir(const char *)

which conflicts with dired.c's unconditional (ifndef MSDOS) declaration:

extern DIR *opendir(char *)

I have to run of now and help a friend move, but I can try to dig into this a little more when I return this evening, if it will help.

Thanks,
*Chad

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Sun, Jul 25, 2010 at 22:12, Chad Brown <yandros@mit.edu> wrote:

> DIR *opendir(const char *)
>
> which conflicts with dired.c's unconditional (ifndef MSDOS) declaration:
>
> extern DIR *opendir(char *)

So, it's char* vs. const char*.

> I have to run of now and help a friend move, but I can try to dig into this a little more when I return this evening, if it will help.

Yes, please.

    Juanma

Re: recent commit broke src/dired.c on OSX

[Eli Zaretskii]

> From: Chad Brown <yandros@MIT.EDU>
> Date: Sun, 25 Jul 2010 13:12:26 -0700
> Cc: emacs-devel@gnu.org, "Randal L. Schwartz" <merlyn@stonehenge.com>
> 
> The resulting build gives warnings:
> 
> dired.c: In function ‘Ffile_attributes’:
> dired.c:1016: warning: comparison is always false due to limited range of data type
> dired.c:1016: warning: comparison is always false due to limited range of data type
> dired.c:1020: warning: comparison is always false due to limited range of data type
> dired.c:1020: warning: comparison is always false due to limited range of data type
> dired.c:1068: warning: comparison is always false due to limited range of data type
> dired.c:1068: warning: comparison is always false due to limited range of data type

This is normal, don't worry about it.

> It seems that Darwin's dirent.h prototype is:
> 
> DIR *opendir(const char *)
> 
> which conflicts with dired.c's unconditional (ifndef MSDOS) declaration:
> 
> extern DIR *opendir(char *)

Darwin should pick up its dirent.h instead of these explicit
prototypes, which are for systems that don't have dirent.h.

Re: recent commit broke src/dired.c on OSX

[Dan Nicolaescu]

Juanma Barranquero <lekktu@gmail.com> writes:

> On Sun, Jul 25, 2010 at 19:40, Randal L. Schwartz <merlyn@stonehenge.com> wrote:
>
>> Sorry, right commit, wrong reason.  Here's the error:
>
>>     char *
>>     getcwd(char *buf, size_t size);
>
> This is already fixed on the trunk, assuming that in your build
> environment HAVE_GETCWD is set.

If HAVE_GETCWD is set there should not be a need to have a prototype,
better just include unistd.h in that case.

IMHO we should get rid of all such extern declarations, and just
include the corresponding header files.

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Sun, Jul 25, 2010 at 22:50, Dan Nicolaescu <dann@gnu.org> wrote:

> IMHO we should get rid of all such extern declarations, and just
> include the corresponding header files.

FWIW, I agree. I was just making Emacs cleaner under
-Wstrict-prototypes, but I agree that these kinds of isolate extern
declarations are just hacks.

Please feel free to follow your suggestion, and TIA.

    Juanma

Re: recent commit broke src/dired.c on OSX

[Eli Zaretskii]

> From: Dan Nicolaescu <dann@gnu.org>
> Date: Sun, 25 Jul 2010 16:50:50 -0400
> Cc: emacs-devel@gnu.org, "Randal L. Schwartz" <merlyn@stonehenge.com>
> 
> >>     char *
> >>     getcwd(char *buf, size_t size);
> >
> > This is already fixed on the trunk, assuming that in your build
> > environment HAVE_GETCWD is set.
> 
> If HAVE_GETCWD is set there should not be a need to have a prototype,
> better just include unistd.h in that case.
> 
> IMHO we should get rid of all such extern declarations, and just
> include the corresponding header files.

100% agreement.

Re: recent commit broke src/dired.c on OSX

[Jan Djärv]

Juanma Barranquero skrev 2010-07-25 19.30:
> On Sun, Jul 25, 2010 at 19:23, Randal L. Schwartz<merlyn@stonehenge.com>  wrote:
>
>>     -extern DIR *opendir ();
>>     -extern struct direct *readdir ();
>>     +extern DIR *opendir (char *);
>>     +extern struct direct *readdir (DIR *);
>
>> Needs to be backed out, or made more portable.
>
> Sure. I'd prefer to make it more portable (that already compiles on
> Windows / MinGW and Ubuntu / GCC).
>
> What is the right prototype in your system? (You didn't attach the
> error output, BTW). Could you send a patch with the relevant #ifdef?
>

AFAIK, the correct prototype for opendir is

   DIR *opendir(const char *);

even on Ubunt, as well as OSX and of course POSIX and Single Unix 
Specification.  But on GNU/Linux SYSV_SYSTEM_DIR is defined, but not on OSX. 
So on GNU/Linux, the wrong prototype is not seen by the compiler.

I think it is an error to extern declare system functions, the proper include 
file should be used instead.
All the #ifdefs that try to figure out if dirent.h (the POSIX-mandated header 
file for opendir) is present should be replaced with a configure check for 
dirent.h.  Then SYSV_SYSTEM_DIR can go away.

	Jan D.

Re: recent commit broke src/dired.c on OSX

[Jan Djärv]

Jan Djärv skrev 2010-07-25 23.26:
>
> All the #ifdefs that try to figure out if dirent.h (the POSIX-mandated header
> file for opendir) is present should be replaced with a configure check for
> dirent.h. Then SYSV_SYSTEM_DIR can go away.
>

Correction, it is:


#include <sys/types.h>
#include <dirent.h>

DIR *opendir(const char *dirname);


	Jan D.

Re: recent commit broke src/dired.c on OSX

[Chad Brown]

Jan seems to be correct, that it's simply an issue of detecting dirent.h (instead of guessing via SYSV_SYSTEM_DIR).  In the meantime, the following trivial patch fixes the problem on OSX.

=== modified file 'src/dired.c'
*** src/dired.c	2010-07-25 00:20:51 +0000
--- src/dired.c	2010-07-26 02:21:58 +0000
***************
*** 48,59 ****
     Since applying strlen to the name always works, we'll just do that.  */
  #define NAMLEN(p) strlen (p->d_name)
  
! #ifdef SYSV_SYSTEM_DIR
  
  #include <dirent.h>
  #define DIRENTRY struct dirent
  
! #else /* not SYSV_SYSTEM_DIR */
  
  #ifdef MSDOS
  #include <dirent.h>
--- 48,59 ----
     Since applying strlen to the name always works, we'll just do that.  */
  #define NAMLEN(p) strlen (p->d_name)
  
! #if defined(SYSV_SYSTEM_DIR) || defined(DARWIN_OS)
  
  #include <dirent.h>
  #define DIRENTRY struct dirent
  
! #else /* not (SYSV_SYSTEM_DIR || DARWIN_OS) */
  
  #ifdef MSDOS
  #include <dirent.h>

Re: recent commit broke src/dired.c on OSX

[Chad Brown]

Here's a stab at implementing Jan's suggestion:  testing for dirent.h in configure instead of dancing around SYSV_SYSTEM_DIR.  The changes are pretty small, and I have signed papers already anyway.

I did simplify a test that checked for MSDOS, and I have no way to test on that platform, so I would appreciate it if someone would check to make sure I did no harm.  Thanks.

*Chad
P.S. If someone would prefer unidiffs instead, just let me know.  Also, I could put together a ChangeLog entry if that would be helpful -- I just realized that I hadn't done so, but the changes are pretty straightforward.

=== modified file 'configure'
*** configure	2010-07-25 19:09:54 +0000
--- configure	2010-07-26 02:39:53 +0000
***************
*** 6149,6155 ****
    linux/version.h sys/systeminfo.h termios.h limits.h string.h stdlib.h \
    stdio_ext.h fcntl.h strings.h coff.h pty.h sys/mman.h \
    sys/param.h sys/vlimit.h sys/resource.h locale.h sys/_mbstate_t.h \
!   sys/utsname.h pwd.h utmp.h
  do :
    as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
  ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
--- 6149,6155 ----
    linux/version.h sys/systeminfo.h termios.h limits.h string.h stdlib.h \
    stdio_ext.h fcntl.h strings.h coff.h pty.h sys/mman.h \
    sys/param.h sys/vlimit.h sys/resource.h locale.h sys/_mbstate_t.h \
!   sys/utsname.h pwd.h utmp.h dirent.h
  do :
    as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
  ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"

=== modified file 'configure.in'
*** configure.in	2010-07-25 19:09:54 +0000
--- configure.in	2010-07-26 02:36:43 +0000
***************
*** 1204,1210 ****
    linux/version.h sys/systeminfo.h termios.h limits.h string.h stdlib.h \
    stdio_ext.h fcntl.h strings.h coff.h pty.h sys/mman.h \
    sys/param.h sys/vlimit.h sys/resource.h locale.h sys/_mbstate_t.h \
!   sys/utsname.h pwd.h utmp.h)
  
  AC_MSG_CHECKING(if personality LINUX32 can be set)
  AC_TRY_COMPILE([#include <sys/personality.h>], [personality (PER_LINUX32)],
--- 1204,1210 ----
    linux/version.h sys/systeminfo.h termios.h limits.h string.h stdlib.h \
    stdio_ext.h fcntl.h strings.h coff.h pty.h sys/mman.h \
    sys/param.h sys/vlimit.h sys/resource.h locale.h sys/_mbstate_t.h \
!   sys/utsname.h pwd.h utmp.h dirent.h)
  
  AC_MSG_CHECKING(if personality LINUX32 can be set)
  AC_TRY_COMPILE([#include <sys/personality.h>], [personality (PER_LINUX32)],

=== modified file 'src/config.in'
*** src/config.in	2010-07-25 19:09:54 +0000
--- src/config.in	2010-07-26 02:43:53 +0000
***************
*** 737,742 ****
--- 737,745 ----
  /* Define to 1 if you have the <utmp.h> header file. */
  #undef HAVE_UTMP_H
  
+ /* Define to 1 if you have the <dirent.h> header file. */
+ #undef HAVE_DIRENT_H
+ 
  /* Define to 1 if you have the `vfork' function. */
  #undef HAVE_VFORK
  

=== modified file 'src/dired.c'
*** src/dired.c	2010-07-25 00:20:51 +0000
--- src/dired.c	2010-07-26 03:03:56 +0000
***************
*** 48,76 ****
     Since applying strlen to the name always works, we'll just do that.  */
  #define NAMLEN(p) strlen (p->d_name)
  
! #ifdef SYSV_SYSTEM_DIR
  
  #include <dirent.h>
  #define DIRENTRY struct dirent
  
! #else /* not SYSV_SYSTEM_DIR */
  
- #ifdef MSDOS
- #include <dirent.h>
- #else
  #include <sys/dir.h>
- #endif
- 
  #include <sys/stat.h>
  
- #ifndef MSDOS
  #define DIRENTRY struct direct
  
  extern DIR *opendir (char *);
  extern struct direct *readdir (DIR *);
  
! #endif /* not MSDOS */
! #endif /* not SYSV_SYSTEM_DIR */
  
  /* Some versions of Cygwin don't have d_ino in `struct dirent'.  */
  #if defined(MSDOS) || defined(__CYGWIN__)
--- 48,69 ----
     Since applying strlen to the name always works, we'll just do that.  */
  #define NAMLEN(p) strlen (p->d_name)
  
! #ifdef HAVE_DIRENT_H
  
  #include <dirent.h>
  #define DIRENTRY struct dirent
  
! #else /* not HAVE_DIRENT */
  
  #include <sys/dir.h>
  #include <sys/stat.h>
  
  #define DIRENTRY struct direct
  
  extern DIR *opendir (char *);
  extern struct direct *readdir (DIR *);
  
! #endif /* HAVE_DIRENT_H */
  
  /* Some versions of Cygwin don't have d_ino in `struct dirent'.  */
  #if defined(MSDOS) || defined(__CYGWIN__)

=== modified file 'src/sysdep.c'
*** src/sysdep.c	2010-07-13 10:57:00 +0000
--- src/sysdep.c	2010-07-26 02:48:50 +0000
***************
*** 2447,2453 ****
  \f
  /* Directory routines for systems that don't have them. */
  
! #ifdef SYSV_SYSTEM_DIR
  
  #include <dirent.h>
  
--- 2447,2453 ----
  \f
  /* Directory routines for systems that don't have them. */
  
! #ifdef HAVE_DIRENT
  
  #include <dirent.h>
  
***************
*** 2464,2470 ****
    return rtnval;
  }
  #endif /* not HAVE_CLOSEDIR */
! #endif /* SYSV_SYSTEM_DIR */
  
  \f
  int
--- 2464,2470 ----
    return rtnval;
  }
  #endif /* not HAVE_CLOSEDIR */
! #endif /* HAVE_DIRENT */
  
  \f
  int

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Mon, Jul 26, 2010 at 05:12, Chad Brown <yandros@mit.edu> wrote:

> P.S. If someone would prefer unidiffs instead, just let me know.

Yes, please..

> Also, I could put together a ChangeLog entry if that would be helpful

...and yes.

    Juanma

Re: recent commit broke src/dired.c on OSX

[Jan Djärv]

2010-07-26 05:12, Chad Brown skrev:
> Here's a stab at implementing Jan's suggestion:  testing for dirent.h in
> configure instead of dancing around SYSV_SYSTEM_DIR.  The changes are
> pretty small, and I have signed papers already anyway.
>
> I did simplify a test that checked for MSDOS, and I have no way to test on
> that platform, so I would appreciate it if someone would check to make sure
> I did no harm.  Thanks.
>
> *Chad P.S. If someone would prefer unidiffs instead, just let me know.
> Also, I could put together a ChangeLog entry if that would be helpful -- I
> just realized that I hadn't done so, but the changes are pretty
> straightforward.
>

You don't have to include configure and config.in in the diffs, those are 
generated from configure.in.  After your changes, SYSV_SYSTEM_DIR should be 
removed from thse files:

s/aix4-2.h
s/bsd-common.h
s/cygwin.h
s/gnu-linux.h
s/msdos.h
s/usg5-4.h

Thanks,

	Jan D.

Re: recent commit broke src/dired.c on OSX

[Jan Djärv]

2010-07-26 05:12, Chad Brown skrev:

> ! #else /* not HAVE_DIRENT */
>
>    #include <sys/dir.h>
>    #include <sys/stat.h>
>
>    #define DIRENTRY struct direct
>
>    extern DIR *opendir (char *);
>    extern struct direct *readdir (DIR *);
>

I don't have a system that uses sys/dir.h, but shouldn't it have prototypes 
for opendir and readdir?  Anybody know of such a system that we still support?

	Jan D.

Re: recent commit broke src/dired.c on OSX

[David Kastrup]

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

Juanma Barranquero <lekktu@gmail.com> writes:

> On Mon, Jul 26, 2010 at 05:12, Chad Brown <yandros@mit.edu> wrote:
>
>> P.S. If someone would prefer unidiffs instead, just let me know.
>
> Yes, please..

Uh, save as /tmp/junk.patch, load it into Emacs, type C-c C-u and you
have unidiff like the following:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2760 bytes --]

=== modified file 'configure'
--- configure	2010-07-25 19:09:54 +0000
+++ configure	2010-07-26 02:39:53 +0000
@@ -6149,7 +6149,7 @@
   linux/version.h sys/systeminfo.h termios.h limits.h string.h stdlib.h \
   stdio_ext.h fcntl.h strings.h coff.h pty.h sys/mman.h \
   sys/param.h sys/vlimit.h sys/resource.h locale.h sys/_mbstate_t.h \
-  sys/utsname.h pwd.h utmp.h
+  sys/utsname.h pwd.h utmp.h dirent.h
 do :
   as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
 ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"

=== modified file 'configure.in'
--- configure.in	2010-07-25 19:09:54 +0000
+++ configure.in	2010-07-26 02:36:43 +0000
@@ -1204,7 +1204,7 @@
   linux/version.h sys/systeminfo.h termios.h limits.h string.h stdlib.h \
   stdio_ext.h fcntl.h strings.h coff.h pty.h sys/mman.h \
   sys/param.h sys/vlimit.h sys/resource.h locale.h sys/_mbstate_t.h \
-  sys/utsname.h pwd.h utmp.h)
+  sys/utsname.h pwd.h utmp.h dirent.h)
 
 AC_MSG_CHECKING(if personality LINUX32 can be set)
 AC_TRY_COMPILE([#include <sys/personality.h>], [personality (PER_LINUX32)],

=== modified file 'src/config.in'
--- src/config.in	2010-07-25 19:09:54 +0000
+++ src/config.in	2010-07-26 02:43:53 +0000
@@ -737,6 +737,9 @@
 /* Define to 1 if you have the <utmp.h> header file. */
 #undef HAVE_UTMP_H
 
+/* Define to 1 if you have the <dirent.h> header file. */
+#undef HAVE_DIRENT_H
+
 /* Define to 1 if you have the `vfork' function. */
 #undef HAVE_VFORK
 

=== modified file 'src/dired.c'
--- src/dired.c	2010-07-25 00:20:51 +0000
+++ src/dired.c	2010-07-26 03:03:56 +0000
@@ -48,29 +48,22 @@
    Since applying strlen to the name always works, we'll just do that.  */
 #define NAMLEN(p) strlen (p->d_name)
 
-#ifdef SYSV_SYSTEM_DIR
+#ifdef HAVE_DIRENT_H
 
 #include <dirent.h>
 #define DIRENTRY struct dirent
 
-#else /* not SYSV_SYSTEM_DIR */
+#else /* not HAVE_DIRENT */
 
-#ifdef MSDOS
-#include <dirent.h>
-#else
 #include <sys/dir.h>
-#endif
-
 #include <sys/stat.h>
 
-#ifndef MSDOS
 #define DIRENTRY struct direct
 
 extern DIR *opendir (char *);
 extern struct direct *readdir (DIR *);
 
-#endif /* not MSDOS */
-#endif /* not SYSV_SYSTEM_DIR */
+#endif /* HAVE_DIRENT_H */
 
 /* Some versions of Cygwin don't have d_ino in `struct dirent'.  */
 #if defined(MSDOS) || defined(__CYGWIN__)

=== modified file 'src/sysdep.c'
--- src/sysdep.c	2010-07-13 10:57:00 +0000
+++ src/sysdep.c	2010-07-26 02:48:50 +0000
@@ -2447,7 +2447,7 @@
 \f
 /* Directory routines for systems that don't have them. */
 
-#ifdef SYSV_SYSTEM_DIR
+#ifdef HAVE_DIRENT
 
 #include <dirent.h>
 
@@ -2464,7 +2464,7 @@
   return rtnval;
 }
 #endif /* not HAVE_CLOSEDIR */
-#endif /* SYSV_SYSTEM_DIR */
+#endif /* HAVE_DIRENT */
 
 \f
 int






[-- Attachment #3: Type: text/plain, Size: 20 bytes --]



-- 
David Kastrup

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Mon, Jul 26, 2010 at 10:44, David Kastrup <dak@gnu.org> wrote:

> Uh, save as /tmp/junk.patch, load it into Emacs, type C-c C-u and you
> have unidiff like the following:

I know, but I was answering at 05:43 local time (that was a long
night, not an early morning) and so wasn't thinking too clearly.
Thanks for the reminder.

Anyway, most people (RMS excluded) seem to find unidiffs easier, so it
is better to send patches here that way.

    Juanma

Re: recent commit broke src/dired.c on OSX

[Eli Zaretskii]

> From: Chad Brown <yandros@MIT.EDU>
> Date: Sun, 25 Jul 2010 20:12:01 -0700
> 
> Here's a stab at implementing Jan's suggestion:  testing for dirent.h in configure instead of dancing around SYSV_SYSTEM_DIR.  The changes are pretty small, and I have signed papers already anyway.
> 
> I did simplify a test that checked for MSDOS, and I have no way to test on that platform, so I would appreciate it if someone would check to make sure I did no harm.  Thanks.

Whenever you add a test to configure and src/config.in, you need to
add a suitable edit to msdos/sed*.inp scripts.  In this case, they
should edit config.in to define HAVE_DIRENT_H, since evidently the
MSDOS build has it.  If you don't do that, the DOS build will break.

Re: recent commit broke src/dired.c on OSX

[Chad Brown]

[-- Attachment #1: Type: text/plain, Size: 85 bytes --]

Second try at the patch, with ChangeLogs and msdos build system changes included.


[-- Attachment #2: dirent.patch --]
[-- Type: application/octet-stream, Size: 6660 bytes --]

=== modified file 'ChangeLog'
--- ChangeLog	2010-07-25 19:09:54 +0000
+++ ChangeLog	2010-07-27 01:56:57 +0000
@@ -1,3 +1,7 @@
+2010-07-27  Chad Brown <yandros@mit.edu>
+
+	* configure.in: Check for dirent.h
+
 2010-07-25  Andreas Schwab  <schwab@linux-m68k.org>
 
 	* configure.in: Check for __executable_start.

=== modified file 'configure.in'
--- configure.in	2010-07-25 19:09:54 +0000
+++ configure.in	2010-07-26 02:36:43 +0000
@@ -1204,7 +1204,7 @@
   linux/version.h sys/systeminfo.h termios.h limits.h string.h stdlib.h \
   stdio_ext.h fcntl.h strings.h coff.h pty.h sys/mman.h \
   sys/param.h sys/vlimit.h sys/resource.h locale.h sys/_mbstate_t.h \
-  sys/utsname.h pwd.h utmp.h)
+  sys/utsname.h pwd.h utmp.h dirent.h)
 
 AC_MSG_CHECKING(if personality LINUX32 can be set)
 AC_TRY_COMPILE([#include <sys/personality.h>], [personality (PER_LINUX32)],

=== modified file 'msdos/ChangeLog'
--- msdos/ChangeLog	2010-07-24 11:53:19 +0000
+++ msdos/ChangeLog	2010-07-27 02:22:21 +0000
@@ -1,3 +1,7 @@
+2010-07-27  Chad Brown  <yandros@mit.edu>
+
+	* sed2v2.inp (HAVE_DIRENT_H): Edit to 1
+
 2010-07-12  Eli Zaretskii  <eliz@gnu.org>
 
 	* sed1v2.inp (C_WARNINGS_SWITCH, PROFILING_CFLAGS, PROFILING_LDFLAGS):

=== modified file 'msdos/sed2v2.inp'
--- msdos/sed2v2.inp	2010-07-11 13:43:10 +0000
+++ msdos/sed2v2.inp	2010-07-27 01:05:52 +0000
@@ -57,6 +57,7 @@
 /^#undef PACKAGE_TARNAME/s/^.*$/#define PACKAGE_TARNAME ""/
 /^#undef PACKAGE_VERSION/s/^.*$/#define PACKAGE_VERSION ""/
 /^#undef SYS_SIGLIST_DECLARED/s/^.*$/#define SYS_SIGLIST_DECLARED 1/
+/^#undef HAVE_DIRENT_H/s/^.*$/#define HAVE_DIRENT_H 1/
 
 /^#undef GNU_MALLOC *$/s/^.*$/#define GNU_MALLOC 1/
 /^#undef REL_ALLOC *$/s/^.*$/#define REL_ALLOC 1/

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2010-07-26 00:43:58 +0000
+++ src/ChangeLog	2010-07-27 02:19:03 +0000
@@ -1,3 +1,11 @@
+2010-07-27  Chad Brown  <yandros@mit.edu>
+
+	Replace tests for SYSV_SYSTEM_DIR with HAVE_DIRENT_H, set via autoconf
+	* dired.c, sysdep.c: test HAVE_DIRENT_H instead of SYSV_SYSTEM_DIR
+
+	* s/aix4-2.h, s/bsd-common.h, s/cygwin.h, s/gnu-linux.h,
+	  s/msdos.h, s/usg5-4.h: don't define SYSV_SYSTEM_DIR
+
 2010-07-26  Juanma Barranquero  <lekktu@gmail.com>
 
 	* lread.c (unreadpure, mapatoms_1): Make static.

=== modified file 'src/config.in'
--- src/config.in	2010-07-25 19:09:54 +0000
+++ src/config.in	2010-07-26 02:43:53 +0000
@@ -737,6 +737,9 @@
 /* Define to 1 if you have the <utmp.h> header file. */
 #undef HAVE_UTMP_H
 
+/* Define to 1 if you have the <dirent.h> header file. */
+#undef HAVE_DIRENT_H
+
 /* Define to 1 if you have the `vfork' function. */
 #undef HAVE_VFORK
 

=== modified file 'src/dired.c'
--- src/dired.c	2010-07-25 00:20:51 +0000
+++ src/dired.c	2010-07-27 00:55:10 +0000
@@ -48,29 +48,22 @@
    Since applying strlen to the name always works, we'll just do that.  */
 #define NAMLEN(p) strlen (p->d_name)
 
-#ifdef SYSV_SYSTEM_DIR
+#ifdef HAVE_DIRENT_H
 
 #include <dirent.h>
 #define DIRENTRY struct dirent
 
-#else /* not SYSV_SYSTEM_DIR */
+#else /* not HAVE_DIRENT_H */
 
-#ifdef MSDOS
-#include <dirent.h>
-#else
 #include <sys/dir.h>
-#endif
-
 #include <sys/stat.h>
 
-#ifndef MSDOS
 #define DIRENTRY struct direct
 
 extern DIR *opendir (char *);
 extern struct direct *readdir (DIR *);
 
-#endif /* not MSDOS */
-#endif /* not SYSV_SYSTEM_DIR */
+#endif /* HAVE_DIRENT_H */
 
 /* Some versions of Cygwin don't have d_ino in `struct dirent'.  */
 #if defined(MSDOS) || defined(__CYGWIN__)

=== modified file 'src/s/aix4-2.h'
--- src/s/aix4-2.h	2010-06-30 02:48:54 +0000
+++ src/s/aix4-2.h	2010-07-26 08:23:46 +0000
@@ -47,11 +47,6 @@
 /* Define HAVE_SOCKETS if system supports 4.2-compatible sockets.  */
 #define HAVE_SOCKETS
 
-
-/* Define SYSV_SYSTEM_DIR to use the V.3 getdents/readir library
-   functions.  Almost, but not quite the same as the 4.2 functions.  */
-#define SYSV_SYSTEM_DIR
-
 /* The file containing the kernel's symbol table is called /unix.  */
 #define KERNEL_FILE "/unix"
 

=== modified file 'src/s/bsd-common.h'
--- src/s/bsd-common.h	2010-06-06 08:57:39 +0000
+++ src/s/bsd-common.h	2010-07-26 08:23:45 +0000
@@ -45,8 +45,6 @@
 #define HAVE_TERMIOS
 #define NO_TERMIO
 
-#define SYSV_SYSTEM_DIR
-
 /* If the system's imake configuration file defines `NeedWidePrototypes'
    as `NO', we must define NARROWPROTO manually.  Such a define is
    generated in the Makefile generated by `xmkmf'.  If we don't

=== modified file 'src/s/cygwin.h'
--- src/s/cygwin.h	2010-06-06 08:57:39 +0000
+++ src/s/cygwin.h	2010-07-26 08:23:44 +0000
@@ -89,7 +89,6 @@
 #define CYGWIN 1
 
 #define PENDING_OUTPUT_COUNT(FILE) ((FILE)->_p - (FILE)->_bf._base)
-#define SYSV_SYSTEM_DIR 1
 
 #define HAVE_SOCKETS
 

=== modified file 'src/s/gnu-linux.h'
--- src/s/gnu-linux.h	2010-06-06 08:57:39 +0000
+++ src/s/gnu-linux.h	2010-07-26 08:23:43 +0000
@@ -124,8 +124,6 @@
 #define INTERRUPT_INPUT
 #endif /* emacs */
 
-#define SYSV_SYSTEM_DIR       /* use dirent.h */
-
 #define POSIX                 /* affects getpagesize.h and systty.h */
 
 /* This is to work around mysterious gcc failures in some system versions.

=== modified file 'src/s/msdos.h'
--- src/s/msdos.h	2010-07-12 16:56:59 +0000
+++ src/s/msdos.h	2010-07-26 08:23:42 +0000
@@ -38,8 +38,6 @@
    It sets the Lisp variable system-type.  */
 #define SYSTEM_TYPE "ms-dos"
 
-#define SYSV_SYSTEM_DIR
-
 /* subprocesses should be defined if you want to have code for
    asynchronous subprocesses (as used in M-x compile and M-x shell).
    This is the only system that needs this.  */

=== modified file 'src/s/usg5-4.h'
--- src/s/usg5-4.h	2010-07-11 10:31:10 +0000
+++ src/s/usg5-4.h	2010-07-26 08:23:40 +0000
@@ -35,10 +35,6 @@
    for terminal control.  */
 #define HAVE_TERMIO
 
-/* Define SYSV_SYSTEM_DIR to use the V.3 getdents/readir library
-   functions. Almost, but not quite the same as the 4.2 functions.  */
-#define SYSV_SYSTEM_DIR
-
 /* The file containing the kernel's symbol table is called /unix.  */
 #define KERNEL_FILE "/unix"
 

=== modified file 'src/sysdep.c'
--- src/sysdep.c	2010-07-13 10:57:00 +0000
+++ src/sysdep.c	2010-07-26 09:01:39 +0000
@@ -2447,7 +2447,7 @@
 \f
 /* Directory routines for systems that don't have them. */
 
-#ifdef SYSV_SYSTEM_DIR
+#ifdef HAVE_DIRENT_H
 
 #include <dirent.h>
 
@@ -2464,7 +2464,7 @@
   return rtnval;
 }
 #endif /* not HAVE_CLOSEDIR */
-#endif /* SYSV_SYSTEM_DIR */
+#endif /* HAVE_DIRENT_H */
 
 \f
 int

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

> Second try at the patch, with ChangeLogs and msdos build system changes included.

AFAICS, Windows also has dirent.h, at least MinGW has it.

    Juanma

Re: recent commit broke src/dired.c on OSX

[Óscar Fuentes]

Juanma Barranquero <lekktu@gmail.com> writes:

> AFAICS, Windows also has dirent.h, at least MinGW has it.

MSVC++ has direct.h

MSDN flags `mkdir' as obsolete and recommends `_mkdir'.

MinGW has direct.h too.

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Wed, Jul 28, 2010 at 01:28, Óscar Fuentes <ofv@wanadoo.es> wrote:

> MSVC++ has direct.h
>
> MSDN flags `mkdir' as obsolete and recommends `_mkdir'.
>
> MinGW has direct.h too.

I fail to see the relevance.

    Juanma

Re: recent commit broke src/dired.c on OSX

[Óscar Fuentes]

Juanma Barranquero <lekktu@gmail.com> writes:

> On Wed, Jul 28, 2010 at 01:28, Óscar Fuentes <ofv@wanadoo.es> wrote:
>
>> MSVC++ has direct.h
>>
>> MSDN flags `mkdir' as obsolete and recommends `_mkdir'.
>>
>> MinGW has direct.h too.
>
> I fail to see the relevance.

I fail to understand your failure to see the relevance.

You said:

"AFAICS, Windows also has dirent.h, at least MinGW has it."

and I was expanding that information to cover MSVC++, which falls under
the Windows category too.

Perhaps you missed that I was mentioning direCt.h instead of direNt.h?

It can be useful to know that using dirent.h won't work for VC++, but
using direct.h will work for both VC++ and MinGW.

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Wed, Jul 28, 2010 at 02:33, Óscar Fuentes <ofv@wanadoo.es> wrote:

> I fail to understand your failure to see the relevance.

I see your failure and raise it :-)

> Perhaps you missed that I was mentioning direCt.h instead of direNt.h?

No. But the functions offered by direCt.h are not related to the ones
in direNt.h. So, how does including direCt.h affect opendir / readdir?

> It can be useful to know that using dirent.h won't work for VC++

Yes.

> but using direct.h will work for both VC++ and MinGW.

Will work, if the code is adapted to using it, you mean?

    Juanma

Re: recent commit broke src/dired.c on OSX

[Óscar Fuentes]

Juanma Barranquero <lekktu@gmail.com> writes:

>> Perhaps you missed that I was mentioning direCt.h instead of direNt.h?
>
> No. But the functions offered by direCt.h are not related to the ones
> in direNt.h. So, how does including direCt.h affect opendir / readdir?

Ops! I jumped in the middle of the thread and thought that you were
discussing mkdir/rmdir. Today I was working on the Windows part of the
cmake build and the implementation of mkdir on dired.c is causing me
some headaches. Seems I'm obsessed with it :-/

Sorry for the noise.

[snip]

Re: recent commit broke src/dired.c on OSX

[Juanma Barranquero]

On Wed, Jul 28, 2010 at 02:54, Óscar Fuentes <ofv@wanadoo.es> wrote:

> Sorry for the noise.

No problem. At least there was some real-world reason for my failure
to understand, apart from the usual sleep deprivation.

    Juanma

Re: recent commit broke src/dired.c on OSX

[Jan Djärv]

Chad Brown skrev 2010-07-27 23.40:
> Second try at the patch, with ChangeLogs and msdos build system changes included.
>

Looks good to me, don't forget to run autoheader and autoconf before you 
commit (I sometimes do...).

	Jan D.

Re: recent commit broke src/dired.c on OSX

[Randal L. Schwartz]

>>>>> "Randal" == Randal L Schwartz <merlyn@stonehenge.com> writes:

Randal> I was building from the latest git.  I'll have to wait until it catches up.

Even as of:

  Author: Jan D. <jan.h.d@swipnet.se>
  Date:   Thu Jul 29 19:10:41 2010 +0200

HEAD doesn't build on darwin.  Same problem. :(

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
See http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion

Re: recent commit broke src/dired.c on OSX

[Jan Djärv]

There was a patch to fix this on this list, but it hasn't been checked in for 
some reason.  If you really want to compile Emacs, just remove the opendir 
declaration in src/dired.c.

	Jan D.


Randal L. Schwartz skrev 2010-07-29 20.21:
>>>>>> "Randal" == Randal L Schwartz<merlyn@stonehenge.com>  writes:
>
> Randal>  I was building from the latest git.  I'll have to wait until it catches up.
>
> Even as of:
>
>    Author: Jan D.<jan.h.d@swipnet.se>
>    Date:   Thu Jul 29 19:10:41 2010 +0200
>
> HEAD doesn't build on darwin.  Same problem. :(
>

Re: recent commit broke src/dired.c on OSX

[Randal L. Schwartz]

>>>>> "Jan" == Jan Djärv <jan.h.d@swipnet.se> writes:

Jan> There was a patch to fix this on this list, but it hasn't been checked in for
Jan> some reason.  If you really want to compile Emacs, just remove the opendir
Jan> declaration in src/dired.c.

I have no urgent need to compile emacs.  I'm just reporting on the state
of things. :)

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc.
See http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion

Re: recent commit broke src/dired.c on OSX

[Chad Brown]

On Jul 29, 2010, at 12:11 PM, Jan Djärv wrote:

> There was a patch to fix this on this list, but it hasn't been checked in for some reason.  If you really want to compile Emacs, just remove the opendir declaration in src/dired.c.
> 
> 	Jan D.

Yeah, sorry about that.  I hadn't expected to use commit privileges under bazaar when I set it up, and haven't figured out how to convert the http read-only access I initially set up to sftp yet.

If I could beg the indulgence of someone else to commit the patch this one time, I would be grateful, as I don't know if I'll be able to manage the switch before an impending week of travel.

My apologies and thanks,
*Chad

Re: recent commit broke src/dired.c on OSX

[Jan Djärv]

Chad Brown skrev 2010-07-29 21.29:
>
> On Jul 29, 2010, at 12:11 PM, Jan Djärv wrote:
>
>> There was a patch to fix this on this list, but it hasn't been checked in
>> for some reason.  If you really want to compile Emacs, just remove the
>> opendir declaration in src/dired.c.
>>
>> Jan D.
>
> Yeah, sorry about that.  I hadn't expected to use commit privileges under
> bazaar when I set it up, and haven't figured out how to convert the http
> read-only access I initially set up to sftp yet.
>
> If I could beg the indulgence of someone else to commit the patch this one
> time, I would be grateful, as I don't know if I'll be able to manage the
> switch before an impending week of travel.
>
> My apologies and thanks, *Chad

I checked it in.  There was some issue about dirent.h and w32, can somebody 
check it?

	Jan D.

Re: recent commit broke src/dired.c on OSX

[Óscar Fuentes]

Jan Djärv <jan.h.d@swipnet.se> writes:

> I checked it in.  There was some issue about dirent.h and w32, can
> somebody check it?

emacs.exe still is compiled fine after the change, if is that what you
want to know.

Re: recent commit broke src/dired.c on OSX

[Jan Djärv]

Óscar Fuentes skrev 2010-07-29 22.37:
> The following message is a courtesy copy of an article
> that has been posted to gmane.emacs.devel as well.
>
> Jan Djärv<jan.h.d@swipnet.se>  writes:
>
>> I checked it in.  There was some issue about dirent.h and w32, can
>> somebody check it?
>
> emacs.exe still is compiled fine after the change, if is that what you
> want to know.

Juanma Barranquero said this:

 > AFAICS, Windows also has dirent.h, at least MinGW has it.

I don't know what change that implies.

	Jan D.

Re: recent commit broke src/dired.c on OSX

[Óscar Fuentes]

Jan Djärv <jan.h.d@swipnet.se> writes:

>>> I checked it in.  There was some issue about dirent.h and w32, can
>>> somebody check it?
>>
>> emacs.exe still is compiled fine after the change, if is that what you
>> want to know.
>
> Juanma Barranquero said this:
>
>> AFAICS, Windows also has dirent.h, at least MinGW has it.
>
> I don't know what change that implies.

If the user builds Emacs using the Windows-specific procedure, I see no
problem, as SYSV_SYSTEM_DIR was not defined there and hence the change
makes no effect.

Re: recent commit broke src/dired.c on OSX

[Chad Brown]

On Jul 29, 2010, at 2:07 PM, Jan Djärv wrote:

> Juanma Barranquero said this:
> 
> > AFAICS, Windows also has dirent.h, at least MinGW has it.
> 
> I don't know what change that implies.
> 
> 	Jan D.

First, thank you for applying the patch.

I looked at this, but I don't have a win32 build environment available.  It's possible that a test could be simplified, but since I didn't have a way to test MinGW versus Cygwin or MSYS or whatever), I left it alone. 

*Chad