octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Patches for MSVC


From: Michael Goffioul
Subject: Re: Patches for MSVC
Date: Sun, 21 Feb 2010 15:47:57 +0000

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, octave is simply not compilable
under MSVC and maybe under other systems, as problems have
been reported under Mac OS X and MinGW.

I don't know how much time it will take to get a proper working
solution within gnulib. This was only a proposed temporary
solution.

> | 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?

It avoids having to #undef min/max later on. I removed all
occrurences of that hack.

> | 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.

It tells libtool where binaries should be installed. AFAIK This should also
be required by MinGW or cygwin. Under Win32 systems, the libtool
library that is installed is the import library. But you also need to install
the actual DLL in the bin directory. This is done by libtool in a postinstall
step. By default (if no -bindir flag is given), the target bin dir is computed
relatively to the target install lib dir as "../bin/". But as octave
libs are not
installed in the standard ".../lib/" directory, the DLL's get installed
incorrectly in ".../lib/octave-3-3-50+/../bin/" == ".../lib/bin/".

If this change is not required by cygwin or MinGW, then I'm ok to not
apply this change to the octave source tree. I'll let Benjamin and Marco
comment on this.

> | 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.

Is it ok to also include the '#undef' and the -DNOMINMAX addition
(see above)? Those usually appears at the same location, so separating
them will make it more difficult for me to do.

> | 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.

Sorry about that. I forgot to change my vim settings.

Michael.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]