Re: windows build failure

[Eli Zaretskii]

> Date: Tue, 22 Feb 2011 13:44:48 -0800
> From: Paul Eggert <address@hidden>
> CC: address@hidden
> 
> [1:text/plain Hide]
> On 02/22/2011 02:49 AM, Eli Zaretskii wrote:
> >> From: Paul Eggert<address@hidden>
> >> 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 *&address@hidden 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.