Re: guile 2.0.9 build on mingw

[ludo@gnu.org (Ludovic Courtès)]

[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]

Eli Zaretskii <eliz@gnu.org> skribis:

>> Date: Tue, 11 Jun 2013 19:53:21 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: ludo@gnu.org, guile-user@gnu.org
>> 
>> Btw, --disable-posix disables too much, for no good reason.  E.g., it
>> removes such innocent and important functionalities as "chdir",
>> "open", and "close".  This is unnecessary, especially since most of
>> the functions disabled by --disable-posix already have HAVE_foo guards
>> around them, so systems that don't have them will not have the
>> corresponding Guile feature.  I will try to lift as much of the
>> disabled functionality as MinGW can bear.
>
> Please find the changes to do this below.  I removed most of the
> HAVE_POSIX ifdef's, as they are no longer needed.

Sorry I had overlooked this one.  It looks nice!

We would need the copyright to be assigned to the FSF before it can be
committed.  How does that sound?  If you’re willing to do so, I can send
you the paperwork off-line.

> The bug report at http://bugs.gnu.org/14171 says that when Guile is
> compiled with enable-posix, it "can't start the REPL".  Can someone
> please show me a simple way of trying that?  I'd like to see if my
> build, which didn't use --disable-posix, has any problems in that
> area, and if so try to debug them.

No idea.

A few comments:

> --- libguile/filesys.c~0	2013-04-09 09:52:31.000000000 +0300
> +++ libguile/filesys.c	2013-06-12 13:41:31.244477700 +0300
> @@ -112,7 +112,12 @@
>  
>  /* Some more definitions for the native Windows port. */
>  #ifdef __MINGW32__
> -# define fsync(fd) _commit (fd)
> +# define fsync(fd)    _commit (fd)
> +# define WIN32_LEAN_AND_MEAN

What does that mean?  :-)

> +# include <windows.h>
> +/* FIXME: Should use 'link' module from gnulib.  */
> +# define link(f1,f2)  CreateHardLink(f2, f1, NULL)
> +# define HAVE_LINK 1

Let’s do it then, and remove that part of the patch.

> -#ifdef HAVE_POSIX
> -

Note that HAVE_POSIX really means --enable-posix.  Since this patch
keeps that option, it should probably keep the #ifdefs.  That would be a
topic for a separate patch.

> @@ -921,7 +958,7 @@ SCM_DEFINE (scm_setegid, "setegid", 1, 0
>  {
>    int rv;
>  
> -#ifdef HAVE_SETEUID
> +#ifdef HAVE_SETEGID

Oops, applied.

> @@ -1320,7 +1357,130 @@ scm_open_process (SCM mode, SCM prog, SC
>    }
>  #endif
>  
> +#ifdef HAVE_FORK
>    pid = fork ();
> +#elif defined(__MINGW32__)
> +  {
> +    int save_stdin = -1, save_stdout = -1;
> +    int errno_save;
> +
> +    if (reading)

[...]

Could this MinGW-specific code be moved to a separate function?

I think the patch also needs something like this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 309 bytes --]

--- a/configure.ac
+++ b/configure.ac
@@ -760,7 +760,7 @@ AC_CHECK_FUNCS([DINFINITY DQNAN cexp chsize clog clog10 ctermid		\
   sched_setaffinity sendfile])
 
 AM_CONDITIONAL([BUILD_ICE_9_POPEN],
-  [test "x$enable_posix" = "xyes" && test "x$ac_cv_func_fork" = "xyes"])
+  [test "x$enable_posix" = "xyes"])
 

[-- Attachment #3: Type: text/plain, Size: 21 bytes --]


Thanks,
Ludo’.