On Thu, Oct 2, 2014 at 5:46 PM, Ken Brown <kbr...@cornell.edu> 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.