[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Minor build fixes
From: |
Gavin Smith |
Subject: |
Re: Minor build fixes |
Date: |
Thu, 2 Oct 2014 21:37:00 +0100 |
On Thu, Oct 2, 2014 at 5:46 PM, Ken Brown <address@hidden> wrote:
> I just took over as the Cygwin maintainer of Texinfo, and I bumped into a
> few (easily fixed) build problems. The first two exist in texinfo-5.2, and
> the rest only in the trunk.
Thanks for the report. Comments below:
> 1. The compiler produces lots of warnings about setmode being redefined. It
> would be simple to silence the warnings, but the real issue in my opinion is
> that Cygwin should be treated the same as Unix, so that there's no need for
> setmode at all:
>
> Index: system.h
> ===================================================================
> --- system.h (revision 5852)
> +++ system.h (working copy)
> @@ -119,6 +119,13 @@
> - text files can have their lines ended either with \n or with \r\n
> pairs;
> These are all parameterized here except the last, which is
> handled by the source code as appropriate (mostly, in info/). */
> +
> +/* Cygwin is a Posix system even though it allows the use of O_BINARY. */
> +#ifdef __CYGWIN__
> +# undef O_BINARY
> +# define O_BINARY 0
> +#endif
> +
> #ifndef O_BINARY
> # ifdef _O_BINARY
> # define O_BINARY _O_BINARY
> @@ -152,23 +159,13 @@
> # define HAVE_LONG_FILENAMES(dir) (1)
> # define NULL_DEVICE "NUL"
> # endif /* O_BINARY && !__MSDOS__ */
> -# ifdef __CYGWIN__
> -# define DEFAULT_TMPDIR "/tmp/"
> -# define PATH_SEP ":"
> -# define STRIP_DOT_EXE 0
> -# undef NULL_DEVICE
> -# define NULL_DEVICE "/dev/null"
> -# define PIPE_USE_FORK 1
> -# else /* O_BINARY && !__CYGWIN__ */
> -# ifdef __MINGW32__
> -# define SET_SCREEN_SIZE_HELPER terminal_prep_terminal()
> -# endif /* _WIN32 */
> -# define DEFAULT_TMPDIR "c:/"
> -# define PATH_SEP ";"
> -# define STRIP_DOT_EXE 1
> -# define PIPE_USE_FORK 0
> -# endif /* O_BINARY && !__CYGWIN__ */
> - /* Back to any O_BINARY system. */
> +# ifdef __MINGW32__
> +# define SET_SCREEN_SIZE_HELPER terminal_prep_terminal()
> +# endif /* _WIN32 */
> +# define DEFAULT_TMPDIR "c:/"
> +# define PATH_SEP ";"
> +# define STRIP_DOT_EXE 1
> +# define PIPE_USE_FORK 0
> # define FILENAME_CMP mbscasecmp
> # define FILENAME_CMPN mbsncasecmp
> # define FOPEN_RBIN "rb"
>
If I understand this right, O_BINARY is being checked to see if it is
a Posix-style system, right, but Cygwin has O_BINARY defined? O_BINARY
is not actually being set in system.h for the benefit of source code
including it? I noticed in info/filesys.c O_BINARY is used for opening
a file, so will it matter if this value is 0 under Cygwin?
The problem with setmode is that it is being redefined as "_setmode",
correct? Does anyone know what systems need to use _setmode instead of
setmode?
I see the SET_BINARY macro, whose expansion uses setmode, is used in
info/makedoc.c. Would it matter if setmode wasn't used here under
Cygwin?
Does it matter that the other macros will get the Unix definitions,
e.g. HAVE_DRIVE will always be false, comparison of filenames will be
case-sensitive, "\" will not be allowed as a directory separator?
> 2. The following patch provides the prototype for ioctl:
>
> Index: info/termdep.h
> ===================================================================
> --- info/termdep.h (revision 5852)
> +++ info/termdep.h (working copy)
> @@ -49,7 +49,7 @@
> # endif /* !HAVE_TERMIO_H */
> #endif /* !HAVE_TERMIOS_H */
>
> -#ifdef GWINSZ_IN_SYS_IOCTL
> +#ifndef __MINGW32__
> # include <sys/ioctl.h>
> #endif
>
I see the same thing (conditional on __MINGW32__) is done when
including sys/ioctl.h in info/ioctl.h and info/man.h. It looks like
GWINSZ_IN_SYS_IOCTL is set by the AC_HEADER_TIOCGWINSZ macro in
configure.ac. However, your patch is probably what we want because
we're not just including <sys/ioctl.h> to get TIOCGWINSZ (although I
don't understand why you would want the definition of TIOCGWINSZ and
/not/ have the definition of ioctl, so maybe I'm missing something?)
> 3. The following is needed for linking, now that info/info-utils.c uses
> libiconv:
>
> Index: info/Makefile.am
> ===================================================================
> --- info/Makefile.am (revision 5852)
> +++ info/Makefile.am (working copy)
> @@ -27,7 +27,7 @@
> -DINFODIR=\"$(infodir)\" \
> -DINFODIR2=\"$(datadir)/info\"
>
> -LDADD = $(top_builddir)/gnulib/lib/libgnu.a $(TERMLIBS) $(LIBINTL)
> +LDADD = $(top_builddir)/gnulib/lib/libgnu.a $(TERMLIBS) $(LIBINTL)
> $(LIBICONV)
>
> EXTRA_DIST = README pcterm.c
>
This looks good. gnulib-tool should report on what should be added to
LDADD. I ran "../gnulib/gnulib-HEAD-d9361da/gnulib-tool --update
--dry-run" (ignore the path to gnulib-tool), and in the output it did
suggest $(LIBICONV) in LDADD.
> 4. info-utils.c wouldn't compile for me because Cygwin doesn't have the
> header <nl_types.h>. But this doesn't seem to be needed; the compilation
> worked if I just removed that include:
>
> Index: info/info-utils.c
> ===================================================================
> --- info/info-utils.c (revision 5852)
> +++ info/info-utils.c (working copy)
> @@ -24,7 +24,6 @@
> #include "info-utils.h"
> #include "tag.h"
>
> -#include <nl_types.h>
> #include <langinfo.h>
> #if HAVE_ICONV
> # include <iconv.h>
>
OK, it doesn't look like it's needed.
> 5. Now that infokeys is no longer a standalone program, the attempt to
> create a man page for it fails. I assume the following is the right fix:
Yes, I would assume so.
Re: Minor build fixes, Gavin Smith, 2014/10/06