Re: MS-Windows build using Posix configury

[Paul Eggert]

Thanks for getting this to work.

On 04/16/13 11:24, Eli Zaretskii wrote:

> +lib-src/emacsclient.res
> ...
> +nt/emacs.res
> +src/temacs.map
> +src/temacs.tmp

How about adding *.res, *.map, *.tmp instead?  That should simplify
future maintenance.

> -         cd $(DESTDIR)${bindir} && $(LN_S) $(EMACSFULL) $(EMACS); \
> +         cd $(DESTDIR)${bindir} && $(LN_EMACS) $(EMACSFULL) $(EMACS); \

Why prefer hard links on Windows but not everywhere else?
How about something like the following instead?

    cd $(DESTDIR)${bindir} && { \
      ln $(EMACSFULL) $(EMACS) 2>/dev/null || $(LN_S) $(EMACSFULL) $(EMACS); \
    }; \

That way, we don't need LN_EMACS.

> +     if test "x$(NTDIR)" != "x"; then \
> +       cd $(NTDIR) && \
> +         $(MAKE) install $(MFLAGS) prefix=${prefix} \
> +           exec_prefix=${exec_prefix} bindir=${bindir} \
> +           libexecdir=${libexecdir} archlibdir=${archlibdir} \
> +           INSTALL_STRIP=${INSTALL_STRIP}; \
> +     fi

How about breaking this out into a separate rule (install-arch-dep-nt,
say) and executing only on nt platforms?  That will cause the output
of 'make' to be shorter on non-mingw platforms.  It could be
implemented something like this, say:

install-arch-dep: install-arch-dep-$(NTDIR)
install-arch-dep-:
install-arch-dep-nt:
        cd $(NTDIR) && ...

> +     if test "x$(NTDIR)" != "x"; then \
> +       cd $(NTDIR) &&                                        \
> +        $(MAKE) $(MFLAGS) uninstall                  \
> +           prefix=${prefix} exec_prefix=${exec_prefix}       \
> +           bindir=${bindir} libexecdir=${libexecdir} 
> archlibdir=${archlibdir}; \
> +     fi

Similarly, this should be broken out into a rule uninstall-nt.

>         gnu-linux|freebsd|netbsd)
>           AC_DEFINE(HAVE_SOUND, 1, [Define to 1 if you have sound support.])
>           ;;
> +       mingw32)
> +         AC_DEFINE(HAVE_SOUND, 1, [Define to 1 if you have sound support.])
> +         ;;

The two cases should be combined, by appending "|mingw32" to the
previous case.

>    if test "${opsys}" != "cygwin"; then
> -    AC_MSG_ERROR([Using w32 with an autotools build is only supported for 
> Cygwin.])
> +    if test "${opsys}" != "mingw32"; then
> +      AC_MSG_ERROR([Using w32 with an autotools build is only supported for 
> Cygwin and MinGW32.])
> +    fi
>    fi

Please turn this into a "case" instead of a nested "if", as
"case" is easier to read for this sort of thing.


> +  AC_CHECK_HEADER([windows.h], [HAVE_W32=yes],
> +                  [AC_MSG_ERROR([The windows.h header file is required,
> +               but cannot be found.])])

This code is redundant and can be removed; the next test checks that
windows.h compiles, which is a stronger test anyway.  Also, there's an
earlier AC_CHECK_HEADER([windows.h]...) test that is already there;
perhaps it can be removed too?

> +if test "${opsys}" = "mingw32"; then
> +  if test "${with_xpm}" != "no"; then
> +    AC_CHECK_HEADER(X11/xpm.h, HAVE_XPM=yes, HAVE_XPM=no, [
> +#define FOR_MSW 1])
> +  fi
> +
> +  if test "${HAVE_XPM}" = "yes"; then
> +    AC_DEFINE(HAVE_XPM, 1, [Define to 1 if you have the Xpm library 
> (-lXpm).])
> +  fi
> +fi

There are a number of instances of code like this, where a reasonably
large amount of code is duplicated and surrounded by "if test
"${opsys}" = "mingw32".  It'd be better to duplicate less code and do
the mingw32-specific stuff more compactly.  Perhaps this is too much
to do right now, but at least this deserves a FIXME, as in the long
run this is increasing maintenance effort.

> +if test "${opsys}" = "mingw32"; then
> +  AC_DEFINE(NULL_DEVICE, ["NUL:"], [Name of the file to open to get
> +    a null file, or a data sink.])
> +else
> +  AC_DEFINE(NULL_DEVICE, ["/dev/null"], [Name of the file to open to get
> +    a null file, or a data sink.])
> +fi

For stuff like this, how about putting it into conf_post.h instead?
That'll be clearer and will help 'configure' run faster.  E.g.,
something like this in conf_post.h:

   /* Override the default definitions.  */
   #ifdef __MINGW32__
   # define NULL_DEVICE "NUL:"
   # define IS_DIRECTORY_SEP(c) ((c) == '/' || c == '\')
   ...
   #endif

   /* Default definitions, suitable for GNU and POSIXish platforms.  */

   /* Name of the file to open to get a empty input file,
      or a data sink output file.  */
   #ifndef NULL_DEVICE
   # define NULL_DEVICE "/dev/null"
   #endif

   /* Return true if C is a directory separator in file names.  */
   #ifndef IS_DIRECTORY_SEP
   # define IS_DIRECTORY_SEP(c) ((c) == '/')
   #endif

and remove all that stuff from configure.ac.

> -       regex.o $(LOADLIBES) -o etags
> +       regex.o $(LOADLIBES) $(NTLIB) -o etags${EXEEXT}
> ...
> -       ${srcdir}/ebrowse.c $(LOADLIBES) -o ebrowse
> +       ${srcdir}/ebrowse.c $(LOADLIBES) $(NTLIB) -o ebrowse${EXEEXT}
> ... etc ...

Perhaps append $(NTLIB) to LOADLIBES's value instead?