Re: Patches for MSVC

[Michael Goffioul]

On Sun, Feb 21, 2010 at 3:22 PM, John W. Eaton <address@hidden> wrote:
> I intend to avoid the problems with gnulib definitions in a different
> way.  See the messages I posted in the thread
>
>  
> https://www-old.cae.wisc.edu/pipermail/octave-maintainers/2010-February/015251.html
>
> so please don't check in the parts of the changeset that deal with
> gnulib symbol problems.

I know about the proposed change. I still included those changes
in the patch as at the moment, octa

>
> See below for comments on the other changes.
>
> | diff -r f0ac2fa91733 bootstrap.conf
> | --- a/bootstrap.conf  Thu Feb 11 14:13:28 2010 -0500
> | +++ b/bootstrap.conf  Sun Feb 21 14:31:37 2010 +0000
> | @@ -23,6 +23,7 @@
> |    fcntl
> |    fnmatch
> |    getcwd
> | +  getdtablesize
> |    gethostname
> |    getopt-gnu
> |    gettimeofday
>
> This problem should already be fixed by an added dependency in the
> gnulib modules.  Update your gnulib checkout and it should be handled
> correctly without having to explicitlly list this module that we don't
> use directly.
>
> | diff -r f0ac2fa91733 configure.ac
> | --- a/configure.ac    Thu Feb 11 14:13:28 2010 -0500
> | +++ b/configure.ac    Sun Feb 21 14:31:37 2010 +0000
> | @@ -430,6 +430,11 @@
> |  AC_SUBST(XTRA_CFLAGS)
> |  AC_SUBST(XTRA_CXXFLAGS)
> |
> | +## Avoid #define of min/max from windows.h header
> | +if test "$have_msvc" = "yes"; then
> | +  AC_DEFINE(NOMINMAX, 1, [Define if you want to avoid min/max macro 
> definition in Windows headers])
> | +fi
> | +
>
> I don't see where this is used, so do we need it?
>
> |  ### Use -static if compiling on Alpha OSF/1 1.3 systems.
> |
> |  case "$canonical_host_type" in
> | @@ -605,10 +610,10 @@
> |
> |  ### Check for pcre/regex library.
> |
> | -## check for pcre-config, and if so, set CPPFLAGS appropriately
> | +## check for pcre-config, and if so, set XTRA_CXXFLAGS appropriately
> |  AC_CHECK_PROG(WITH_PCRE_CONFIG, pcre-config, yes, no)
> |  if test $WITH_PCRE_CONFIG = yes ; then
> | -  CPPFLAGS="$CPPFLAGS $(pcre-config --cflags)"
> | +  XTRA_CXXFLAGS="$XTRA_CXXFLAGS $(pcre-config --cflags)"
> |  fi
> |
> |  ## NB: no need to do separate check for pcre.h header -- checking macros 
> is good enough
>
> Please check this one in as a separate changeset since it fixes a
> specific problem that is independent of the rest.
>
> | diff -r f0ac2fa91733 libcruft/Makefile.am
> | --- a/libcruft/Makefile.am    Thu Feb 11 14:13:28 2010 -0500
> | +++ b/libcruft/Makefile.am    Sun Feb 21 14:31:37 2010 +0000
> | @@ -32,7 +32,8 @@
> |  libcruft_la_CPPFLAGS = @CRUFT_DLL_DEFS@
> |
> |  libcruft_la_LDFLAGS = \
> | -  -release $(version) $(NO_UNDEFINED_LDFLAG) @XTRA_CRUFT_SH_LDFLAGS@
> | +  -release $(version) $(NO_UNDEFINED_LDFLAG) @XTRA_CRUFT_SH_LDFLAGS@ \
> | +  -bindir $(bindir)
> |
> |  libcruft_la_LIBADD = $(LAPACK_LIBS) $(BLAS_LIBS) $(FLIBS)
>
> Is the -bindir option handled by libtool?  What does it do?  If it is
> documented, I don't see it.  In any case, please check in a separate
> changeset to add -bindir to LDFLAGS.
>
> | diff -r f0ac2fa91733 libcruft/misc/quit.h
> | --- a/libcruft/misc/quit.h    Thu Feb 11 14:13:28 2010 -0500
> | +++ b/libcruft/misc/quit.h    Sun Feb 21 14:31:37 2010 +0000
> | @@ -35,9 +35,8 @@
> |
> |  #if defined (__WIN32__) && ! defined (_POSIX_VERSION)
> |
> | +#define WIN32_LEAN_AND_MEAN
> |  #include <windows.h>
> | -#undef min
> | -#undef max
> |
> |  CRUFT_API extern void w32_sigint_init (void);   /* setup */
> |  CRUFT_API extern void w32_raise_final (void);   /* tear down */
>
> Please check in all the WIN32_LEAN_AND_MEAN changes as a separate
> changeset.
>
> | diff -r f0ac2fa91733 liboctave/oct-glob.cc
> | --- a/liboctave/oct-glob.cc   Thu Feb 11 14:13:28 2010 -0500
> | +++ b/liboctave/oct-glob.cc   Sun Feb 21 14:31:39 2010 +0000
> | @@ -24,6 +24,7 @@
> |  #include <config.h>
> |  #endif
> |
> | +#include <algorithm>
> |  #include <string>
> |
> |  #include <fnmatch.h>
> | @@ -78,6 +79,13 @@
> |          {
> |            glob_t glob_info;
> |
> | +#if defined (OCTAVE_HAVE_WINDOWS_FILESYSTEM) \
> | +       && ! defined (OCTAVE_HAVE_POSIX_FILESYSTEM)
> | +           std::replace_if (xpat.begin (), xpat.end (),
> | +                            std::bind2nd (std::equal_to<char> (), '\\'),
> | +                            '/');
> | +#endif
> | +
> |            int err = ::glob (xpat.c_str (), GLOB_NOSORT, 0, &glob_info);
> |
> |            if (! err)
> | @@ -98,7 +106,19 @@
> |                    retval.resize (k+n);
> |
> |                    for (int j = 0; j < n; j++)
> | -                    retval[k++] = matches[j];
> | +                 {
> | +                   std::string tmp = matches[j];
> | +
> | +#if defined (OCTAVE_HAVE_WINDOWS_FILESYSTEM) \
> | +                   && ! defined (OCTAVE_HAVE_POSIX_FILESYSTEM)
> | +                       std::replace_if (tmp.begin (), tmp.end (),
> | +                                        std::bind2nd (std::equal_to<char> 
> (),
> | +                                                      '/'),
> | +                                        '\\');
> | +#endif
> | +
> | +                   retval[k++] = tmp;
> | +                 }
> |                  }
> |
> |                globfree (&glob_info);
>
> Please check this change in as a separate changeset.  Please also use
> spaces instead of tabs for indenting.
>
> | diff -r f0ac2fa91733 src/ov-bool.h
> | --- a/src/ov-bool.h   Thu Feb 11 14:13:28 2010 -0500
> | +++ b/src/ov-bool.h   Sun Feb 21 14:31:40 2010 +0000
> | @@ -48,6 +48,7 @@
> |  // Real scalar values.
> |
> |  class
> | +OCTINTERP_API
> |  octave_bool : public octave_base_scalar<bool>
> |  {
> |  public:
>
> Please check in all the changes that add these tags as a separate
> changeset.
>
> | diff -r f0ac2fa91733 src/sighandlers.cc
> | --- a/src/sighandlers.cc      Thu Feb 11 14:13:28 2010 -0500
> | +++ b/src/sighandlers.cc      Sun Feb 21 14:31:40 2010 +0000
> | @@ -178,15 +178,20 @@
> |    act.sa_handler = handler;
> |    act.sa_flags = 0;
> |
> | +#if defined (SIGALRM)
> |    if (sig == SIGALRM)
> |      {
> |  #if defined (SA_INTERRUPT)
> |        act.sa_flags |= SA_INTERRUPT;
> |  #endif
> |      }
> | +#endif
> |  #if defined (SA_RESTART)
> | +#if defined (SIGALRM)
> | +  else
> | +#endif
> |    // FIXME -- Do we also need to explicitly disable SA_RESTART?
> | -  else if (restart_syscalls)
> | +  if (restart_syscalls)
> |      act.sa_flags |= SA_RESTART;
> |  #endif
>
> Please check this in as a separate changeset.
>
> Thanks,
>
> jwe
>