Re: windows build failure

[Paul Eggert]

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 *&address@hidden 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  <address@hidden>
 
+	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  <address@hidden>
 
+	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  <address@hidden>
+
        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  <address@hidden>
 
+	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  <address@hidden>
 
+	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  <address@hidden>  (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 */
 }
 
 
@@ -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,

symlink-patch2.txt.gz
Description: GNU Zip compressed data