Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-24 Thread Eli Zaretskii 
> From: Andy Wingo 
> Cc: guile-devel@gnu.org
> Date: Sat, 23 Jul 2016 22:36:17 +0200
> 
> >> > * libguile/stime.c (scm_strftime) [__MINGW32__]: Don't use the
> >> > trick of appending "0" to the time-zone string, Windows runtime
> >> > doesn't support that.
> >> > +#ifndef __MINGW32__
> >> > +/* Don't do this for MinGW: it only supports fixed-format
> >> > +   TTTnnnDDD TZ specifications, and gets confused if a zero is
> >> > +   appended.  */
> >> 
> >> This patch disables the setzone() call entirely; seems to be the wrong
> >> thing, given that MinGW doesn't appear to have struct tm->tm_zone.  What
> >> if we just disable appending the 0 to the time zone?
> >
> > I'm not sure I understand what you have in mind, but if you show a
> > patch and a simple test, I can run it here.
> 
> The current code:
> 
> #if !defined (HAVE_STRUCT_TM_TM_ZONE)
> /* it seems the only way to tell non-GNU versions of strftime what
>zone to use (for the %Z format) is to set TZ in the
>environment.  interrupts and thread switching must be deferred
>until TZ is restored.  */
> char **oldenv = NULL;
> SCM zone_spec = SCM_SIMPLE_VECTOR_REF (stime, 10);
> int have_zone = 0;
> 
> if (scm_is_true (zone_spec) && scm_c_string_length (zone_spec) > 0)
>   {
> /* it's not required that the TZ setting be correct, just that
>it has the right name.  so try something like TZ=EST0.
>using only TZ=EST would be simpler but it doesn't work on
>some OSs, e.g., Solaris.  */
> SCM zone =
>   scm_string_append (scm_list_2 (zone_spec,
>  scm_from_locale_string ("0")));
> 
> have_zone = 1;
> SCM_CRITICAL_SECTION_START;
> oldenv = setzone (zone, SCM_ARG2, FUNC_NAME);
>   }
> #endif
> 
> You said that appending the 0 confuses MinGW, so you skip this section
> and the corresponding one after the nstrftime.  But you're skipping the
> setzone too, so I don't see how the strftime tests would work.

Appending 0 confuses MinGW because time-zone strings on Windows can be
something like "Jerusalem Standard Time", in which case appending 0
will not DTRT.  If the TZ string is a 3-letter name like EST, then
EST0 will indeed work on Windows.

But what setzone does causes much more trouble: it replaces the
environ array with one that has a single variable TZ, and that
confuses the heck out of the Windows C runtime.  Windows programs
cannot run with an empty environ.

Frankly, I don't understand why all this stuff with setzone is needed,
because on Windows all that's required to do what I think this code is
trying to do is this:

  putenv ("TZ=whatever");
  tzset ();
  tm->tm_isdst = 0;

The last bit is required for when the "whatever" zone doesn't have a
DST name (like "EDT"), in which case %Z will produce an empty string.
The code already does call tzset, so it should simply call putenv and
set the isdst member, instead of using this strange method implemented
in setzone.  Am I missing something?

> According to POSIX, our TZ value is valid:
> 
>   http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
> 
> What is the MinGW restriction?  You mention it in the comment but I
> don't understand the comment.

If you are asking about the forms of the TZ variable, they are
documented here:

  https://msdn.microsoft.com/en-us/library/90s5c885(v=vs.71).aspx

But that's not the important problem with setzone.

Of course, the same problems exist with localtime and mktime: they
should also call putenv instead of setzone.

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-23 Thread Andy Wingo 
On Sat 23 Jul 2016 15:07, Eli Zaretskii  writes:

>> From: Andy Wingo 
>> Cc: guile-devel@gnu.org
>> Date: Sat, 23 Jul 2016 14:15:06 +0200
>> 
>> > * libguile/stime.c (scm_strftime) [__MINGW32__]: Don't use the
>> > trick of appending "0" to the time-zone string, Windows runtime
>> > doesn't support that.
>> > +#ifndef __MINGW32__
>> > +/* Don't do this for MinGW: it only supports fixed-format
>> > +   TTTnnnDDD TZ specifications, and gets confused if a zero is
>> > +   appended.  */
>> 
>> This patch disables the setzone() call entirely; seems to be the wrong
>> thing, given that MinGW doesn't appear to have struct tm->tm_zone.  What
>> if we just disable appending the 0 to the time zone?
>
> I'm not sure I understand what you have in mind, but if you show a
> patch and a simple test, I can run it here.

The current code:

#if !defined (HAVE_STRUCT_TM_TM_ZONE)
/* it seems the only way to tell non-GNU versions of strftime what
   zone to use (for the %Z format) is to set TZ in the
   environment.  interrupts and thread switching must be deferred
   until TZ is restored.  */
char **oldenv = NULL;
SCM zone_spec = SCM_SIMPLE_VECTOR_REF (stime, 10);
int have_zone = 0;

if (scm_is_true (zone_spec) && scm_c_string_length (zone_spec) > 0)
  {
/* it's not required that the TZ setting be correct, just that
   it has the right name.  so try something like TZ=EST0.
   using only TZ=EST would be simpler but it doesn't work on
   some OSs, e.g., Solaris.  */
SCM zone =
  scm_string_append (scm_list_2 (zone_spec,
 scm_from_locale_string ("0")));

have_zone = 1;
SCM_CRITICAL_SECTION_START;
oldenv = setzone (zone, SCM_ARG2, FUNC_NAME);
  }
#endif

You said that appending the 0 confuses MinGW, so you skip this section
and the corresponding one after the nstrftime.  But you're skipping the
setzone too, so I don't see how the strftime tests would work.

According to POSIX, our TZ value is valid:

  http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html

What is the MinGW restriction?  You mention it in the comment but I
don't understand the comment.

Andy

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-23 Thread Eli Zaretskii 
> From: Andy Wingo 
> Cc: guile-devel@gnu.org
> Date: Sat, 23 Jul 2016 14:15:06 +0200
> 
> > * libguile/stime.c (scm_strftime) [__MINGW32__]: Don't use the
> > trick of appending "0" to the time-zone string, Windows runtime
> > doesn't support that.
> > +#ifndef __MINGW32__
> > +/* Don't do this for MinGW: it only supports fixed-format
> > +   TTTnnnDDD TZ specifications, and gets confused if a zero is
> > +   appended.  */
> 
> This patch disables the setzone() call entirely; seems to be the wrong
> thing, given that MinGW doesn't appear to have struct tm->tm_zone.  What
> if we just disable appending the 0 to the time zone?

I'm not sure I understand what you have in mind, but if you show a
patch and a simple test, I can run it here.

Thanks.

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-23 Thread Andy Wingo 
On Sat 16 Jul 2016 13:24, Eli Zaretskii  writes:

> From f55f1e8de40b38cc745a930bf5a374c73d3c67ce Mon Sep 17 00:00:00 2001
> From: Eli Zaretskii 
> Date: Sat, 16 Jul 2016 14:22:06 +0300
> Subject: [PATCH] Fix 'strftime' for MS-Windows
>
> * libguile/stime.c (scm_strftime) [__MINGW32__]: Don't use the
> trick of appending "0" to the time-zone string, Windows runtime
> doesn't support that.
> +#ifndef __MINGW32__
> +/* Don't do this for MinGW: it only supports fixed-format
> +   TTTnnnDDD TZ specifications, and gets confused if a zero is
> +   appended.  */

This patch disables the setzone() call entirely; seems to be the wrong
thing, given that MinGW doesn't appear to have struct tm->tm_zone.  What
if we just disable appending the 0 to the time zone?

Andy

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-23 Thread Andy Wingo 
On Fri 22 Jul 2016 12:21, Eli Zaretskii  writes:

> Ping!  Two out of 3 patches sent here:
>
>   https://lists.gnu.org/archive/html/guile-devel/2016-07/msg00085.html
>
> are still waiting to be accepted.

I pushed the basename/dirname patch, using dirname-lgpl from gnulib as
Ludo had suggested many moons ago.  Just the strftime one remaining.  Tx
for the ping.

Andy

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-23 Thread Andy Wingo 
On Sat 16 Jul 2016 17:37, Eli Zaretskii  writes:

>> From: Andy Wingo 
>> Cc: guile-devel@gnu.org
>> Date: Sat, 16 Jul 2016 15:39:23 +0200
>> 
>> I think the right thing here is to use the mkostemp gnulib module
>> instead and pass O_BINARY in the flags.  I have made this change in
>> git; please let me know if it causes problems for you.
>
> I'm sure it will DTRT, but it's hard for me to test such changes out
> of Git.

Understood.  As I mentioned there is a service that rolls tarballs from
git:

  https://hydra.nixos.org/job/gnu/guile-2-0/tarball

Just choose the latest one, in this case:

  https://hydra.nixos.org/build/37980471/download/4/guile-2.0.12.10-141e.tar.xz

Cheers,

Andy

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-23 Thread Andy Wingo 
On Fri 15 Jul 2016 21:04, Eli Zaretskii  writes:

> The issues with dirname and basename, for which I posted a patch here:
>
>   https://lists.gnu.org/archive/html/guile-devel/2014-07/msg00012.html
>
> were subsequently discussed, but the code was not changed, AFAICT.

I think we want to go with Gnulib here, as Ludovic mentioned in the
response to this mail.  I guess we should convert the string to UTF-8,
call gnulib's dir_name and base_name on it.  Done, will push shortly.

Andy

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-22 Thread Eli Zaretskii 
Ping!  Two out of 3 patches sent here:

  https://lists.gnu.org/archive/html/guile-devel/2016-07/msg00085.html

are still waiting to be accepted.

Thanks.

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-16 Thread Eli Zaretskii 
> From: Andy Wingo 
> Cc: guile-devel@gnu.org
> Date: Sat, 16 Jul 2016 15:39:23 +0200
> 
> I think the right thing here is to use the mkostemp gnulib module
> instead and pass O_BINARY in the flags.  I have made this change in
> git; please let me know if it causes problems for you.

I'm sure it will DTRT, but it's hard for me to test such changes out
of Git.

Thanks.

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-16 Thread Andy Wingo 
On Sat 16 Jul 2016 13:24, Eli Zaretskii  writes:

> diff --git a/libguile/filesys.c b/libguile/filesys.c
> index 48232e8..c47c2f4 100644
> --- a/libguile/filesys.c
> +++ b/libguile/filesys.c
> @@ -1472,6 +1472,14 @@ SCM_DEFINE (scm_mkstemp, "mkstemp!", 1, 0, 0,
>SCM_SYSCALL (rv = mkstemp (c_tmpl));
>if (rv == -1)
>  SCM_SYSERROR;
> +#ifdef __MINGW32__
> +  /* Files created by this function are used for *.go files, so make
> + sure they use binary I/O, or else the produced *.go files will be
> + corrupted by end-of-line conversion and ^Z "software EOF"
> + misfeature.  Gnulib's 'mkstemp' uses the default text mode to
> + open the file .  */
> +  _setmode (rv, _O_BINARY);
> +#endif

This function is used for other purposes as well.  I think the right
thing here is to use the mkostemp gnulib module instead and pass
O_BINARY in the flags.  I have made this change in git; please let me
know if it causes problems for you.

Andy

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-16 Thread Eli Zaretskii 
> From: Andy Wingo 
> Cc: guile-devel@gnu.org
> Date: Sat, 16 Jul 2016 10:33:56 +0200
> 
> > Can these please be applied?
> 
> Sure let's work on it.  Would you mind submitting these again, making
> sure they apply cleanly to stable-2.0?

3 patches against the current stable-2.0 are attached.

>From 8a1e4631fc2dddf5ca63039b4a77ae0d33d3cb90 Mon Sep 17 00:00:00 2001
From: Eli Zaretskii 
Date: Sat, 16 Jul 2016 14:17:25 +0300
Subject: [PATCH] Open temporary files in binary mode on MS-Windows

* libguile/filesys.c (scm_mkstemp) [__MINGW32__]: Make sure the
temporary file is open in binary mode.
---
 libguile/filesys.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/libguile/filesys.c b/libguile/filesys.c
index 48232e8..c47c2f4 100644
--- a/libguile/filesys.c
+++ b/libguile/filesys.c
@@ -1472,6 +1472,14 @@ SCM_DEFINE (scm_mkstemp, "mkstemp!", 1, 0, 0,
   SCM_SYSCALL (rv = mkstemp (c_tmpl));
   if (rv == -1)
 SCM_SYSERROR;
+#ifdef __MINGW32__
+  /* Files created by this function are used for *.go files, so make
+ sure they use binary I/O, or else the produced *.go files will be
+ corrupted by end-of-line conversion and ^Z "software EOF"
+ misfeature.  Gnulib's 'mkstemp' uses the default text mode to
+ open the file .  */
+  _setmode (rv, _O_BINARY);
+#endif
 
   scm_substring_move_x (scm_from_locale_string (c_tmpl),
 			SCM_INUM0, scm_string_length (tmpl),
-- 
2.9.0.windows.1

>From f598db99c0da4aefce0b393f52a4da8f0c4c055e Mon Sep 17 00:00:00 2001
From: Eli Zaretskii 
Date: Sat, 16 Jul 2016 14:20:23 +0300
Subject: [PATCH] Fix 'dirname' and 'basename' on MS-Windows

* libguile/filesys.c (is_drive_letter): New helper function.
(scm_dirname, scm_basename): Use it.  Don't assume the leading
slash is always at the beginning of the file name.  Support UNC
file names on MS-Windows.
---
 libguile/filesys.c | 87 +++---
 1 file changed, 76 insertions(+), 11 deletions(-)

diff --git a/libguile/filesys.c b/libguile/filesys.c
index c47c2f4..55df768 100644
--- a/libguile/filesys.c
+++ b/libguile/filesys.c
@@ -449,6 +449,18 @@ is_file_name_separator (SCM c)
   return 0;
 }
 
+static int
+is_drive_letter (SCM c)
+{
+#ifdef __MINGW32__
+  if (SCM_CHAR (c) >= 'a' && SCM_CHAR (c) <= 'z')
+return 1;
+  else if (SCM_CHAR (c) >= 'A' && SCM_CHAR (c) <= 'Z')
+return 1;
+#endif
+  return 0;
+}
+
 SCM_DEFINE (scm_stat, "stat", 1, 1, 0, 
 (SCM object, SCM exception_on_error),
 	"Return an object containing various information about the file\n"
@@ -1522,24 +1534,60 @@ SCM_DEFINE (scm_dirname, "dirname", 1, 0, 0,
 {
   long int i;
   unsigned long int len;
+  /* Length of prefix before the top-level slash.  Always zero on
+ Posix hosts, but may be non-zero on Windows.  */
+  long prefix_len = 0;
+  int is_unc = 0;
+  unsigned long unc_end = 0;
 
   SCM_VALIDATE_STRING (1, filename);
 
   len = scm_i_string_length (filename);
+  if (len >= 2
+  && is_drive_letter (scm_c_string_ref (filename, 0))
+  && scm_is_eq (scm_c_string_ref (filename, 1), SCM_MAKE_CHAR (':')))
+{
+  prefix_len = 1;
+  if (len > 2 && is_file_name_separator (scm_c_string_ref (filename, 2)))
+	prefix_len++;
+}
+#ifdef __MINGW32__
+  if (len > 1
+  && is_file_name_separator (scm_c_string_ref (filename, 0))
+  && is_file_name_separator (scm_c_string_ref (filename, 1)))
+{
+  is_unc = 1;
+  prefix_len = 1;
+}
+#endif
 
   i = len - 1;
 
-  while (i >= 0 && is_file_name_separator (scm_c_string_ref (filename, i)))
+  while (i >= prefix_len
+	 && is_file_name_separator (scm_c_string_ref (filename, i)))
 --i;
-  while (i >= 0 && !is_file_name_separator (scm_c_string_ref (filename, i)))
+  if (is_unc)
+unc_end = i + 1;
+  while (i >= prefix_len
+	 && !is_file_name_separator (scm_c_string_ref (filename, i)))
 --i;
-  while (i >= 0 && is_file_name_separator (scm_c_string_ref (filename, i)))
+  while (i >= prefix_len
+	 && is_file_name_separator (scm_c_string_ref (filename, i)))
 --i;
 
-  if (i < 0)
+  if (i < prefix_len)
 {
-  if (len > 0 && is_file_name_separator (scm_c_string_ref (filename, 0)))
-	return scm_c_substring (filename, 0, 1);
+  if (is_unc)
+	return scm_c_substring (filename, 0, unc_end);
+  else if (len > prefix_len
+	  && is_file_name_separator (scm_c_string_ref (filename, prefix_len)))
+	return scm_c_substring (filename, 0, prefix_len + 1);
+#ifdef __MINGW32__
+  else if (len > prefix_len
+	   && scm_is_eq (scm_c_string_ref (filename, 1),
+			 SCM_MAKE_CHAR (':')))
+	return scm_c_substring (filename, 0, prefix_len + 1);
+#endif
   else
 	return scm_dot_string;
 }
@@ -1557,6 +1605,9 @@ SCM_DEFINE (scm_basename, "basename", 1, 1, 0,
 #define FUNC_NAME s_scm_basename
 {
   int i, j, len, end;
+  /* Length of prefix before the top-level slash.  Always zero on
+ Posix hosts, but may be non-zero on Windows.  */
+  long

Re: MinGW-related patches that were reported in 2014 but not applied

2016-07-16 Thread Andy Wingo 
Hi,

On Fri 15 Jul 2016 21:04, Eli Zaretskii  writes:

> I built Guile 2.0.12 today with MinGW, and to my chagrin found that
> some of the patches I sent long ago are still not in the repository.
>
> Below please find the list of those patches.
>
> The stime.c patch in this message was not applied, although it causes
> failures in time.test (explained in the message):
>
>   https://lists.gnu.org/archive/html/guile-devel/2014-06/msg00012.html
>
> The issues with dirname and basename, for which I posted a patch here:
>
>   https://lists.gnu.org/archive/html/guile-devel/2014-07/msg00012.html
>
> were subsequently discussed, but the code was not changed, AFAICT.
>
> This patch is needed, because without it Guile will write corrupted
> *.go files:
>
>   https://lists.gnu.org/archive/html/guile-devel/2014-08/msg00052.html
>
> Can these please be applied?

Sure let's work on it.  Would you mind submitting these again, making
sure they apply cleanly to stable-2.0?

Thanks,

Andy

MinGW-related patches that were reported in 2014 but not applied

2016-07-15 Thread Eli Zaretskii 
I built Guile 2.0.12 today with MinGW, and to my chagrin found that
some of the patches I sent long ago are still not in the repository.

Below please find the list of those patches.

The stime.c patch in this message was not applied, although it causes
failures in time.test (explained in the message):

  https://lists.gnu.org/archive/html/guile-devel/2014-06/msg00012.html

The issues with dirname and basename, for which I posted a patch here:

  https://lists.gnu.org/archive/html/guile-devel/2014-07/msg00012.html

were subsequently discussed, but the code was not changed, AFAICT.

This patch is needed, because without it Guile will write corrupted
*.go files:

  https://lists.gnu.org/archive/html/guile-devel/2014-08/msg00052.html

Can these please be applied?

TIA