windows build failure

windows build failure

[Sean Sieger]

gcc -I. -c -gdwarf-2 -g3  -DEMACSDEBUG -DENABLE_CHECKING -DXASSERTS -fno-crossju
mping  -Ic:/imagesupport/include -DHAVE_CONFIG_H=1 -I. -I../nt/inc -I../src -o o
o/i386/md5.o md5.c
rm oo/i386/libgnu.a
rm: cannot remove `oo/i386/libgnu.a': No such file or directory
make[1]: [oo/i386/libgnu.a] Error 1 (ignored)
ar -rsc oo/i386/libgnu.a oo/i386/dtoastr.o oo/i386/getopt.o oo/i386/getopt1.o oo
/i386/strftime.o oo/i386/time_r.o oo/i386/md5.o
make[1]: Leaving directory `c:/emacs-24.0.50/lib'
make   -C ../src bootstrap
make[1]: Entering directory `c:/emacs-24.0.50/src'
make -w  temacs CFLAGS="-I. -c -gdwarf-2 -g3  -DEMACSDEBUG -DENABLE_CHECKING -DX
ASSERTS -fno-crossjumping  -Ic:/imagesupport/include -Demacs=1 -DHAVE_CONFIG_H -
I../lib -I../nt/inc -DHAVE_NTGUI=1 -DUSE_CRT_DLL=1 -DPURESIZE=5000000"
make[2]: Entering directory `c:/emacs-24.0.50/src'
mkdir "oo"
mkdir "oo/i386"
echo oo/i386 > stamp_BLD
make[2]: *** No rule to make target `filemode.c', needed by `gl-stamp'.  Stop.
make[2]: Leaving directory `c:/emacs-24.0.50/src'
make[1]: *** [bootstrap-temacs] Error 2
make[1]: Leaving directory `c:/emacs-24.0.50/src'
make: *** [bootstrap-gmake] Error 2

Re: windows build failure

[Sean Sieger]

Shoot.  Sorry, you knew that.

Re: windows build failure

[Christoph]

On 2/21/2011 12:37 PM, Sean Sieger wrote:

> make[2]: *** No rule to make target `filemode.c', needed by `gl-stamp'.  Stop.
> make[2]: Leaving directory `c:/emacs-24.0.50/src'
> make[1]: *** [bootstrap-temacs] Error 2
> make[1]: Leaving directory `c:/emacs-24.0.50/src'
> make: *** [bootstrap-gmake] Error 2

A fix is in the works.

Re: windows build failure

[Paul Eggert]

On 02/21/2011 11:37 AM, Sean Sieger wrote:

> make[2]: *** No rule to make target `filemode.c', needed by `gl-stamp'.  Stop.

Yes, that's due to the filemode change installed a few hours ago; see
<http://lists.gnu.org/archive/html/emacs-devel/2011-02/msg00935.html>
and the parent thread.  As discussed in that thread, the Windows builds
should need minor tweaking and that should happen soon.

Re: windows build failure

[Eli Zaretskii]

> Date: Mon, 21 Feb 2011 11:48:51 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> On 02/21/2011 11:37 AM, Sean Sieger wrote:
> 
> > make[2]: *** No rule to make target `filemode.c', needed by `gl-stamp'.  Stop.
> 
> Yes, that's due to the filemode change installed a few hours ago; see
> <http://lists.gnu.org/archive/html/emacs-devel/2011-02/msg00935.html>
> and the parent thread.  As discussed in that thread, the Windows builds
> should need minor tweaking and that should happen soon.

Done (revision 103375).

Re: windows build failure

[Lennart Borgman]

On Mon, Feb 21, 2011 at 9:21 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon, 21 Feb 2011 11:48:51 -0800
>> From: Paul Eggert <eggert@cs.ucla.edu>
>> Cc: emacs-devel@gnu.org
>>
>> On 02/21/2011 11:37 AM, Sean Sieger wrote:
>>
>> > make[2]: *** No rule to make target `filemode.c', needed by `gl-stamp'.  Stop.
>>
>> Yes, that's due to the filemode change installed a few hours ago; see
>> <http://lists.gnu.org/archive/html/emacs-devel/2011-02/msg00935.html>
>> and the parent thread.  As discussed in that thread, the Windows builds
>> should need minor tweaking and that should happen soon.
>
> Done (revision 103375).


Thanks Eli.

Re: windows build failure

[Eli Zaretskii]

> Date: Mon, 21 Feb 2011 11:48:51 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> On 02/21/2011 11:37 AM, Sean Sieger wrote:
> 
> > make[2]: *** No rule to make target `filemode.c', needed by `gl-stamp'.  Stop.
> 
> Yes, that's due to the filemode change installed a few hours ago; see
> <http://lists.gnu.org/archive/html/emacs-devel/2011-02/msg00935.html>
> and the parent thread.  As discussed in that thread, the Windows builds
> should need minor tweaking and that should happen soon.

Btw, one fallout of the filemode.c import is that now all platforms
have S_IFLNK defined (by virtue of lib/sys_stat.h), so the remapping
of lstat to stat on fileio.c and dired.c is no longer needed, and can
be deleted.  Unless I'm missing something, that is.

Re: windows build failure

[Sean Sieger]

Eli Zaretskii <eliz@gnu.org> writes:

    Done (revision 103375).

Great.  Built successfully.

Re: windows build failure

[Paul Eggert]

On 02/21/2011 12:37 PM, Eli Zaretskii wrote:

> one fallout of the filemode.c import is that now all platforms
> have S_IFLNK defined (by virtue of lib/sys_stat.h), so the remapping
> of lstat to stat on fileio.c and dired.c is no longer needed, and can
> be deleted.  Unless I'm missing something, that is.

Thanks for mentioning that.  I committed simplifications along those
lines to the trunk as revision 103380.  I don't expect
this to affects Windows builds, as the only new symbol in config.in
is HAVE_LSTAT and Windows doesn't have lstat.

One minor thing: Gnulib's <sys/stat.h> substitute guarantees
the existence of S_ISLNK, but not S_IFLNK.

Re: windows build failure

[Eli Zaretskii]

> Date: Mon, 21 Feb 2011 18:09:07 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> On 02/21/2011 12:37 PM, Eli Zaretskii wrote:
> 
> > one fallout of the filemode.c import is that now all platforms
> > have S_IFLNK defined (by virtue of lib/sys_stat.h), so the remapping
> > of lstat to stat on fileio.c and dired.c is no longer needed, and can
> > be deleted.  Unless I'm missing something, that is.
> 
> Thanks for mentioning that.  I committed simplifications along those
> lines to the trunk as revision 103380.

Thanks.

> One minor thing: Gnulib's <sys/stat.h> substitute guarantees
> the existence of S_ISLNK, but not S_IFLNK.

Yes, that was a typo on my part (to my defense, I would point out the
time when I wrote that ;-).

Please note that the portions of the code that need symlinks (and the
corresponding library functions) be supported by the host are
currently guarded by "#ifdef S_IFLNK", which the Windows and DOS
platforms do not define.  OTOH, S_ISLNK _is_ defined on all
platforms.  This makes me uneasy, because I've never seen a situation
with such a mismatch.  Normally, if S_IFLNK is defined, so is S_ISLNK,
and vice versa.  (I realize that this may have been otherwise in the
past, but I don't think any of those older systems are still in use,
or supported by Emacs.)

IOW, we use this mismatch as a trick to get filemode.c compile,
without breaking non-Posix platforms which don't really have symlinks.

I'm worried that someone unaware of this trick might assume that
S_IFLNK is also available, and will write code that will subtly break
non-Posix platforms.

If I'm not the only one who feels uneasy about this mismatch, we may
wish doing something about it.

(Personally, I wonder why gnulib removed the "#ifdef S_ISLNK" guards
in the first place: the artificial definition to zero in sys_stat.in.h
is only fine if one ignores the de-facto practice of testing for
S_IFLNK or S_ISLNK to guard code elsewhere that cannot be compiled or
run on platforms without symlinks.  So it looks like an anti-social
decision on the part of gnulib, since it does support MinGW.)

Re: windows build failure

[Paul Eggert]

On 02/22/2011 12:57 AM, Eli Zaretskii wrote:
>  the artificial definition to zero in sys_stat.in.h
> is only fine if one ignores the de-facto practice of testing for
> S_IFLNK or S_ISLNK to guard code elsewhere that cannot be compiled or
> run on platforms without symlinks

No, the idea is that normal code should not use #ifdef S_IFLNK.
It should instead use "if (S_ISLNK (...)) ...".
On hosts that don't have symbolic links, this "if"
gets optimized away to nothing, so it's just as fast
as the #ifdef.

Using plain "if" tends to lead to code that is
easier to maintain than using "#ifdef", in cases
like these.  Among other things, trivial typos
inside the then-part are detected even when compiling
on platforms that lack the feature in question.

Large chunks of GNU code have been written using this style,
in programs such as coreutils, and it typically works well.

Re: windows build failure

[Eli Zaretskii]

> Date: Tue, 22 Feb 2011 01:40:24 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> On 02/22/2011 12:57 AM, Eli Zaretskii wrote:
> >  the artificial definition to zero in sys_stat.in.h
> > is only fine if one ignores the de-facto practice of testing for
> > S_IFLNK or S_ISLNK to guard code elsewhere that cannot be compiled or
> > run on platforms without symlinks
> 
> No, the idea is that normal code should not use #ifdef S_IFLNK.

Well, "normal code" in Emacs does.  We have 5 such places at this
time, if I didn't miss any.

> It should instead use "if (S_ISLNK (...)) ...".
> On hosts that don't have symbolic links, this "if"
> gets optimized away to nothing, so it's just as fast
> as the #ifdef.

How can we use this idea, when the code in question calls functions
like `symlink' or `readlink", which don't exist on hosts that don't
support symlinks?  This won't link, unless we do something else in
addition.  If we want to go your way, we need to decide how to resolve
this difficulty (and do it in a way that won't become broken when you
resync with gnulib an hour or a day or a year from now ;-).

> Using plain "if" tends to lead to code that is easier to maintain
> than using "#ifdef", in cases like these.

I have no issues with using "if", but we must decide how to do that
without breaking compilation/link.

Once compilation and link work, we will also need to carefully review
the affected code, because its logic might subtly assume something
about the underlying platforms' behavior wrt symlinks, and that logic
might do The Wrong Thing when applied to hosts without symlinks, in
the parts that are common to "if" and its "else".

> Large chunks of GNU code have been written using this style,
> in programs such as coreutils, and it typically works well.

For some value of "well".  I really don't want to begin griping about
the MinGW port of Coreutils on Windows (that's OT here, anyway), but
for starters, it is linked against a ported glibc, which includes a
bastard semi-broken emulation of symlinks.  So I'm not sure anyone has
ever tested this approach in practice, at least not in Coreutils.

Again, I'm not against going the "if" path, I just wanted to (a) point
out the issue and hear from the relevant parties what, if anything,
they prefer to do about that, and (b) make sure we all understand that
fixing it cleanly is not a trivial job (but not rocket science,
either, of course).  Once the decision is made, I will be more than
willing to contribute to this effort.

Re: windows build failure

[Paul Eggert]

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

On 02/22/2011 02:49 AM, Eli Zaretskii wrote:
>> From: Paul Eggert<eggert@cs.ucla.edu>
>> No, the idea is that normal code should not use #ifdef S_IFLNK.
>
> Well, "normal code" in Emacs does.  We have 5 such places at this
> time, if I didn't miss any.

These #ifdef uses are problematic, and should be removed.  For example,
on a GNU system (make-symbolic-link A B) signals a file-already-exists
error if B already exists, but if I'm reading the code correctly this
does not happen on a system lacking symlinks.  It's better to remove
unnecessary discrepancies like this.

> How can we use this idea, when the code in question calls functions
> like `symlink' or `readlink", which don't exist on hosts that don't
> support symlinks?

Use substitutes for symlink and readlink that always fail.
That's easy and works well in practice.

> Once compilation and link work, we will also need to carefully review
> the affected code,

The affected code is fairly small.  A complete patch for your review
is below; it removes the problematic #ifdefs and thereby
shrinks the main Emacs source code by about 25 lines.
(As usual, I'm attaching the full patch, which includes files
autogenerated from gnulib.)

The new symbols in config.in that you may have to look at for Windows
include FILE_SYSTEM_ACCEPTS_DRIVE_LETTER_PREFIX,
FILE_SYSTEM_BACKSLASH_IS_FILE_NAME_SEPARATOR,
FILE_SYSTEM_DRIVE_PREFIX_CAN_BE_RELATIVE, HAVE_READLINK, HAVE_SYMLINK,
REPLACE_FUNC_STAT_DIR, and REPLACE_FUNC_STAT_FILE.

This patch also works around some other portability issues with
symlinks, with respect to file names that end with "/".  This
doesn't affect Windows, but we might as well fix those too,
since we get them for essentially zero programming cost here.

[ChangeLog]
Work around some portability problems with symlinks.
* Makefile.in (GNULIB_MODULES): Add lstat, readlink, symlink.
* configure.in (lstat, HAVE_LSTAT): Remove special hack.
* lib/lstat.c, lib/readlink.c, lib/stat.c, lib/symlink.c:
* m4/dos.m4, m4/lstat.m4, m4/readlink.m4, m4/stat.m4, m4/symlink.m4:
New files, automatically generated from gnulib.
* aclocal.m4, configure, lib/Makefile.in, lib/gnulib.mk:
* lib/stdlib.in.h, m4/gl-comp.m4, m4/stdlib_h.m4: Regenerate.

2011-02-22  Paul Eggert  <eggert@cs.ucla.edu>
[src/ChangeLog]
Work around some portability problems with symlinks.
* fileio.c (Frename_file, Fmake_symbolic_link, Ffile_symlink_p):
Simplify the code by assuming that the readlink and symlink calls
exist, even if they always fail on this host.
(Ffile_readable_p): Likewise, for fifos.
* config.in: Regenerate.
=== modified file 'Makefile.in'
--- Makefile.in	2011-02-22 01:55:20 +0000
+++ Makefile.in	2011-02-22 18:48:53 +0000
@@ -332,7 +332,7 @@
  # as per $(gnulib_srcdir)/DEPENDENCIES.
  GNULIB_MODULES = \
    crypto/md5 dtoastr filemode getloadavg getopt-gnu \
-  ignore-value mktime strftime sys_stat
+  ignore-value lstat mktime readlink strftime symlink sys_stat
  GNULIB_TOOL_FLAGS = \
   --import --no-changelog --no-vc-files --makefile-name=gnulib.mk
  sync-from-gnulib: $(gnulib_srcdir)

=== modified file 'configure.in'
--- configure.in	2011-02-22 01:55:20 +0000
+++ configure.in	2011-02-22 18:48:14 +0000
@@ -2661,15 +2661,6 @@
  gl_ASSERT_NO_GNULIB_TESTS
  gl_INIT

-# Emacs does not care about lstat's behavior on files whose names end in
-# trailing slashes, so it does not use the gnulib lstat module.
-# However, Emacs does want the "#define lstat stat" in sys/stat.h
-# when lstat does not exist, so it pretends to use the lstat module
-# even though it implements only the lstat-checking part of that module.
-AC_CHECK_FUNCS_ONCE([lstat])
-test $ac_cv_func_lstat = yes || HAVE_LSTAT=0
-gl_SYS_STAT_MODULE_INDICATOR([lstat])
-
  # UNIX98 PTYs.
  AC_CHECK_FUNCS(grantpt)


=== modified file 'src/fileio.c'
--- src/fileio.c	2011-02-22 01:55:20 +0000
+++ src/fileio.c	2011-02-22 18:39:29 +0000
@@ -2178,14 +2178,11 @@
        if (errno == EXDEV)
  	{
            int count;
-#ifdef S_IFLNK
            symlink_target = Ffile_symlink_p (file);
            if (! NILP (symlink_target))
              Fmake_symbolic_link (symlink_target, newname,
                                   NILP (ok_if_already_exists) ? Qnil : Qt);
-          else
-#endif
-	  if (!NILP (Ffile_directory_p (file)))
+	  else if (!NILP (Ffile_directory_p (file)))
  	    call4 (Qcopy_directory, file, newname, Qt, Qnil);
  	  else
  	    /* We have already prompted if it was an integer, so don't
@@ -2197,11 +2194,7 @@
  	  count = SPECPDL_INDEX ();
  	  specbind (Qdelete_by_moving_to_trash, Qnil);

-	  if (!NILP (Ffile_directory_p (file))
-#ifdef S_IFLNK
-	      && NILP (symlink_target)
-#endif
-	      )
+	  if (!NILP (Ffile_directory_p (file)) && NILP (symlink_target))
  	    call2 (Qdelete_directory, file, Qt);
  	  else
  	    Fdelete_file (file, Qnil);
@@ -2311,7 +2304,6 @@
      RETURN_UNGCPRO (call4 (handler, Qmake_symbolic_link, filename,
  			   linkname, ok_if_already_exists));

-#ifdef S_IFLNK
    encoded_filename = ENCODE_FILE (filename);
    encoded_linkname = ENCODE_FILE (linkname);

@@ -2338,12 +2330,6 @@
      }
    UNGCPRO;
    return Qnil;
-
-#else
-  UNGCPRO;
-  xsignal1 (Qfile_error, build_string ("Symbolic links are not supported"));
-
-#endif /* S_IFLNK */
  }

  \f
@@ -2482,7 +2468,7 @@
    return Qnil;
  #else /* not DOS_NT and not macintosh */
    flags = O_RDONLY;
-#if defined (S_IFIFO) && defined (O_NONBLOCK)
+#ifdef O_NONBLOCK
    /* Opening a fifo without O_NONBLOCK can wait.
       We don't want to wait.  But we don't want to mess wth O_NONBLOCK
       except in the case of a fifo, on a system which handles it.  */
@@ -2584,6 +2570,10 @@
    (Lisp_Object filename)
  {
    Lisp_Object handler;
+  char *buf;
+  int bufsize;
+  int valsize;
+  Lisp_Object val;

    CHECK_STRING (filename);
    filename = Fexpand_file_name (filename, Qnil);
@@ -2594,13 +2584,6 @@
    if (!NILP (handler))
      return call2 (handler, Qfile_symlink_p, filename);

-#ifdef S_IFLNK
-  {
-  char *buf;
-  int bufsize;
-  int valsize;
-  Lisp_Object val;
-
    filename = ENCODE_FILE (filename);

    bufsize = 50;
@@ -2635,10 +2618,6 @@
    xfree (buf);
    val = DECODE_FILE (val);
    return val;
-  }
-#else /* not S_IFLNK */
-  return Qnil;
-#endif /* not S_IFLNK */
  }

  DEFUN ("file-directory-p", Ffile_directory_p, Sfile_directory_p, 1, 1, 0,




[-- Attachment #2: emacs-symlink-diff.txt.gz --]
[-- Type: application/x-gzip, Size: 15638 bytes --]

Re: windows build failure

[Eli Zaretskii]

> Date: Tue, 22 Feb 2011 13:44:48 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> [1:text/plain Hide]
> On 02/22/2011 02:49 AM, Eli Zaretskii wrote:
> >> From: Paul Eggert<eggert@cs.ucla.edu>
> >> No, the idea is that normal code should not use #ifdef S_IFLNK.
> >
> > Well, "normal code" in Emacs does.  We have 5 such places at this
> > time, if I didn't miss any.
> 
> These #ifdef uses are problematic, and should be removed.

That was my original point, I believe.

> For example,
> on a GNU system (make-symbolic-link A B) signals a file-already-exists
> error if B already exists, but if I'm reading the code correctly this
> does not happen on a system lacking symlinks.  It's better to remove
> unnecessary discrepancies like this.

I'm not sure this particular discrepancy should be removed.  It is not
useful to bitch at the user with secondary issues when there's a
bigger issue looming.  Imagine:

  User: M-x make-symbolic-link RET foo RET bar RET
  Emacs: File bar already exists; make it a link anyway?
  User: yes RET
  Emacs: Making symbolic link: Function not implemented
  User: :-( why *&^%$#@! didn't you tell me that to begin with??

> The affected code is fairly small.  A complete patch for your review
> is below; it removes the problematic #ifdefs and thereby
> shrinks the main Emacs source code by about 25 lines.
> (As usual, I'm attaching the full patch, which includes files
> autogenerated from gnulib.)

Thanks.  However, this is too heaviweight (like always with gnulib).
If all we need is define 2 always-fail functions for w32 and for
MS-DOS, let's just do that on src/w32.c and src/msdos.c, where no one
who is interested in Posix platforms only will ever need to see or
consider them.  It will take less than 10 lines of code, and no
additional changes anywhere.

By contrast, your proposal introduces half a dozen new preprocessor
macros, replacements for 4 functions (2 of which no platforms need),
and additional configury stuff which will only be needed if someone
ever makes the Windows build of Emacs use autotools (and for now, just
increases the labor required to get these changes to work on Windows).

Also, there's an effort in Emacs maintenance to reduce the number of
preprocessor symbols used by the code, so I don't think adding more is
a good idea.

I'm particularly nervous about code that messes with `stat', for which
Emacs has its own from-ground-up implementation that already takes
care of the trailing slashes issue.  It took a lot of labor to get to
the current version, so I'd really prefer not to introduce a wrapper
that was clearly designed to call the version of `stat' in Microsoft's
runtime, which Emacs doesn't use.

An example of what could go wrong with these replacements can be seen
in this excerpt from rpl_lstat:

  +  /* This replacement file can blindly check against '/' rather than
  +     using the ISSLASH macro, because all platforms with '\\' either
  +     lack symlinks (mingw) or have working lstat (cygwin) and thus do
  +     not compile this file.  0 len should have already been filtered
  +     out above, with a failure return of ENOENT.  */
  +  len = strlen (file);
  +  if (file[len - 1] != '/' || S_ISDIR (sbuf->st_mode))
  +    return 0;

The assumption stated in the comment is wrong for DJGPP v2.04, which
is supported in the DOS build, where there's a working `symlink', but
a backslash is a valid directory separator.

More generally, Emacs has accumulated through the years fixes for many
idiosyncrasies of the MS systems.  I would prefer not to touch that by
importing external code that tries to fix the same issues in subtly
different ways.

> @@ -2311,7 +2304,6 @@
>       RETURN_UNGCPRO (call4 (handler, Qmake_symbolic_link, filename,
>   			   linkname, ok_if_already_exists));
> 
> -#ifdef S_IFLNK
>     encoded_filename = ENCODE_FILE (filename);
>     encoded_linkname = ENCODE_FILE (linkname);
> 
> @@ -2338,12 +2330,6 @@
>       }
>     UNGCPRO;
>     return Qnil;
> -
> -#else
> -  UNGCPRO;
> -  xsignal1 (Qfile_error, build_string ("Symbolic links are not supported"));
> -
> -#endif /* S_IFLNK */

I would prefer not to change the user-visible behavior here.  Even if
the target of the symlink does not exist, the current error message is
much more clear and self-explanatory than the half-cryptic
"make-symbolic-link: Function not implemented" (which function? not
implemented by whom? Emacs certainly implements make-symbolic-link, as
"C-h f" will readily show).  How about testing for ENOSYS explicitly?

And there's still the issue with emitting this error message without
calling barf_or_query_if_file_exists, see above.

Thanks.

Re: windows build failure

[Eli Zaretskii]

> Date: Wed, 23 Feb 2011 04:47:40 -0500
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> I'm particularly nervous about code that messes with `stat', for which
> Emacs has its own from-ground-up implementation

That was about the w32 build only, of course.  Sorry for not saying
that explicitly in the first place.

Re: windows build failure

[Paul Eggert]

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

On 02/23/2011 01:47 AM, Eli Zaretskii wrote:

> How about testing for ENOSYS explicitly?

Yes, that's easily enough done, and is in the revised patch at the end
of this message (the full patch, including regenerated files, is attached).

> If all we need is define 2 always-fail functions for w32 and for
> MS-DOS, let's just do that on src/w32.c and src/msdos.c

That sounds fine, and that can be combined with this approach, since
the w32 and DOS ports supply their own hand-built config.h files.

However, we should also work around the known bugs with lstat etc. on
non-Windows platforms, which the Gnulib code handles.  These have to
do with file names that have trailing slashes; some Unixish hosts
incorrectly treat "foo/" as a symbolic link, for example, even when
it's a directory or is nonexistent, if "foo" happens to be a symlink.

> your proposal introduces half a dozen new preprocessor macros,
> replacements for 4 functions (2 of which no platforms need), and
> additional configury stuff which will only be needed if someone ever
> makes the Windows build of Emacs use autotools

I've simplified the proposal so that it introduces into src/config.in
only the preprocessor symbols needed to work around known problems in
Unixish systems mentioned above.  The hand-built Windows ports
shouldn't need to worry about these symbols; they need only to supply
their own always-failing versions of readlink and symlink, which can
be as trivial as a couple of lines in nt/inc/sys/stat.h (e.g.,
"#define readlink(f,b,s) (errno = ENOSYS, -1)").

> Imagine:
>
>    User: M-x make-symbolic-link RET foo RET bar RET
>    Emacs: File bar already exists; make it a link anyway?
>    User: yes RET
>    Emacs: Making symbolic link: Function not implemented
>    User: :-( why *&^%$#@! didn't you tell me that to begin with??

This scenario already occurs in the mainstream code.  If I am running
Emacs on a GNU/Linux host, using files on a file server that does not
support symbolic links, I will already see behavior like that.  It's
more consistent if Emacs's behavior is similar on Windows.

> I'm particularly nervous about code that messes with `stat' ...
> I'd really prefer not to introduce a wrapper

You don't have to.  That wrapper is built only if you're running
ordinary 'configure' on a host with a buggy 'stat'.  The Windows
build doesn't run ordinary 'configure', so it shouldn't need or
build that wrapper.

> The assumption stated in the comment is wrong for DJGPP v2.04, which
> is supported in the DOS build, where there's a working `symlink', but
> a backslash is a valid directory separator.

I'll forward that to the gnulib mailing list, but as per the above
it's irrelevant to the proposed change for Emacs, since the code in
question isn't being used for Emacs.

Here's the revised patch (full version attached).

=== modified file 'ChangeLog'
--- ChangeLog	2011-02-25 07:23:41 +0000
+++ ChangeLog	2011-02-25 20:05:36 +0000
@@ -1,5 +1,11 @@
  2011-02-25  Paul Eggert  <eggert@cs.ucla.edu>
  
+	Simplify symlink portability workaround.
+	* m4/dos.m4: Remove.
+	* aclocal.m4, configure, lib/Makefile.in, lib/gnulib.mk, lib/stat.c:
+	* m4/gl-comp.m4, m4/stat.m4, src/config.in: Regenerate from gnulib.
+	* lib/dosname.h: New file, regenerated from gnulib.
+
  	* configure, lib/Makefile.in, lib/getopt_int.h, lib/gnulib.mk:
  	* lib/stdlib.in.h, m4/stdlib_h.m4: Regenerate.
  
@@ -10,6 +16,17 @@
  
  2011-02-22  Paul Eggert  <eggert@cs.ucla.edu>
  
+	Work around some portability problems with symlinks.
+	* Makefile.in (GNULIB_MODULES): Add lstat, readlink, symlink.
+	* configure.in (lstat, HAVE_LSTAT): Remove special hack.
+	* lib/lstat.c, lib/readlink.c, lib/stat.c, lib/symlink.c:
+	* m4/dos.m4, m4/lstat.m4, m4/readlink.m4, m4/stat.m4, m4/symlink.m4:
+	New files, automatically generated from gnulib.
+	* aclocal.m4, configure, lib/Makefile.in, lib/gnulib.mk:
+	* lib/stdlib.in.h, m4/gl-comp.m4, m4/stdlib_h.m4: Regenerate.
+
+2011-02-22  Paul Eggert  <eggert@cs.ucla.edu>
+
  	Assume S_ISLNK etc. work, since gnulib supports this.
  	* Makefile.in (GNULIB_MODULES): Add sys_stat.
  	* configure.in: Check for lstat and set HAVE_LSTAT=0 if not.

=== modified file 'Makefile.in'
--- Makefile.in	2011-02-22 01:55:20 +0000
+++ Makefile.in	2011-02-22 19:30:07 +0000
@@ -332,7 +332,7 @@
  # as per $(gnulib_srcdir)/DEPENDENCIES.
  GNULIB_MODULES = \
    crypto/md5 dtoastr filemode getloadavg getopt-gnu \
-  ignore-value mktime strftime sys_stat
+  ignore-value lstat mktime readlink strftime symlink sys_stat
  GNULIB_TOOL_FLAGS = \
   --import --no-changelog --no-vc-files --makefile-name=gnulib.mk
  sync-from-gnulib: $(gnulib_srcdir)

=== modified file 'configure.in'
--- configure.in	2011-02-24 04:28:17 +0000
+++ configure.in	2011-02-25 06:42:06 +0000
@@ -2661,15 +2661,6 @@
  gl_ASSERT_NO_GNULIB_TESTS
  gl_INIT
  
-# Emacs does not care about lstat's behavior on files whose names end in
-# trailing slashes, so it does not use the gnulib lstat module.
-# However, Emacs does want the "#define lstat stat" in sys/stat.h
-# when lstat does not exist, so it pretends to use the lstat module
-# even though it implements only the lstat-checking part of that module.
-AC_CHECK_FUNCS_ONCE([lstat])
-test $ac_cv_func_lstat = yes || HAVE_LSTAT=0
-gl_SYS_STAT_MODULE_INDICATOR([lstat])
-
  # UNIX98 PTYs.
  AC_CHECK_FUNCS(grantpt)
  

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-02-25 06:30:50 +0000
+++ src/ChangeLog	2011-02-25 21:20:06 +0000
@@ -1,5 +1,11 @@
  2011-02-25  Paul Eggert  <eggert@cs.ucla.edu>
  
+	Simplify symlink portability workaround.
+	* fileio.c (Fmake_symbolic_link): Treat ENOSYS specially, and
+	generate a special message for it.  Suggested by Eli Zaretskii in
+	<http://lists.gnu.org/archive/html/emacs-devel/2011-02/msg00995.html>.
+	* config.in: Regenerate.
+
  	* dired.c (Ffile_attributes): Increase size of modes from 10 to 12
  	as per recent filemodestring API change.  Reported by Jonas Öster in
  	<http://lists.gnu.org/archive/html/emacs-devel/2011-02/msg01069.html>.
@@ -26,6 +32,13 @@
  
  2011-02-22  Paul Eggert  <eggert@cs.ucla.edu>
  
+	Work around some portability problems with symlinks.
+	* fileio.c (Frename_file, Fmake_symbolic_link, Ffile_symlink_p):
+	Simplify the code by assuming that the readlink and symlink calls
+	exist, even if they always fail on this host.
+	(Ffile_readable_p): Likewise, for fifos.
+	* config.in: Regenerate.
+
  	* dired.c (Ffile_attributes): Simplify and avoid #ifdef.
  
  2011-02-22  Wolfgang Jenkner  <wjenkner@inode.at>  (tiny change)

=== modified file 'src/fileio.c'
--- src/fileio.c	2011-02-22 01:55:20 +0000
+++ src/fileio.c	2011-02-25 21:20:06 +0000
@@ -2178,14 +2178,11 @@
        if (errno == EXDEV)
  	{
            int count;
-#ifdef S_IFLNK
            symlink_target = Ffile_symlink_p (file);
            if (! NILP (symlink_target))
              Fmake_symbolic_link (symlink_target, newname,
                                   NILP (ok_if_already_exists) ? Qnil : Qt);
-          else
-#endif
-	  if (!NILP (Ffile_directory_p (file)))
+	  else if (!NILP (Ffile_directory_p (file)))
  	    call4 (Qcopy_directory, file, newname, Qt, Qnil);
  	  else
  	    /* We have already prompted if it was an integer, so don't
@@ -2197,11 +2194,7 @@
  	  count = SPECPDL_INDEX ();
  	  specbind (Qdelete_by_moving_to_trash, Qnil);
  
-	  if (!NILP (Ffile_directory_p (file))
-#ifdef S_IFLNK
-	      && NILP (symlink_target)
-#endif
-	      )
+	  if (!NILP (Ffile_directory_p (file)) && NILP (symlink_target))
  	    call2 (Qdelete_directory, file, Qt);
  	  else
  	    Fdelete_file (file, Qnil);
@@ -2311,7 +2304,6 @@
      RETURN_UNGCPRO (call4 (handler, Qmake_symbolic_link, filename,
  			   linkname, ok_if_already_exists));
  
-#ifdef S_IFLNK
    encoded_filename = ENCODE_FILE (filename);
    encoded_linkname = ENCODE_FILE (linkname);
  
@@ -2333,17 +2325,17 @@
  	      return Qnil;
  	    }
  	}
+      if (errno == ENOSYS)
+	{
+	  UNGCPRO;
+	  xsignal1 (Qfile_error,
+		    build_string ("Symbolic links are not supported"));
+	}
  
        report_file_error ("Making symbolic link", list2 (filename, linkname));
      }
    UNGCPRO;
    return Qnil;
-
-#else
-  UNGCPRO;
-  xsignal1 (Qfile_error, build_string ("Symbolic links are not supported"));
-
-#endif /* S_IFLNK */
  }
  
  \f
@@ -2482,7 +2474,7 @@
    return Qnil;
  #else /* not DOS_NT and not macintosh */
    flags = O_RDONLY;
-#if defined (S_IFIFO) && defined (O_NONBLOCK)
+#ifdef O_NONBLOCK
    /* Opening a fifo without O_NONBLOCK can wait.
       We don't want to wait.  But we don't want to mess wth O_NONBLOCK
       except in the case of a fifo, on a system which handles it.  */
@@ -2584,6 +2576,10 @@
    (Lisp_Object filename)
  {
    Lisp_Object handler;
+  char *buf;
+  int bufsize;
+  int valsize;
+  Lisp_Object val;
  
    CHECK_STRING (filename);
    filename = Fexpand_file_name (filename, Qnil);
@@ -2594,13 +2590,6 @@
    if (!NILP (handler))
      return call2 (handler, Qfile_symlink_p, filename);
  
-#ifdef S_IFLNK
-  {
-  char *buf;
-  int bufsize;
-  int valsize;
-  Lisp_Object val;
-
    filename = ENCODE_FILE (filename);
  
    bufsize = 50;
@@ -2635,10 +2624,6 @@
    xfree (buf);
    val = DECODE_FILE (val);
    return val;
-  }
-#else /* not S_IFLNK */
-  return Qnil;
-#endif /* not S_IFLNK */
  }
  
  DEFUN ("file-directory-p", Ffile_directory_p, Sfile_directory_p, 1, 1, 0,



[-- Attachment #2: symlink-patch2.txt.gz --]
[-- Type: application/x-gzip, Size: 30751 bytes --]

Re: Remove S_IFLNK (was: windows build failure)

[Eli Zaretskii]

> Date: Fri, 25 Feb 2011 13:28:24 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> > If all we need is define 2 always-fail functions for w32 and for
> > MS-DOS, let's just do that on src/w32.c and src/msdos.c
> 
> That sounds fine, and that can be combined with this approach, since
> the w32 and DOS ports supply their own hand-built config.h files.
> 
> However, we should also work around the known bugs with lstat etc. on
> non-Windows platforms, which the Gnulib code handles.  These have to
> do with file names that have trailing slashes; some Unixish hosts
> incorrectly treat "foo/" as a symbolic link, for example, even when
> it's a directory or is nonexistent, if "foo" happens to be a symlink.

I'd say if we didn't hear about these problems until now, they are not
important enough to justify such heavyweight changes and added
complexity with remapping functions, replacing them, etc.

However, if Stefan and Chong are okay with this, so be it.

> > Imagine:
> >
> >    User: M-x make-symbolic-link RET foo RET bar RET
> >    Emacs: File bar already exists; make it a link anyway?
> >    User: yes RET
> >    Emacs: Making symbolic link: Function not implemented
> >    User: :-( why *&^%$#@! didn't you tell me that to begin with??
> 
> This scenario already occurs in the mainstream code.  If I am running
> Emacs on a GNU/Linux host, using files on a file server that does not
> support symbolic links, I will already see behavior like that.  It's
> more consistent if Emacs's behavior is similar on Windows.

I could quote Emerson about foolish consistency here (a Posix host
would have hard time knowing in advance that the call will fail,
whereas a Windows host doesn't have that problem), but again, if I'm
the only one who cares about this, I give up.

> Here's the revised patch (full version attached).

Thanks.  When you decide to merge it, please give me a day or two to
add the stub functions for Windows and DOS, so that when you commit,
these two ports will still build.

Re: Remove S_IFLNK

[Stefan Monnier]

> I'd say if we didn't hear about these problems until now, they are not
> important enough to justify such heavyweight changes and added
> complexity with remapping functions, replacing them, etc.
> However, if Stefan and Chong are okay with this, so be it.

I don't care much either way.  To me this seems like it's trying to fix
something nobody cares about, but at the same time, the author of the
patch obviously cares enough about it to come up with a patch, so...

If the resulting code is cleaner and/or more efficient, I guess it's
good, but I'm not sure that it is.


        Stefan

Re: Remove S_IFLNK

[Paul Eggert]

On 02/26/2011 10:01 AM, Eli Zaretskii wrote:
> they are not important enough to justify such heavyweight changes

The changes are not heavyweight.  In Emacs proper, the latest
proposal deletes 26 and inserts 9 lines, and I could easily reduce
that "9" to a "3" if you hadn't asked to retain a specially-worded
diagnostic for ENOSYS failure.

It's true that the change imports more stuff from gnulib, but
that's OK too.  It's normal, and expected, and is good software
engineering practice, to move the porting complexity out of Emacs
proper and into the porting code.

> When you decide to merge it, please give me a day or two to add
> the stub functions for Windows and DOS, so that when you commit,
> these two ports will still build.

OK, I'll wait a couple of days before merging it.

Re: Remove S_IFLNK

[Eli Zaretskii]

> Date: Sat, 26 Feb 2011 14:36:19 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org, monnier@iro.umontreal.ca, cyd@stupidchicken.com
> 
> > When you decide to merge it, please give me a day or two to add
> > the stub functions for Windows and DOS, so that when you commit,
> > these two ports will still build.
> 
> OK, I'll wait a couple of days before merging it.

You can go ahead, I installed the stubs.