unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* MinGW issues in Guile 2.0.11
@ 2014-06-08 15:04 Eli Zaretskii
  2014-06-09 15:47 ` Eli Zaretskii
                   ` (4 more replies)
  0 siblings, 5 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-08 15:04 UTC (permalink / raw)
  To: guile-devel

(Disappointed by lack of responses, but not enough to refrain from
reporting more problems.)

Running the Guile test suite on MS-Windows reveals the following
issues:

1. i18n.test completely fails, because it depends on the ability to
   change the program's locale at run time.  I wish this whole test
   were skipped on Windows.  (I'm quite sure I reported this last
   year.)

2. c-api.test fails with many messages such as this one:

     'CUR' is not recognized as an internal or external command,
     operable program or batch file.

   This is because c-api.test does this:

     (define (egrep string filename)
       (zero? (system (string-append "egrep '" string "' " filename " >/dev/null"))))
     ...
		 (if (and (file-exists? file)
			  (egrep "SEEK_(SET|CUR|END)" file))

   There are two problems here: quoting that is not supported by
   Windows shells, and redirection to /dev/null.  The former is easily
   fixed portably:

     (system (string-append "egrep \"" string \"" " filename

   The latter requires either an OS-dependent string in the *.scm
   source of the test, or a variable (called, e.g., null-device) that
   will be set correctly for each platform, which could then be used
   by the test unconditionally.

3. load.test fails:

     FAIL: load.test: search-path for "foo.scm" yields "dir1/foo.scm"

   (The messages are misleading: "yields" should be "should yield".)

   The test fails because:

    . it compares file names with equal?, which fails when Windows
      file names with mixed forward and backslashes are used, and/or
      when the files differ but for letter case

    . the expected result uses a relative file name of temp-dir, while
      search-path returns absolute file names

   Fixed by using "/" to create a file name from its parts in load.c:

--- libguile/load.c~	2014-02-28 23:01:27 +0200
+++ libguile/load.c	2014-06-08 13:27:24 +0300
@@ -452,11 +452,15 @@ scm_c_string_has_an_ext (char *str, size
   return 0;
 }
 
+#if 0
 #ifdef __MINGW32__
 #define FILE_NAME_SEPARATOR_STRING "\\"
 #else
 #define FILE_NAME_SEPARATOR_STRING "/"
 #endif
+#else
+#define FILE_NAME_SEPARATOR_STRING "/"
+#endif
 
 static int
 is_file_name_separator (SCM c)

   I don't see any reasons to use the backslashes when constructing
   Windows file names.  Unless someone can tell why using backslashes
   is a good idea, I propose to remove the Windows setting of
   FILE_NAME_SEPARATOR_STRING.

4. While looking into this problem, I found and removed the MinGW
   specific code in canonical_suffix:

@@ -877,23 +881,6 @@ canonical_suffix (SCM fname)
 
   /* CANON should be absolute.  */
   canon = scm_canonicalize_path (fname);
-  
-#ifdef __MINGW32__
-  {
-    size_t len = scm_c_string_length (canon);
-
-    /* On Windows, an absolute file name that doesn't start with a
-       separator starts with a drive component.  Transform the drive
-       component to a file name element: c:\foo -> \c\foo. */
-    if (len >= 2
-        && is_absolute_file_name (canon)
-        && !is_file_name_separator (scm_c_string_ref (canon, 0)))
-      return scm_string_append
-        (scm_list_3 (scm_from_latin1_string (FILE_NAME_SEPARATOR_STRING),
-                     scm_c_substring (canon, 0, 1),
-                     scm_c_substring (canon, 2, len)));
-  }
-#endif

   I think this is not a good idea, because the resulting \c\foo\bar
   file name is then passed to C APIs which cannot possibly find such
   files.  This file-name syntax is only supported by MSYS runtime,
   MinGW programs know nothing about this.  Suggest to remove.

5. ftw.scm invents absolute-file-name?, and does it wrong.  Suggest to
   fix:

--- module/ice-9/ftw.scm~0	2014-02-21 00:58:22 +0200
+++ module/ice-9/ftw.scm	2014-06-08 07:38:10 +0300
@@ -222,6 +222,7 @@
         (loop (cdr nodes) (string-append result "/" (car nodes))))))
 
 (define (abs? filename)
-  (char=? #\/ (string-ref filename 0)))
+  (absolute-file-name? filename))
 
 ;; `visited?-proc' returns a test procedure VISITED? which when called as
 ;; (VISITED? stat-obj) returns #f the first time a distinct file is seen,

6. 'times', 'sleep' and 'usleep' were not working, so time.test
   failed.  The fixes are in gnulib, let's hope the gnulib maintainers
   will accept them.

   Btw, the documented return value of 'sleep' and 'usleep' relies on
   the assumption that 'select' zeroes out its timeout argument when
   the waiting period expires and no data or signal arrives.  AFAIU,
   this assumption is not portable; in particular the gnulib
   implementation does not guarantee that.

7. Another reason for failed tests in time.test is the unportable code
   that sets TZ in the environment to force a timezone change.  The
   way it does that completely confuses the MinGW runtime, so I needed
   to disable that part for MinGW, which at least lets it pass all but
   1 test.  Here's the patch:

--- libguile/stime.c~0	2014-06-08 12:12:56 +0300
+++ libguile/stime.c	2014-06-08 13:03:01 +0300
@@ -682,6 +682,10 @@ SCM_DEFINE (scm_strftime, "strftime", 2,
 
   tbuf = scm_malloc (size);
   {
+#ifndef __MINGW32__
+    /* Don't do this for MinGW: it only supports fixed-format
+       TTTnnnDDD TZ specifications, and gets confused if a zero is
+       appended.  */
 #if !defined (HAVE_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
@@ -706,6 +710,7 @@ SCM_DEFINE (scm_strftime, "strftime", 2,
 	oldenv = setzone (zone, SCM_ARG2, FUNC_NAME);
       }
 #endif
+#endif
 
 #ifdef LOCALTIME_CACHE
     tzset ();
@@ -720,6 +725,7 @@ SCM_DEFINE (scm_strftime, "strftime", 2,
 	tbuf = scm_malloc (size);
       }
 
+#ifndef __MINGW32__
 #if !defined (HAVE_TM_ZONE)
     if (have_zone)
       {
@@ -727,6 +733,7 @@ SCM_DEFINE (scm_strftime, "strftime", 2,
 	SCM_CRITICAL_SECTION_END;
       }
 #endif
+#endif
     }
 
   result = scm_from_utf8_string (tbuf + 1);





^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW issues in Guile 2.0.11
  2014-06-08 15:04 MinGW issues in Guile 2.0.11 Eli Zaretskii
@ 2014-06-09 15:47 ` Eli Zaretskii
  2014-06-09 18:01 ` Mark H Weaver
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-09 15:47 UTC (permalink / raw)
  To: guile-devel

> Date: Sun, 08 Jun 2014 18:04:07 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> 4. While looking into this problem, I found and removed the MinGW
>    specific code in canonical_suffix:

That was a mistake, as I now realize: that code is required for
keeping auto-compiled .scm files in the cache, since Windows doesn't
allow colons in file names.  So I retract that part of the patches I
proposed.

A few more issues:

1. version.test always results in UNRESOLVED, because it erroneously
   tests a numeric result with pass-if.  Here's a trivial patch to fix
   that:

--- test-suite/tests/version.test~0	2012-01-31 01:32:38 +0200
+++ test-suite/tests/version.test	2014-06-09 18:05:35 +0300
@@ -24,7 +24,7 @@
 	 (and (string? (major-version))
 	      (string? (minor-version))
 	      (string? (micro-version))
-	      (string-contains (version)
-                               (string-append (major-version) "."
-                                              (minor-version) "."
-                                              (micro-version)))))
+	      (number? (string-contains (version)
+					(string-append (major-version) "."
+						       (minor-version) "."
+						       (micro-version))))))



2. Building GDB with Guile reveals a problem in the iselect.h header
   file installed by Guile: it includes <sys/select.h>, which doesn't
   exist on MS-Windows.  When Guile is built, that header is provided
   by gnulib, but it isn't (and cannot be) installed.  I propose the
   following to fix the problem:

--- libguile/iselect.h~0	2014-02-15 01:00:33 +0200
+++ libguile/iselect.h	2014-06-09 18:26:52 +0300
@@ -28,7 +28,16 @@
 /* Needed for FD_SET on some systems.  */
 #include <sys/types.h>
 
-#include <sys/select.h>
+#if BUILDING_LIBGUILE
+# include <sys/select.h>
+#else  /* !BUILDING_LIBGUILE */
+# if SCM_HAVE_SYS_SELECT_H
+#  include <sys/select.h>
+# endif
+# if SCM_HAVE_WINSOCK2_H
+#  include <winsock2.h>
+# endif
+#endif	/* !BUILDING_LIBGUILE */
 
 SCM_API int scm_std_select (int fds,
 			    fd_set *rfds,

3. The nice new feature of auto-loading Guile-specific GDB support
   script when libguile is loaded seems to be useless if the program
   was statically linked with libguile, because the library as such is
   never read by GDB in that case, and so the auto-load hooks never
   trigger.  So the script should be renamed or copied to match the
   program name, or loaded manually.  I think the Guile manual should
   mention that.  In addition, I think the gdbinit file which comes
   with Guile should also be mentioned in the manual, as the commands
   it defines are very useful when debugging Guile on the C level.

4. The libguile built on Windows includes the libcharset.o module
   (taken from gnulib), which is also part of libintl.  This causes
   link failure when statically linking a program (in my case, GDB)
   with Guile, if that program mentions libintl.a before libguile on
   the link command line, because the symbol locale_charset is defined
   twice.  This is a nasty gotcha, and I wish it could be solved,
   because working around it is not easy.  For example, the Guile
   built configury could notice that libintl will be linked in, and
   refrain from including that module from gnulib, even if the
   underlying platform needs some of its code (I think in this case it
   was nl_langinfo).  Alternatively, locale_charset could be declared
   'static', assuming that Guile doesn't call it (gnulib already has a
   provision for doing that).

5. Finally, the auto-compilation message displayed by the library when
   the application loads some .scm file for the first time could
   confuse users.  Here's an example from GDB:

     ;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
     ;;;       or pass the --no-auto-compile argument to disable.
     ;;; compiling d:\usr\share\gdb/guile/gdb/boot.scm
     ;;; compiling d:\usr\share\gdb/guile/gdb\init.scm
     ;;; compiled D:\usr\eli/.cache/guile/ccache/2.0-LE-4-2.0/d/usr/share/gdb/guile/gdb/init.scm.go
     ;;; compiled D:\usr\eli/.cache/guile/ccache/2.0-LE-4-2.0/d/usr/share/gdb/guile/gdb/boot.scm.go

   This talks about a --no-auto-compile switch, which obviously
   doesn't exist except in guile the standalone program.  I wonder
   whether a library should suppress these messages by default.

Thanks.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW issues in Guile 2.0.11
  2014-06-08 15:04 MinGW issues in Guile 2.0.11 Eli Zaretskii
  2014-06-09 15:47 ` Eli Zaretskii
@ 2014-06-09 18:01 ` Mark H Weaver
  2014-06-09 18:25   ` Eli Zaretskii
  2014-06-09 19:30 ` MinGW vs. setlocale Ludovic Courtès
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 67+ messages in thread
From: Mark H Weaver @ 2014-06-09 18:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:
> (Disappointed by lack of responses, but not enough to refrain from
> reporting more problems.)

I'm currently in the middle of a housing move, so it might be a week or
two before I can respond properly to your messages, but they are
nonetheless appreciated.  If no one beats me to it, I'll get to them
as soon as I can.

    Thanks,
      Mark



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW issues in Guile 2.0.11
  2014-06-09 18:01 ` Mark H Weaver
@ 2014-06-09 18:25   ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-09 18:25 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guile-devel

> From: Mark H Weaver <mhw@netris.org>
> Cc: guile-devel@gnu.org
> Date: Mon, 09 Jun 2014 14:01:28 -0400
> 
> Hi Eli,
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > (Disappointed by lack of responses, but not enough to refrain from
> > reporting more problems.)
> 
> I'm currently in the middle of a housing move, so it might be a week or
> two before I can respond properly to your messages, but they are
> nonetheless appreciated.  If no one beats me to it, I'll get to them
> as soon as I can.

No rush, and thanks for responding.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* MinGW vs. setlocale
  2014-06-08 15:04 MinGW issues in Guile 2.0.11 Eli Zaretskii
  2014-06-09 15:47 ` Eli Zaretskii
  2014-06-09 18:01 ` Mark H Weaver
@ 2014-06-09 19:30 ` Ludovic Courtès
  2014-06-10 16:17   ` Eli Zaretskii
  2014-06-09 19:32 ` MinGW vs. c-api.test Ludovic Courtès
  2014-06-09 19:42 ` Windows file name separators Ludovic Courtès
  4 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-09 19:30 UTC (permalink / raw)
  To: guile-devel

Hello!

Eli Zaretskii <eliz@gnu.org> skribis:

> (Disappointed by lack of responses, but not enough to refrain from
> reporting more problems.)

Heh.  :-/  Personally, I prefer short messages focused on a single issue:
that’s easier to act on, and less intimidating.

At any rate, thank you for persevering!

> 1. i18n.test completely fails, because it depends on the ability to
>    change the program's locale at run time.  I wish this whole test
>    were skipped on Windows.  (I'm quite sure I reported this last
>    year.)

What does ‘setlocale’ return when called more than once on Windows?  Is
there an exception thrown or something that would allow i18n.test to
determine that tests should be skipped?

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* MinGW vs. c-api.test
  2014-06-08 15:04 MinGW issues in Guile 2.0.11 Eli Zaretskii
                   ` (2 preceding siblings ...)
  2014-06-09 19:30 ` MinGW vs. setlocale Ludovic Courtès
@ 2014-06-09 19:32 ` Ludovic Courtès
  2014-06-10  9:05   ` Neil Jerram
  2014-06-10 11:44   ` Ludovic Courtès
  2014-06-09 19:42 ` Windows file name separators Ludovic Courtès
  4 siblings, 2 replies; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-09 19:32 UTC (permalink / raw)
  To: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

> 2. c-api.test fails with many messages such as this one:
>
>      'CUR' is not recognized as an internal or external command,
>      operable program or batch file.
>
>    This is because c-api.test does this:
>
>      (define (egrep string filename)
>        (zero? (system (string-append "egrep '" string "' " filename " >/dev/null"))))
>      ...
> 		 (if (and (file-exists? file)
> 			  (egrep "SEEK_(SET|CUR|END)" file))
>
>    There are two problems here: quoting that is not supported by
>    Windows shells, and redirection to /dev/null.  The former is easily
>    fixed portably:
>
>      (system (string-append "egrep \"" string \"" " filename
>
>    The latter requires either an OS-dependent string in the *.scm
>    source of the test, or a variable (called, e.g., null-device) that
>    will be set correctly for each platform, which could then be used
>    by the test unconditionally.

What’s the name of /dev/null on Windows?

Ludo’.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Windows file name separators
  2014-06-08 15:04 MinGW issues in Guile 2.0.11 Eli Zaretskii
                   ` (3 preceding siblings ...)
  2014-06-09 19:32 ` MinGW vs. c-api.test Ludovic Courtès
@ 2014-06-09 19:42 ` Ludovic Courtès
  2014-06-10 16:00   ` Eli Zaretskii
  4 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-09 19:42 UTC (permalink / raw)
  To: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

> 3. load.test fails:
>
>      FAIL: load.test: search-path for "foo.scm" yields "dir1/foo.scm"
>
>    (The messages are misleading: "yields" should be "should yield".)
>
>    The test fails because:
>
>     . it compares file names with equal?, which fails when Windows
>       file names with mixed forward and backslashes are used, and/or
>       when the files differ but for letter case
>
>     . the expected result uses a relative file name of temp-dir, while
>       search-path returns absolute file names
>
>    Fixed by using "/" to create a file name from its parts in load.c:
>
> --- libguile/load.c~	2014-02-28 23:01:27 +0200
> +++ libguile/load.c	2014-06-08 13:27:24 +0300
> @@ -452,11 +452,15 @@ scm_c_string_has_an_ext (char *str, size
>    return 0;
>  }
>  
> +#if 0
>  #ifdef __MINGW32__
>  #define FILE_NAME_SEPARATOR_STRING "\\"
>  #else
>  #define FILE_NAME_SEPARATOR_STRING "/"
>  #endif
> +#else
> +#define FILE_NAME_SEPARATOR_STRING "/"
> +#endif
>  
>  static int
>  is_file_name_separator (SCM c)
>
>    I don't see any reasons to use the backslashes when constructing
>    Windows file names.  Unless someone can tell why using backslashes
>    is a good idea, I propose to remove the Windows setting of
>    FILE_NAME_SEPARATOR_STRING.

I’m confused: this was added in 4bab7f01 precisely to support MinGW or
Windows.  Similarly, boot-9.scm has ‘file-name-separator-string’ and
related stuff.  This was the result of the discussion at
<http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10474#89>.

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-09 19:32 ` MinGW vs. c-api.test Ludovic Courtès
@ 2014-06-10  9:05   ` Neil Jerram
  2014-06-10 11:42     ` Ludovic Courtès
  2014-06-10 15:32     ` Eli Zaretskii
  2014-06-10 11:44   ` Ludovic Courtès
  1 sibling, 2 replies; 67+ messages in thread
From: Neil Jerram @ 2014-06-10  9:05 UTC (permalink / raw)
  To: guile-devel

On 2014-06-09 20:32, ludo@gnu.org wrote:
> What’s the name of /dev/null on Windows?

NUL

For example:

   C:\Users\nj>echo hello >NUL

   C:\Users\nj>

Regards,
      Neil




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-10  9:05   ` Neil Jerram
@ 2014-06-10 11:42     ` Ludovic Courtès
  2014-06-10 15:32     ` Eli Zaretskii
  1 sibling, 0 replies; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-10 11:42 UTC (permalink / raw)
  To: guile-devel

Neil Jerram <neil@ossau.homelinux.net> skribis:

> On 2014-06-09 20:32, ludo@gnu.org wrote:
>> What’s the name of /dev/null on Windows?
>
> NUL
>
> For example:
>
>   C:\Users\nj>echo hello >NUL

OK, thanks!

Ludo’.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-09 19:32 ` MinGW vs. c-api.test Ludovic Courtès
  2014-06-10  9:05   ` Neil Jerram
@ 2014-06-10 11:44   ` Ludovic Courtès
  2014-06-10 15:39     ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-10 11:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

ludo@gnu.org (Ludovic Courtès) skribis:

> Eli Zaretskii <eliz@gnu.org> skribis:
>
>> 2. c-api.test fails with many messages such as this one:
>>
>>      'CUR' is not recognized as an internal or external command,
>>      operable program or batch file.
>>
>>    This is because c-api.test does this:
>>
>>      (define (egrep string filename)
>>        (zero? (system (string-append "egrep '" string "' " filename " >/dev/null"))))
>>      ...
>> 		 (if (and (file-exists? file)
>> 			  (egrep "SEEK_(SET|CUR|END)" file))
>>
>>    There are two problems here: quoting that is not supported by
>>    Windows shells, and redirection to /dev/null.  The former is easily
>>    fixed portably:
>>
>>      (system (string-append "egrep \"" string \"" " filename
>>
>>    The latter requires either an OS-dependent string in the *.scm
>>    source of the test, or a variable (called, e.g., null-device) that
>>    will be set correctly for each platform, which could then be used
>>    by the test unconditionally.

Eli, I noticed there are many other occurrences of /dev/null in the test
suite.  Do they all need to be patched to use NUL, or is /dev/null
somehow interpreted correctly in some contexts?

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-10  9:05   ` Neil Jerram
  2014-06-10 11:42     ` Ludovic Courtès
@ 2014-06-10 15:32     ` Eli Zaretskii
  2014-06-10 15:56       ` David Kastrup
  1 sibling, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-10 15:32 UTC (permalink / raw)
  To: Neil Jerram; +Cc: guile-devel

> Date: Tue, 10 Jun 2014 10:05:52 +0100
> From: Neil Jerram <neil@ossau.homelinux.net>
> 
> On 2014-06-09 20:32, ludo@gnu.org wrote:
> > What’s the name of /dev/null on Windows?
> 
> NUL

Yes, "nul" case-insensitively.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-10 11:44   ` Ludovic Courtès
@ 2014-06-10 15:39     ` Eli Zaretskii
  2014-06-11 12:37       ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-10 15:39 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel <guile-devel@gnu.org>
> Date: Tue, 10 Jun 2014 13:44:17 +0200
> 
> Eli, I noticed there are many other occurrences of /dev/null in the test
> suite.  Do they all need to be patched to use NUL, or is /dev/null
> somehow interpreted correctly in some contexts?

When we send /dev/null to some Guile primitive, which calls open or
fopen internally, gnulib replaces that by nul, so it transparently
works.  So only strings that are passed verbatim to the system shell
need to be fixed.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-10 15:32     ` Eli Zaretskii
@ 2014-06-10 15:56       ` David Kastrup
  2014-06-10 18:00         ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: David Kastrup @ 2014-06-10 15:56 UTC (permalink / raw)
  To: guile-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Tue, 10 Jun 2014 10:05:52 +0100
>> From: Neil Jerram <neil@ossau.homelinux.net>
>> 
>> On 2014-06-09 20:32, ludo@gnu.org wrote:
>> > What’s the name of /dev/null on Windows?
>> 
>> NUL
>
> Yes, "nul" case-insensitively.

If I remember correctly, even something like C:\tmp\nul.txt would serve
as null device, though I cannot vouch for this remaining true with
NT-based Windows systems.  It was the case for those versions running on
top of MSDOS I'm pretty sure.

-- 
David Kastrup




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-06-09 19:42 ` Windows file name separators Ludovic Courtès
@ 2014-06-10 16:00   ` Eli Zaretskii
  2014-06-30 11:15     ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-10 16:00 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Date: Mon, 09 Jun 2014 21:42:36 +0200
> 
> Eli Zaretskii <eliz@gnu.org> skribis:
> 
> > 3. load.test fails:
> >
> >      FAIL: load.test: search-path for "foo.scm" yields "dir1/foo.scm"
> >
> >    (The messages are misleading: "yields" should be "should yield".)
> >
> >    The test fails because:
> >
> >     . it compares file names with equal?, which fails when Windows
> >       file names with mixed forward and backslashes are used, and/or
> >       when the files differ but for letter case
> >
> >     . the expected result uses a relative file name of temp-dir, while
> >       search-path returns absolute file names
> >
> >    Fixed by using "/" to create a file name from its parts in load.c:
> >
> > --- libguile/load.c~	2014-02-28 23:01:27 +0200
> > +++ libguile/load.c	2014-06-08 13:27:24 +0300
> > @@ -452,11 +452,15 @@ scm_c_string_has_an_ext (char *str, size
> >    return 0;
> >  }
> >  
> > +#if 0
> >  #ifdef __MINGW32__
> >  #define FILE_NAME_SEPARATOR_STRING "\\"
> >  #else
> >  #define FILE_NAME_SEPARATOR_STRING "/"
> >  #endif
> > +#else
> > +#define FILE_NAME_SEPARATOR_STRING "/"
> > +#endif
> >  
> >  static int
> >  is_file_name_separator (SCM c)
> >
> >    I don't see any reasons to use the backslashes when constructing
> >    Windows file names.  Unless someone can tell why using backslashes
> >    is a good idea, I propose to remove the Windows setting of
> >    FILE_NAME_SEPARATOR_STRING.
> 
> I’m confused: this was added in 4bab7f01 precisely to support MinGW or
> Windows.  Similarly, boot-9.scm has ‘file-name-separator-string’ and
> related stuff.  This was the result of the discussion at
> <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10474#89>.

Sorry, that's my fault: I didn't explain the problem in enough detail.

There's nothing wrong with the 4bab7f01 commit per se (and you will
see that its only part that I changed is the definition of
FILE_NAME_SEPARATOR_STRING for MinGW).  The problem is not in that
commit, it is elsewhere: in Scheme code, in this case in the test
suite, that compares file names as simple strings.  Such comparisons
fail if the file names differ by the style of directory separators:
one uses forward slashes, the other backslashes, or some mix thereof.

Now, FILE_NAME_SEPARATOR_STRING is used only for constructing file
names from their parts.  It is not used for testing a particular
file-name character for being a directory separator.  Therefore, we
can discard the separate definition of FILE_NAME_SEPARATOR_STRING for
Windows, and use "/" on all platforms.  This makes the problem of
comparing file names easier, and in particular lets Guile pass
load.test.  But it doesn't solve the problem entirely.

To solve this problem completely, we need a function that
canonicalizes a file name wrt directory separators -- converts all
backslashes to forward slashes.  Does Guile have such a function?  If
it does, then comparing the canonicalized file names will work
reliably on Windows.

I hope I made myself clear this time.

Thanks.

P.S. Please CC me on your responses, as I'm not subscribed to the
list, so I need to download the list archives in order to reply.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-09 19:30 ` MinGW vs. setlocale Ludovic Courtès
@ 2014-06-10 16:17   ` Eli Zaretskii
  2014-06-11 13:13     ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-10 16:17 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Date: Mon, 09 Jun 2014 21:30:46 +0200
> 
> > 1. i18n.test completely fails, because it depends on the ability to
> >    change the program's locale at run time.  I wish this whole test
> >    were skipped on Windows.  (I'm quite sure I reported this last
> >    year.)
> 
> What does ‘setlocale’ return when called more than once on Windows?  Is
> there an exception thrown or something that would allow i18n.test to
> determine that tests should be skipped?

A very good question, thank you for asking it.  The short answer is
that yes, it threw an exception.  The long answer is that while
looking into the reasons of these exceptions, I found a few snafus,
and succeeded to fix some, so that most of the i18n test now works on
Windows.

Here are the details.

First, make-locale threw an exception, because it tried to call
'setlocale' with LC_MESSAGES, which the Windows runtime doesn't
support.  locale-categories.h tried to avoid that by conditioning that
call by LC_MESSAGES being defined, but the oh-so-helpful libintl.h
header file just happens to define it to some arbitrary large
constant.  So the ifdef didn't work, and setlocale barfed.  Here's the
suggested solution:

--- libguile/locale-categories.h~0	2010-12-14 21:15:17 +0200
+++ libguile/locale-categories.h	2014-06-10 18:54:06 +0300
@@ -23,8 +23,10 @@
 SCM_DEFINE_LOCALE_CATEGORY (COLLATE)
 SCM_DEFINE_LOCALE_CATEGORY (CTYPE)
 
-#ifdef LC_MESSAGES
-/* MinGW doesn't have `LC_MESSAGES'.  */
+#if defined(LC_MESSAGES) && !(defined(LC_MAX) && LC_MESSAGES > LC_MAX)
+/* MinGW doesn't have `LC_MESSAGES'.  libintl.h might define
+   `LC_MESSAGES' for MinGW to an arbitrary large value which we cannot
+   use in a call to `setlocale'.  */
 SCM_DEFINE_LOCALE_CATEGORY (MESSAGES)
 #endif
 

The next problem is that i18n.test uses Posix locale strings, whereas
the Windows runtime names the same locales by different names.
Moreover, Windows 'setlocale' doesn't support UTF-8 encoding (even
though a Windows UTF-8 codepage exists).  So every test for a locale
other than "C" was failing, because setlocale failed.  I replaced
Posix locales with similar Windows ones; see the following patch, in
which I also emoved all but one LC_MESSAGES, because these always fail
on Windows:

--- test-suite/tests/i18n.test~0	2014-01-22 00:20:53 +0200
+++ test-suite/tests/i18n.test	2014-06-10 11:24:15 +0300
@@ -40,16 +40,19 @@
   (pass-if "make-locale (2 args, list)"
     (not (not (make-locale (list LC_COLLATE LC_MESSAGES) "C"))))
 
+  (pass-if "make-locale (2 args, list)"
+    (not (not (make-locale (list LC_COLLATE LC_NUMERIC) "C"))))
+
   (pass-if "make-locale (3 args)"
     (not (not (make-locale (list LC_COLLATE) "C"
-                           (make-locale (list LC_MESSAGES) "C")))))
+                           (make-locale (list LC_NUMERIC) "C")))))
 
   (pass-if-exception "make-locale with unknown locale" exception:locale-error
     (make-locale LC_ALL "does-not-exist"))
 
   (pass-if "locale?"
     (and (locale? (make-locale (list LC_ALL) "C"))
-         (locale? (make-locale (list LC_MESSAGES LC_NUMERIC) "C"
+         (locale? (make-locale (list LC_TIME LC_NUMERIC) "C"
                                (make-locale (list LC_CTYPE) "C")))))
 
   (pass-if "%global-locale"
@@ -82,19 +85,29 @@
 
 \f
 (define %french-locale-name
-  "fr_FR.ISO-8859-1")
+  (if (string-contains %host-type "-mingw32")
+      "fra_FRA.850"
+      "fr_FR.ISO-8859-1"))
 
 (define %french-utf8-locale-name
-  "fr_FR.UTF-8")
+  (if (string-contains %host-type "-mingw32")
+      "fra_FRA.1252"
+      "fr_FR.UTF-8"))
 
 (define %turkish-utf8-locale-name
-  "tr_TR.UTF-8")
+  (if (string-contains %host-type "-mingw32")
+      "tur_TRK.1254"
+      "tr_TR.UTF-8"))
 
 (define %german-utf8-locale-name
-  "de_DE.UTF-8")
+  (if (string-contains %host-type "-mingw32")
+      "deu_DEU.1252"
+      "de_DE.UTF-8"))
 
 (define %greek-utf8-locale-name
-  "el_GR.UTF-8")
+  (if (string-contains %host-type "-mingw32")
+      "grc_ELL.1253"
+      "el_GR.UTF-8"))
 
 (define %american-english-locale-name
   "en_US")
@@ -148,13 +161,14 @@
   (under-locale-or-unresolved %french-utf8-locale thunk))
 
 (define (under-turkish-utf8-locale-or-unresolved thunk)
-  ;; FreeBSD 8.2 and 9.1, Solaris 2.10, and Darwin 8.11.0 have a broken
-  ;; tr_TR locale where `i' is mapped to uppercase `I' instead of `Ä°',
-  ;; so disable tests on that platform.
+  ;; FreeBSD 8.2 and 9.1, Solaris 2.10, Darwin 8.11.0, and MinGW have
+  ;; a broken tr_TR locale where `i' is mapped to uppercase `I'
+  ;; instead of `İ', so disable tests on that platform.
   (if (or (string-contains %host-type "freebsd8")
           (string-contains %host-type "freebsd9")
           (string-contains %host-type "solaris2.10")
-          (string-contains %host-type "darwin8"))
+          (string-contains %host-type "darwin8")
+	  (string-contains %host-type "-mingw32"))
       (throw 'unresolved)
       (under-locale-or-unresolved %turkish-utf8-locale thunk)))
 
@@ -192,7 +206,10 @@
         ;; strings.
         (dynamic-wind
           (lambda ()
-            (setlocale LC_ALL "fr_FR.UTF-8"))
+	    (setlocale LC_ALL
+		       (if (string-contains %host-type "-mingw32")
+			   "fra_FRA.1252"
+			   "fr_FR.UTF-8")))
	   (lambda ()
	     (string-locale-ci=? "œuf" "ŒUF"))
	   (lambda ()
	     (setlocale LC_ALL "C"))))))

After all these changes, some tests still fail or throw exceptions:

  UNRESOLVED: i18n.test: text collation (French): string-locale-ci=?
  UNRESOLVED: i18n.test: text collation (French): string-locale-ci=? (2 args, wide strings)
  UNRESOLVED: i18n.test: text collation (French): string-locale-ci=? (3 args, wide strings)
  UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>?
  UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>? (wide strings)
  UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>? (wide and narrow strings)
  UNRESOLVED: i18n.test: text collation (French): char-locale-ci<>?
  UNRESOLVED: i18n.test: text collation (French): char-locale-ci<>? (wide)
  UNRESOLVED: i18n.test: text collation (Greek): string-locale-ci=?
  UNRESOLVED: i18n.test: character mapping: char-locale-upcase Turkish
  UNRESOLVED: i18n.test: character mapping: char-locale-downcase Turkish
  UNRESOLVED: i18n.test: string mapping: string-locale-upcase Greek
  UNRESOLVED: i18n.test: string mapping: string-locale-upcase Greek (two sigmas)
  UNRESOLVED: i18n.test: string mapping: string-locale-downcase Greek
  UNRESOLVED: i18n.test: string mapping: string-locale-downcase Greek (two sigmas)
  UNRESOLVED: i18n.test: string mapping: string-locale-upcase Turkish
  UNRESOLVED: i18n.test: string mapping: string-locale-downcase Turkish

I don't know why these fail.  Is it possible that the underlying
functions assume that the string arguments are encoded according to
the locale's codeset?  If so, since the source file is encoded in
UTF-8, that won't work on Windows, and the strings need to be recoded
before they are passed to libunistring functions.  Any ideas for
debugging this are welcome.

  FAIL: i18n.test: nl-langinfo et al.: locale-day (French)
  FAIL: i18n.test: nl-langinfo et al.: locale-day (French, using `%global-locale')

This is because gnulib's nl_langinfo only supports C locale for the
day names.  I'm taking this up with gnulib maintainers.

  FAIL: i18n.test: number->locale-string: French: integer
  FAIL: i18n.test: number->locale-string: French: fraction
  FAIL: i18n.test: number->locale-string: French: fraction, 1 digit
  FAIL: i18n.test: monetary-amount->locale-string: French: integer
  FAIL: i18n.test: monetary-amount->locale-string: French: fraction

There's no blank after the 7th digit, where the test expects it.  Not
sure what kind of problem is that, perhaps again due to gnulib's
nl_langinfo.

  UNRESOLVED: i18n.test: format ~h: French: 12345.5678
  UNRESOLVED: i18n.test: format ~h: English: 12345.5678

~h is not supported on Windows.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-10 15:56       ` David Kastrup
@ 2014-06-10 18:00         ` Eli Zaretskii
  2014-06-11  0:36           ` dsmich
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-10 18:00 UTC (permalink / raw)
  To: David Kastrup; +Cc: guile-devel

> From: David Kastrup <dak@gnu.org>
> Date: Tue, 10 Jun 2014 17:56:46 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Date: Tue, 10 Jun 2014 10:05:52 +0100
> >> From: Neil Jerram <neil@ossau.homelinux.net>
> >> 
> >> On 2014-06-09 20:32, ludo@gnu.org wrote:
> >> > What’s the name of /dev/null on Windows?
> >> 
> >> NUL
> >
> > Yes, "nul" case-insensitively.
> 
> If I remember correctly, even something like C:\tmp\nul.txt would serve
> as null device

Yes, any file name whose basename is nul.WHATEVER is also a null
device.  "nul" is just the simplest form of all possible ones.

> though I cannot vouch for this remaining true with NT-based Windows
> systems.  It was the case for those versions running on top of MSDOS
> I'm pretty sure.

It still works on modern Windows systems as well.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-10 18:00         ` Eli Zaretskii
@ 2014-06-11  0:36           ` dsmich
  2014-06-11  2:52             ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: dsmich @ 2014-06-11  0:36 UTC (permalink / raw)
  To: David Kastrup, Eli Zaretskii; +Cc: guile-devel


---- Eli Zaretskii <eliz@gnu.org> wrote: 
> > From: David Kastrup <dak@gnu.org>
> > Date: Tue, 10 Jun 2014 17:56:46 +0200
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > >> Date: Tue, 10 Jun 2014 10:05:52 +0100
> > >> From: Neil Jerram <neil@ossau.homelinux.net>
> > >> 
> > >> On 2014-06-09 20:32, ludo@gnu.org wrote:
> > >> > What’s the name of /dev/null on Windows?
> > >> 
> > >> NUL
> > >
> > > Yes, "nul" case-insensitively.
> > 
> > If I remember correctly, even something like C:\tmp\nul.txt would serve
> > as null device
> 
> Yes, any file name whose basename is nul.WHATEVER is also a null
> device.  "nul" is just the simplest form of all possible ones.
> 
> > though I cannot vouch for this remaining true with NT-based Windows
> > systems.  It was the case for those versions running on top of MSDOS
> > I'm pretty sure.
> 
> It still works on modern Windows systems as well.

This was an old msdos batch file trick.  There was no way to directly tell if a directory existed,
But if c:\some\dir\nul "existed" then c:\some\dir did.

Ugh.  Glad those days are long past...

-Dale





^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-11  0:36           ` dsmich
@ 2014-06-11  2:52             ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-11  2:52 UTC (permalink / raw)
  To: dsmich; +Cc: dak, guile-devel

> Date: Tue, 10 Jun 2014 20:36:22 -0400
> From:  <dsmich@roadrunner.com>
> Cc: guile-devel@gnu.org
> 
> > It still works on modern Windows systems as well.
> 
> This was an old msdos batch file trick.  There was no way to directly tell if a directory existed,
> But if c:\some\dir\nul "existed" then c:\some\dir did.

Now, that trick no longer works on Windows.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-10 15:39     ` Eli Zaretskii
@ 2014-06-11 12:37       ` Ludovic Courtès
  2014-06-11 15:08         ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-11 12:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel <guile-devel@gnu.org>
>> Date: Tue, 10 Jun 2014 13:44:17 +0200
>> 
>> Eli, I noticed there are many other occurrences of /dev/null in the test
>> suite.  Do they all need to be patched to use NUL, or is /dev/null
>> somehow interpreted correctly in some contexts?
>
> When we send /dev/null to some Guile primitive, which calls open or
> fopen internally, gnulib replaces that by nul, so it transparently
> works.  So only strings that are passed verbatim to the system shell
> need to be fixed.

OK, thanks for the explanation.

Below is the fix I just pushed.

Ludo’.

commit 82b8cfa40cbaea1ef2b8053af574c6d84f2705bc (HEAD, refs/heads/stable-2.0)
Author: Ludovic Courtès <ludo@gnu.org>
Date:   Wed Jun 11 14:35:26 2014 +0200

    tests: Use NUL instead of /dev/null on MinGW.
    
    Reported by Eli Zaretskii <eliz@gnu.org>.
    
    * test-suite/test-suite/lib.scm (%null-device): New variable.
    * test-suite/tests/c-api.test (egrep): Use %NULL-DEVICE instead of
      /dev/null.
    * test-suite/tests/popen.test ("open-input-pipe")["no duplicate"]:
      Likewise.

	Modified   test-suite/test-suite/lib.scm
diff --git a/test-suite/test-suite/lib.scm b/test-suite/test-suite/lib.scm
index e25df78..5628ae0 100644
--- a/test-suite/test-suite/lib.scm
+++ b/test-suite/test-suite/lib.scm
@@ -1,6 +1,6 @@
 ;;;; test-suite/lib.scm --- generic support for testing
 ;;;; Copyright (C) 1999, 2000, 2001, 2004, 2006, 2007, 2009, 2010,
-;;;;   2011, 2012, 2013 Free Software Foundation, Inc.
+;;;;   2011, 2012, 2013, 2014 Free Software Foundation, Inc.
 ;;;;
 ;;;; This program is free software; you can redistribute it and/or
 ;;;; modify it under the terms of the GNU Lesser General Public
@@ -63,6 +63,9 @@
  ;; Using a given locale
  with-locale with-locale* with-latin1-locale with-latin1-locale*
 
+ ;; The bit bucket.
+ %null-device
+
  ;; Reporting results in various ways.
  register-reporter unregister-reporter reporter-registered?
  make-count-reporter print-counts
@@ -571,6 +574,14 @@
     ((_ body ...)
      (with-latin1-locale* (lambda () body ...)))))
 
+(define %null-device
+  ;; On Windows (MinGW), /dev/null does not exist and we must instead
+  ;; use NUL.  Note that file system procedures automatically translate
+  ;; /dev/null, so this variable is only useful for shell snippets.
+  (if (file-exists? "/dev/null")
+      "/dev/null"
+      "NUL"))
+
 \f
 ;;;; REPORTERS
 ;;;;
	Modified   test-suite/tests/c-api.test
diff --git a/test-suite/tests/c-api.test b/test-suite/tests/c-api.test
index 9a2108e..5ce033f 100644
--- a/test-suite/tests/c-api.test
+++ b/test-suite/tests/c-api.test
@@ -1,7 +1,7 @@
 ;;;; c-api.test --- complementary test suite for the c-api     -*- scheme -*-
 ;;;; MDJ 990915 <djurfeldt@nada.kth.se>
 ;;;;
-;;;;   Copyright (C) 1999, 2006, 2012 Free Software Foundation, Inc.
+;;;;   Copyright (C) 1999, 2006, 2012, 2014 Free Software Foundation, Inc.
 ;;;;
 ;;;; This library is free software; you can redistribute it and/or
 ;;;; modify it under the terms of the GNU Lesser General Public
@@ -22,7 +22,8 @@
 (define srcdir (cdr (assq 'srcdir %guile-build-info)))
 
 (define (egrep string filename)
-  (zero? (system (string-append "egrep '" string "' " filename " >/dev/null"))))
+  (zero? (system (string-append "egrep '" string "' " filename
+                                " >" %null-device))))
 
 (define (seek-offset-test dirname)
   (let ((dir (opendir dirname)))
	Modified   test-suite/tests/popen.test
diff --git a/test-suite/tests/popen.test b/test-suite/tests/popen.test
index 2818be0..27e15dc 100644
--- a/test-suite/tests/popen.test
+++ b/test-suite/tests/popen.test
@@ -1,6 +1,6 @@
 ;;;; popen.test --- exercise ice-9/popen.scm      -*- scheme -*-
 ;;;;
-;;;; Copyright 2003, 2006, 2010, 2011, 2013 Free Software Foundation, Inc.
+;;;; Copyright 2003, 2006, 2010, 2011, 2013, 2014 Free Software Foundation, Inc.
 ;;;;
 ;;;; This library is free software; you can redistribute it and/or
 ;;;; modify it under the terms of the GNU Lesser General Public
@@ -109,7 +109,9 @@
                       (with-input-from-port (car p2c)
                         (lambda ()
                           (open-input-pipe
-                           "exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read REPLY")))))))
+                           (format #f "exec 1>~a; echo closed 1>&2; \
+exec 2>~a; read REPLY"
+                                   %null-device %null-device))))))))
        (close-port (cdr c2p)) ;; write side
        (let ((result (eof-object? (read-char port))))
          (display "hello!\n" (cdr p2c))



^ permalink raw reply related	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-10 16:17   ` Eli Zaretskii
@ 2014-06-11 13:13     ` Ludovic Courtès
  2014-06-11 15:33       ` Eli Zaretskii
  2014-06-19  4:53       ` Doug Evans
  0 siblings, 2 replies; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-11 13:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Date: Mon, 09 Jun 2014 21:30:46 +0200
>> 
>> > 1. i18n.test completely fails, because it depends on the ability to
>> >    change the program's locale at run time.  I wish this whole test
>> >    were skipped on Windows.  (I'm quite sure I reported this last
>> >    year.)
>> 
>> What does ‘setlocale’ return when called more than once on Windows?  Is
>> there an exception thrown or something that would allow i18n.test to
>> determine that tests should be skipped?
>
> A very good question, thank you for asking it.  The short answer is
> that yes, it threw an exception.  The long answer is that while
> looking into the reasons of these exceptions, I found a few snafus,
> and succeeded to fix some, so that most of the i18n test now works on
> Windows.

Heh, thanks for investigating!

> First, make-locale threw an exception, because it tried to call
> 'setlocale' with LC_MESSAGES, which the Windows runtime doesn't
> support.  locale-categories.h tried to avoid that by conditioning that
> call by LC_MESSAGES being defined, but the oh-so-helpful libintl.h
> header file just happens to define it to some arbitrary large
> constant.  So the ifdef didn't work, and setlocale barfed.  Here's the
> suggested solution:

Pushed as c84f25b.

> The next problem is that i18n.test uses Posix locale strings, whereas
> the Windows runtime names the same locales by different names.
> Moreover, Windows 'setlocale' doesn't support UTF-8 encoding (even
> though a Windows UTF-8 codepage exists).  So every test for a locale
> other than "C" was failing, because setlocale failed.  I replaced
> Posix locales with similar Windows ones; see the following patch, in
> which I also emoved all but one LC_MESSAGES, because these always fail
> on Windows:

Ditto.

>  (define %french-locale-name
> -  "fr_FR.ISO-8859-1")
> +  (if (string-contains %host-type "-mingw32")
> +      "fra_FRA.850"
> +      "fr_FR.ISO-8859-1"))
>  
>  (define %french-utf8-locale-name
> -  "fr_FR.UTF-8")
> +  (if (string-contains %host-type "-mingw32")
> +      "fra_FRA.1252"
> +      "fr_FR.UTF-8"))
>  
>  (define %turkish-utf8-locale-name
> -  "tr_TR.UTF-8")
> +  (if (string-contains %host-type "-mingw32")
> +      "tur_TRK.1254"
> +      "tr_TR.UTF-8"))
>  
>  (define %german-utf8-locale-name
> -  "de_DE.UTF-8")
> +  (if (string-contains %host-type "-mingw32")
> +      "deu_DEU.1252"
> +      "de_DE.UTF-8"))
>  
>  (define %greek-utf8-locale-name
> -  "el_GR.UTF-8")
> +  (if (string-contains %host-type "-mingw32")
> +      "grc_ELL.1253"
> +      "el_GR.UTF-8"))
>  
>  (define %american-english-locale-name
>    "en_US")
> @@ -148,13 +161,14 @@
>    (under-locale-or-unresolved %french-utf8-locale thunk))
>  
>  (define (under-turkish-utf8-locale-or-unresolved thunk)
> -  ;; FreeBSD 8.2 and 9.1, Solaris 2.10, and Darwin 8.11.0 have a broken
> -  ;; tr_TR locale where `i' is mapped to uppercase `I' instead of `Ä°',
> -  ;; so disable tests on that platform.
> +  ;; FreeBSD 8.2 and 9.1, Solaris 2.10, Darwin 8.11.0, and MinGW have
> +  ;; a broken tr_TR locale where `i' is mapped to uppercase `I'
> +  ;; instead of `İ', so disable tests on that platform.
>    (if (or (string-contains %host-type "freebsd8")
>            (string-contains %host-type "freebsd9")
>            (string-contains %host-type "solaris2.10")
> -          (string-contains %host-type "darwin8"))
> +          (string-contains %host-type "darwin8")
> +	  (string-contains %host-type "-mingw32"))
>        (throw 'unresolved)
>        (under-locale-or-unresolved %turkish-utf8-locale thunk)))
>  
> @@ -192,7 +206,10 @@
>          ;; strings.
>          (dynamic-wind
>            (lambda ()
> -            (setlocale LC_ALL "fr_FR.UTF-8"))
> +	    (setlocale LC_ALL
> +		       (if (string-contains %host-type "-mingw32")
> +			   "fra_FRA.1252"
> +			   "fr_FR.UTF-8")))
> 	   (lambda ()
> 	     (string-locale-ci=? "œuf" "ŒUF"))
> 	   (lambda ()
> 	     (setlocale LC_ALL "C"))))))

Pushed that as 700f6cd.

> After all these changes, some tests still fail or throw exceptions:
>
>   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=?
>   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=? (2 args, wide strings)
>   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=? (3 args, wide strings)
>   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>?
>   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>? (wide strings)
>   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>? (wide and narrow strings)
>   UNRESOLVED: i18n.test: text collation (French): char-locale-ci<>?
>   UNRESOLVED: i18n.test: text collation (French): char-locale-ci<>? (wide)
>   UNRESOLVED: i18n.test: text collation (Greek): string-locale-ci=?
>   UNRESOLVED: i18n.test: character mapping: char-locale-upcase Turkish
>   UNRESOLVED: i18n.test: character mapping: char-locale-downcase Turkish
>   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Greek
>   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Greek (two sigmas)
>   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Greek
>   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Greek (two sigmas)
>   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Turkish
>   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Turkish
>
> I don't know why these fail.

Note that “UNRESOLVED” is not a failure; it means “we can’t run this
test here, so skip it.”

> Is it possible that the underlying functions assume that the string
> arguments are encoded according to the locale's codeset?  If so, since
> the source file is encoded in UTF-8, that won't work on Windows, and
> the strings need to be recoded before they are passed to libunistring
> functions.  Any ideas for debugging this are welcome.

‘under-locale-or-unresolved’ catches any failure to install the French,
Greek, etc. locale (which can happens if the locale is missing on the
system), and throws 'unresolved when that happens.

So presumably, those UNRESOLVED mean that (setlocale LC_ALL "fra_FRA.850")
and similar calls just fail.

>   FAIL: i18n.test: nl-langinfo et al.: locale-day (French)
>   FAIL: i18n.test: nl-langinfo et al.: locale-day (French, using `%global-locale')
>
> This is because gnulib's nl_langinfo only supports C locale for the
> day names.  I'm taking this up with gnulib maintainers.

Great.

>   FAIL: i18n.test: number->locale-string: French: integer
>   FAIL: i18n.test: number->locale-string: French: fraction
>   FAIL: i18n.test: number->locale-string: French: fraction, 1 digit
>   FAIL: i18n.test: monetary-amount->locale-string: French: integer
>   FAIL: i18n.test: monetary-amount->locale-string: French: fraction
>
> There's no blank after the 7th digit, where the test expects it.  Not
> sure what kind of problem is that, perhaps again due to gnulib's
> nl_langinfo.

You could try this:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,m (ice-9 i18n)
scheme@(ice-9 i18n)> (locale-decimal-point (make-locale LC_ALL "fr_FR"))
$2 = ","
scheme@(ice-9 i18n)> (locale-thousands-separator (make-locale LC_ALL "fr_FR"))
$3 = " "
--8<---------------cut here---------------end--------------->8---

(Using the right locale name.)

>   UNRESOLVED: i18n.test: format ~h: French: 12345.5678
>   UNRESOLVED: i18n.test: format ~h: English: 12345.5678
>
> ~h is not supported on Windows.

~h is implemented using ‘number->locale-string’.

Thank you!

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-11 12:37       ` Ludovic Courtès
@ 2014-06-11 15:08         ` Eli Zaretskii
  2014-06-12  8:29           ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-11 15:08 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Wed, 11 Jun 2014 14:37:53 +0200
> 
> OK, thanks for the explanation.
> 
> Below is the fix I just pushed.
> [...]
> +(define %null-device
> +  ;; On Windows (MinGW), /dev/null does not exist and we must instead
> +  ;; use NUL.  Note that file system procedures automatically translate
> +  ;; /dev/null, so this variable is only useful for shell snippets.
> +  (if (file-exists? "/dev/null")
> +      "/dev/null"
> +      "NUL"))

Not sure this is a good idea: I can create a file /dev/null on
Windows, but that doesn't mean it is my null device.

>                        (with-input-from-port (car p2c)
>                          (lambda ()
>                            (open-input-pipe
> -                           "exec 1>/dev/null; echo closed 1>&2; exec 2>/dev/null; read REPLY")))))))
> +                           (format #f "exec 1>~a; echo closed 1>&2; \
> +exec 2>~a; read REPLY"
> +                                   %null-device %null-device))))))))

Here, the pipe will not work on Windows anyway, because there's no
'exec' command.  But it's good to use %null-device nonetheless, for
consistency.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-11 13:13     ` Ludovic Courtès
@ 2014-06-11 15:33       ` Eli Zaretskii
  2014-06-12  8:39         ` Ludovic Courtès
  2014-06-19  4:53       ` Doug Evans
  1 sibling, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-11 15:33 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Wed, 11 Jun 2014 15:13:58 +0200
> 
> > First, make-locale threw an exception, because it tried to call
> > 'setlocale' with LC_MESSAGES, which the Windows runtime doesn't
> > support.  locale-categories.h tried to avoid that by conditioning that
> > call by LC_MESSAGES being defined, but the oh-so-helpful libintl.h
> > header file just happens to define it to some arbitrary large
> > constant.  So the ifdef didn't work, and setlocale barfed.  Here's the
> > suggested solution:
> 
> Pushed as c84f25b.
> 
> > The next problem is that i18n.test uses Posix locale strings, whereas
> > the Windows runtime names the same locales by different names.
> > Moreover, Windows 'setlocale' doesn't support UTF-8 encoding (even
> > though a Windows UTF-8 codepage exists).  So every test for a locale
> > other than "C" was failing, because setlocale failed.  I replaced
> > Posix locales with similar Windows ones; see the following patch, in
> > which I also emoved all but one LC_MESSAGES, because these always fail
> > on Windows:
> 
> Ditto.

Thanks.

> > After all these changes, some tests still fail or throw exceptions:
> >
> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=?
> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=? (2 args, wide strings)
> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=? (3 args, wide strings)
> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>?
> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>? (wide strings)
> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>? (wide and narrow strings)
> >   UNRESOLVED: i18n.test: text collation (French): char-locale-ci<>?
> >   UNRESOLVED: i18n.test: text collation (French): char-locale-ci<>? (wide)
> >   UNRESOLVED: i18n.test: text collation (Greek): string-locale-ci=?
> >   UNRESOLVED: i18n.test: character mapping: char-locale-upcase Turkish
> >   UNRESOLVED: i18n.test: character mapping: char-locale-downcase Turkish
> >   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Greek
> >   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Greek (two sigmas)
> >   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Greek
> >   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Greek (two sigmas)
> >   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Turkish
> >   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Turkish
> >
> > I don't know why these fail.
> 
> Note that “UNRESOLVED” is not a failure; it means “we can’t run this
> test here, so skip it.”

Yes, I understand that.  But "can't run" usually means the test threw
some kind of exception, and I didn't understand why, especially since
_some_ of the text collation tests did work (compare the above with
the full list).

I now know what is the reason for that, and I cannot say that I'm
happier: it's libunistring's fault.  All these tests call libunistring
functions that require the locale's language as an argument.  Problem
is, libunistring doesn't support languages such as "fra" or "trk", it
only supports "fr" and "tr".  In general, it only supports 3-letter
language codes for those languages for which a 2-letter code doesn't
exist.  By contrast, Windows _always_ uses 3-letter codes in valid
locale names.

So what happens is that locale_language always returns an empty
string, and Guile calls u32_casecoll etc. with that empty string,
which only works in the "C" locale.  In any other locale, the
comparison fails with EILSEQ, and Guile throws the appropriate
exception.

IOW, libunistring's port to Windows is not really useful.

I will have to find some way around this issue, because otherwise
Guile will be crippled in any locale but en_US.

(Btw, why does Guile use libunistring instead of the ANSI functions
for locale-dependent string comparison and collation?)

> ‘under-locale-or-unresolved’ catches any failure to install the French,
> Greek, etc. locale (which can happens if the locale is missing on the
> system), and throws 'unresolved when that happens.

Well, after I solved the problem with LC_MESSAGES, make-locale no
longer throws 'unresolved.  The problem is now with the comparison
routines themselves, see above.

> So presumably, those UNRESOLVED mean that (setlocale LC_ALL "fra_FRA.850")
> and similar calls just fail.

No, that one works, I tested it manually.

> >   FAIL: i18n.test: number->locale-string: French: integer
> >   FAIL: i18n.test: number->locale-string: French: fraction
> >   FAIL: i18n.test: number->locale-string: French: fraction, 1 digit
> >   FAIL: i18n.test: monetary-amount->locale-string: French: integer
> >   FAIL: i18n.test: monetary-amount->locale-string: French: fraction
> >
> > There's no blank after the 7th digit, where the test expects it.  Not
> > sure what kind of problem is that, perhaps again due to gnulib's
> > nl_langinfo.
> 
> You could try this:
> 
> --8<---------------cut here---------------start------------->8---
> scheme@(guile-user)> ,m (ice-9 i18n)
> scheme@(ice-9 i18n)> (locale-decimal-point (make-locale LC_ALL "fr_FR"))
> $2 = ","
> scheme@(ice-9 i18n)> (locale-thousands-separator (make-locale LC_ALL "fr_FR"))
> $3 = " "
> --8<---------------cut here---------------end--------------->8---

I did try that, and saw a strange thing: the thousands separator is
displayed as "\xa0".  That is very strange, because nl_langinfo does
return " " for the French locale, as expected.  Why would the blank be
translated into NBSP?  Can this also be due to libunistring problems?

> (Using the right locale name.)
> 
> >   UNRESOLVED: i18n.test: format ~h: French: 12345.5678
> >   UNRESOLVED: i18n.test: format ~h: English: 12345.5678
> >
> > ~h is not supported on Windows.
> 
> ~h is implemented using ‘number->locale-string’.

Maybe I'm confused, but isn't ~h about position directive in formats?
These don't work on Windows.

Thanks.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-11 15:08         ` Eli Zaretskii
@ 2014-06-12  8:29           ` Ludovic Courtès
  2014-06-12 17:16             ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-12  8:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel@gnu.org
>> Date: Wed, 11 Jun 2014 14:37:53 +0200
>> 
>> OK, thanks for the explanation.
>> 
>> Below is the fix I just pushed.
>> [...]
>> +(define %null-device
>> +  ;; On Windows (MinGW), /dev/null does not exist and we must instead
>> +  ;; use NUL.  Note that file system procedures automatically translate
>> +  ;; /dev/null, so this variable is only useful for shell snippets.
>> +  (if (file-exists? "/dev/null")
>> +      "/dev/null"
>> +      "NUL"))
>
> Not sure this is a good idea: I can create a file /dev/null on
> Windows, but that doesn't mean it is my null device.

Yes, but using %host-type isn’t perfect either, no?  What would you
prefer?

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-11 15:33       ` Eli Zaretskii
@ 2014-06-12  8:39         ` Ludovic Courtès
  2014-06-12 18:18           ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-12  8:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel@gnu.org
>> Date: Wed, 11 Jun 2014 15:13:58 +0200

[...]

>> > After all these changes, some tests still fail or throw exceptions:
>> >
>> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=?
>> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=? (2 args, wide strings)
>> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci=? (3 args, wide strings)
>> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>?
>> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>? (wide strings)
>> >   UNRESOLVED: i18n.test: text collation (French): string-locale-ci<>? (wide and narrow strings)
>> >   UNRESOLVED: i18n.test: text collation (French): char-locale-ci<>?
>> >   UNRESOLVED: i18n.test: text collation (French): char-locale-ci<>? (wide)
>> >   UNRESOLVED: i18n.test: text collation (Greek): string-locale-ci=?
>> >   UNRESOLVED: i18n.test: character mapping: char-locale-upcase Turkish
>> >   UNRESOLVED: i18n.test: character mapping: char-locale-downcase Turkish
>> >   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Greek
>> >   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Greek (two sigmas)
>> >   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Greek
>> >   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Greek (two sigmas)
>> >   UNRESOLVED: i18n.test: string mapping: string-locale-upcase Turkish
>> >   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Turkish
>> >
>> > I don't know why these fail.
>> 
>> Note that “UNRESOLVED” is not a failure; it means “we can’t run this
>> test here, so skip it.”
>
> Yes, I understand that.  But "can't run" usually means the test threw
> some kind of exception, and I didn't understand why, especially since
> _some_ of the text collation tests did work (compare the above with
> the full list).
>
> I now know what is the reason for that, and I cannot say that I'm
> happier: it's libunistring's fault.  All these tests call libunistring
> functions that require the locale's language as an argument.  Problem
> is, libunistring doesn't support languages such as "fra" or "trk", it
> only supports "fr" and "tr".  In general, it only supports 3-letter
> language codes for those languages for which a 2-letter code doesn't
> exist.  By contrast, Windows _always_ uses 3-letter codes in valid
> locale names.
>
> So what happens is that locale_language always returns an empty
> string, and Guile calls u32_casecoll etc. with that empty string,
> which only works in the "C" locale.  In any other locale, the
> comparison fails with EILSEQ, and Guile throws the appropriate
> exception.

OK.  (It would be nice if someone would take over maintainership of
libunistring...)

> IOW, libunistring's port to Windows is not really useful.
>
> I will have to find some way around this issue, because otherwise
> Guile will be crippled in any locale but en_US.
>
> (Btw, why does Guile use libunistring instead of the ANSI functions
> for locale-dependent string comparison and collation?)

Because strings are internally either Latin-1 or UTF-32 (UCS-4).

[...]

>> >   FAIL: i18n.test: number->locale-string: French: integer
>> >   FAIL: i18n.test: number->locale-string: French: fraction
>> >   FAIL: i18n.test: number->locale-string: French: fraction, 1 digit
>> >   FAIL: i18n.test: monetary-amount->locale-string: French: integer
>> >   FAIL: i18n.test: monetary-amount->locale-string: French: fraction
>> >
>> > There's no blank after the 7th digit, where the test expects it.  Not
>> > sure what kind of problem is that, perhaps again due to gnulib's
>> > nl_langinfo.
>> 
>> You could try this:
>> 
>> --8<---------------cut here---------------start------------->8---
>> scheme@(guile-user)> ,m (ice-9 i18n)
>> scheme@(ice-9 i18n)> (locale-decimal-point (make-locale LC_ALL "fr_FR"))
>> $2 = ","
>> scheme@(ice-9 i18n)> (locale-thousands-separator (make-locale LC_ALL "fr_FR"))
>> $3 = " "
>> --8<---------------cut here---------------end--------------->8---
>
> I did try that, and saw a strange thing: the thousands separator is
> displayed as "\xa0".  That is very strange, because nl_langinfo does
> return " " for the French locale, as expected.  Why would the blank be
> translated into NBSP?  Can this also be due to libunistring problems?

NBSP is actually a better answer than just space, because it’d be unwise
to introduce a break in the middle of a number.

So does ‘number->locale-string’ return "123\xa0456" for you?

>> (Using the right locale name.)
>> 
>> >   UNRESOLVED: i18n.test: format ~h: French: 12345.5678
>> >   UNRESOLVED: i18n.test: format ~h: English: 12345.5678
>> >
>> > ~h is not supported on Windows.
>> 
>> ~h is implemented using ‘number->locale-string’.
>
> Maybe I'm confused, but isn't ~h about position directive in formats?

Yes, but that’s implemented in Scheme, in ice-9/format.scm.

> These don't work on Windows.

What doesn’t work?  ‘format’ doesn’t rely on any non-portable OS
facility.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-12  8:29           ` Ludovic Courtès
@ 2014-06-12 17:16             ` Eli Zaretskii
  2014-06-12 19:48               ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-12 17:16 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Thu, 12 Jun 2014 10:29:24 +0200
> 
> >> +(define %null-device
> >> +  ;; On Windows (MinGW), /dev/null does not exist and we must instead
> >> +  ;; use NUL.  Note that file system procedures automatically translate
> >> +  ;; /dev/null, so this variable is only useful for shell snippets.
> >> +  (if (file-exists? "/dev/null")
> >> +      "/dev/null"
> >> +      "NUL"))
> >
> > Not sure this is a good idea: I can create a file /dev/null on
> > Windows, but that doesn't mean it is my null device.
> 
> Yes, but using %host-type isn’t perfect either, no?  What would you
> prefer?

How about testing if the absolute name of the current directory starts
with a drive letter?




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-12  8:39         ` Ludovic Courtès
@ 2014-06-12 18:18           ` Eli Zaretskii
  2014-06-15 17:23             ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-12 18:18 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Thu, 12 Jun 2014 10:39:08 +0200
> 
> > I now know what is the reason for that, and I cannot say that I'm
> > happier: it's libunistring's fault.  All these tests call libunistring
> > functions that require the locale's language as an argument.  Problem
> > is, libunistring doesn't support languages such as "fra" or "trk", it
> > only supports "fr" and "tr".  In general, it only supports 3-letter
> > language codes for those languages for which a 2-letter code doesn't
> > exist.  By contrast, Windows _always_ uses 3-letter codes in valid
> > locale names.
> >
> > So what happens is that locale_language always returns an empty
> > string, and Guile calls u32_casecoll etc. with that empty string,
> > which only works in the "C" locale.  In any other locale, the
> > comparison fails with EILSEQ, and Guile throws the appropriate
> > exception.

It turns out the truth is actually much worse.  The main problem is
not with the language, it's with the locale's codeset.  On Windows,
libunistring always thinks that the codeset is the default console
codepage, disregarding any changes by 'setlocale'.  Therefore, text in
any script that is not supported by the default console codepage will
always cause EILSEQ from libunistring.

And while the locale's language is an argument to libunistring APIs,
and therefore can be "fixed" in Guile, the codeset is extracted and
used internally inside libunistring, and never exposed to any API.

> OK.  (It would be nice if someone would take over maintainership of
> libunistring...)

Indeed.  But given the above situation, I just went ahead, built
libunistring from sources, and patched them.  Doing so made almost all
the problems with i18n.test disappear (and I also discovered 2
problems with my previous patch that you already pushed, see below).

I still have one problem left: the Turkish character-mapping tests
are failing.  I think that's because somehow the LC_ALL environment
variable gets set to "C".  With the current libunistring code, that
setting in the environment overrides what's been set by 'setlocale',
and the Turkish language rules are not used.

I will fix this in libunistring, but do you have any idea which code
might be pushing LC_ALL=C into the Guile's environment during the test
suite run?

By the way, how do I run a single test from test-suite?

> > (Btw, why does Guile use libunistring instead of the ANSI functions
> > for locale-dependent string comparison and collation?)
> 
> Because strings are internally either Latin-1 or UTF-32 (UCS-4).

Of course, but did you see what libunistring does?  It calls libiconv
to encode the Unicode strings into the locale's codeset (that's why I
got EILSEQ earlier, see above), and then works with the encoded
string.  Since Guile has a libiconv interface as well, it could easily
do the same, no?  Once the string is in the locale's codeset, all the
libc functions will DTRT wrt collating, sorting, etc.

> >> --8<---------------cut here---------------start------------->8---
> >> scheme@(guile-user)> ,m (ice-9 i18n)
> >> scheme@(ice-9 i18n)> (locale-decimal-point (make-locale LC_ALL "fr_FR"))
> >> $2 = ","
> >> scheme@(ice-9 i18n)> (locale-thousands-separator (make-locale LC_ALL "fr_FR"))
> >> $3 = " "
> >> --8<---------------cut here---------------end--------------->8---
> >
> > I did try that, and saw a strange thing: the thousands separator is
> > displayed as "\xa0".  That is very strange, because nl_langinfo does
> > return " " for the French locale, as expected.  Why would the blank be
> > translated into NBSP?  Can this also be due to libunistring problems?
> 
> NBSP is actually a better answer than just space, because it’d be unwise
> to introduce a break in the middle of a number.

But nl_langinfo returns a blank.  So who converts that to NBSP?

> So does ‘number->locale-string’ return "123\xa0456" for you?

No, I get "123456".  I will revisit this after I finish fixing
libunistring, 

> >> >   UNRESOLVED: i18n.test: format ~h: French: 12345.5678
> >> >   UNRESOLVED: i18n.test: format ~h: English: 12345.5678
> >> >
> >> > ~h is not supported on Windows.
> >> 
> >> ~h is implemented using ‘number->locale-string’.
> >
> > Maybe I'm confused, but isn't ~h about position directive in formats?
> 
> Yes, but that’s implemented in Scheme, in ice-9/format.scm.

Thanks for the pointer, I guess I will need to take a better look at
that.

> > These don't work on Windows.
> 
> What doesn’t work?  ‘format’ doesn’t rely on any non-portable OS
> facility.

I meant positional arguments in printf formats.

Anyway, part of the changes for i18n.test I sent before were in error,
sorry.  Here's a small patch relative to the current git:

diff --git a/test-suite/tests/i18n.test b/test-suite/tests/i18n.test
index c63e3ac..b51ff15 100644
--- a/test-suite/tests/i18n.test
+++ b/test-suite/tests/i18n.test
@@ -99,7 +99,7 @@
 
 (define %turkish-utf8-locale-name
   (if mingw?
-      "tur_TRK.1254"
+      "trk_TUR.1254"
       "tr_TR.UTF-8"))
 
 (define %german-utf8-locale-name
@@ -109,7 +109,7 @@
 
 (define %greek-utf8-locale-name
   (if mingw?
-      "grc_ELL.1253"
+      "ell_GRC.1253"
       "el_GR.UTF-8"))
 
 (define %american-english-locale-name
@@ -164,14 +164,13 @@
   (under-locale-or-unresolved %french-utf8-locale thunk))
 
 (define (under-turkish-utf8-locale-or-unresolved thunk)
-  ;; FreeBSD 8.2 and 9.1, Solaris 2.10, Darwin 8.11.0, and MinGW have
+  ;; FreeBSD 8.2 and 9.1, Solaris 2.10, and Darwin 8.11.0 have
   ;; a broken tr_TR locale where `i' is mapped to uppercase `I'
   ;; instead of `İ', so disable tests on that platform.
   (if (or (string-contains %host-type "freebsd8")
           (string-contains %host-type "freebsd9")
           (string-contains %host-type "solaris2.10")
-          (string-contains %host-type "darwin8")
-          (string-contains %host-type "mingw32"))
+          (string-contains %host-type "darwin8"))
       (throw 'unresolved)
       (under-locale-or-unresolved %turkish-utf8-locale thunk)))
 




^ permalink raw reply related	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-12 17:16             ` Eli Zaretskii
@ 2014-06-12 19:48               ` Ludovic Courtès
  2014-06-12 19:59                 ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-12 19:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel@gnu.org
>> Date: Thu, 12 Jun 2014 10:29:24 +0200
>> 
>> >> +(define %null-device
>> >> +  ;; On Windows (MinGW), /dev/null does not exist and we must instead
>> >> +  ;; use NUL.  Note that file system procedures automatically translate
>> >> +  ;; /dev/null, so this variable is only useful for shell snippets.
>> >> +  (if (file-exists? "/dev/null")
>> >> +      "/dev/null"
>> >> +      "NUL"))
>> >
>> > Not sure this is a good idea: I can create a file /dev/null on
>> > Windows, but that doesn't mean it is my null device.
>> 
>> Yes, but using %host-type isn’t perfect either, no?  What would you
>> prefer?
>
> How about testing if the absolute name of the current directory starts
> with a drive letter?

Like (string-match "^[a-zA-Z]:[/\\]" (getcwd)) ?

Sounds reasonable.

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-12 19:48               ` Ludovic Courtès
@ 2014-06-12 19:59                 ` Eli Zaretskii
  2014-06-12 21:20                   ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-12 19:59 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Thu, 12 Jun 2014 21:48:48 +0200
> 
> >> >> +(define %null-device
> >> >> +  ;; On Windows (MinGW), /dev/null does not exist and we must instead
> >> >> +  ;; use NUL.  Note that file system procedures automatically translate
> >> >> +  ;; /dev/null, so this variable is only useful for shell snippets.
> >> >> +  (if (file-exists? "/dev/null")
> >> >> +      "/dev/null"
> >> >> +      "NUL"))
> >> >
> >> > Not sure this is a good idea: I can create a file /dev/null on
> >> > Windows, but that doesn't mean it is my null device.
> >> 
> >> Yes, but using %host-type isn’t perfect either, no?  What would you
> >> prefer?
> >
> > How about testing if the absolute name of the current directory starts
> > with a drive letter?
> 
> Like (string-match "^[a-zA-Z]:[/\\]" (getcwd)) ?

Yes.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-12 19:59                 ` Eli Zaretskii
@ 2014-06-12 21:20                   ` Ludovic Courtès
  2014-06-13  9:15                     ` Neil Jerram
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-12 21:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel@gnu.org
>> Date: Thu, 12 Jun 2014 21:48:48 +0200
>> 
>> >> >> +(define %null-device
>> >> >> +  ;; On Windows (MinGW), /dev/null does not exist and we must instead
>> >> >> +  ;; use NUL.  Note that file system procedures automatically translate
>> >> >> +  ;; /dev/null, so this variable is only useful for shell snippets.
>> >> >> +  (if (file-exists? "/dev/null")
>> >> >> +      "/dev/null"
>> >> >> +      "NUL"))
>> >> >
>> >> > Not sure this is a good idea: I can create a file /dev/null on
>> >> > Windows, but that doesn't mean it is my null device.
>> >> 
>> >> Yes, but using %host-type isn’t perfect either, no?  What would you
>> >> prefer?
>> >
>> > How about testing if the absolute name of the current directory starts
>> > with a drive letter?
>> 
>> Like (string-match "^[a-zA-Z]:[/\\]" (getcwd)) ?
>
> Yes.

Pushed, thanks.

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-12 21:20                   ` Ludovic Courtès
@ 2014-06-13  9:15                     ` Neil Jerram
  2014-06-13 16:04                       ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Neil Jerram @ 2014-06-13  9:15 UTC (permalink / raw)
  To: guile-devel

> Eli Zaretskii <eliz@gnu.org> skribis:
> 
>>> From: ludo@gnu.org (Ludovic Courtès)
>>> Cc: guile-devel@gnu.org
>>> Date: Thu, 12 Jun 2014 21:48:48 +0200
>>> 
>>> >> >> +(define %null-device
>>> >> >> +  ;; On Windows (MinGW), /dev/null does not exist and we must instead
>>> >> >> +  ;; use NUL.  Note that file system procedures automatically translate
>>> >> >> +  ;; /dev/null, so this variable is only useful for shell snippets.
>>> >> >> +  (if (file-exists? "/dev/null")
>>> >> >> +      "/dev/null"
>>> >> >> +      "NUL"))
>>> >> >
>>> >> > Not sure this is a good idea: I can create a file /dev/null on
>>> >> > Windows, but that doesn't mean it is my null device.
>>> >>
>>> >> Yes, but using %host-type isn’t perfect either, no?  What would you
>>> >> prefer?
>>> >
>>> > How about testing if the absolute name of the current directory starts
>>> > with a drive letter?
>>> 
>>> Like (string-match "^[a-zA-Z]:[/\\]" (getcwd)) ?
>> 
>> Yes.

But my Git Bash shell on Windows (at work) gives me paths like /<drive 
letter>/...
For example:

   nj@PC3946 /c/work/icp (master)
   $ pwd
   /c/work/icp

I think that shell is provided by MinGW/MSYS - so does that mean that 
the regexp check
above might not be correct in all contexts on Windows?

        Neil




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-13  9:15                     ` Neil Jerram
@ 2014-06-13 16:04                       ` Ludovic Courtès
  2014-06-13 16:19                         ` Eli Zaretskii
  2014-06-13 16:31                         ` Mike Gerwitz
  0 siblings, 2 replies; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-13 16:04 UTC (permalink / raw)
  To: Neil Jerram; +Cc: Eli Zaretskii, guile-devel

Neil Jerram <neil@ossau.homelinux.net> skribis:

>> Eli Zaretskii <eliz@gnu.org> skribis:
>>
>>>> From: ludo@gnu.org (Ludovic Courtès)
>>>> Cc: guile-devel@gnu.org
>>>> Date: Thu, 12 Jun 2014 21:48:48 +0200
>>>>
>>>> >> >> +(define %null-device
>>>> >> >> +  ;; On Windows (MinGW), /dev/null does not exist and we must instead
>>>> >> >> +  ;; use NUL.  Note that file system procedures automatically translate
>>>> >> >> +  ;; /dev/null, so this variable is only useful for shell snippets.
>>>> >> >> +  (if (file-exists? "/dev/null")
>>>> >> >> +      "/dev/null"
>>>> >> >> +      "NUL"))
>>>> >> >
>>>> >> > Not sure this is a good idea: I can create a file /dev/null on
>>>> >> > Windows, but that doesn't mean it is my null device.
>>>> >>
>>>> >> Yes, but using %host-type isn’t perfect either, no?  What would you
>>>> >> prefer?
>>>> >
>>>> > How about testing if the absolute name of the current directory starts
>>>> > with a drive letter?
>>>>
>>>> Like (string-match "^[a-zA-Z]:[/\\]" (getcwd)) ?
>>>
>>> Yes.
>
> But my Git Bash shell on Windows (at work) gives me paths like /<drive
> letter>/...
> For example:
>
>   nj@PC3946 /c/work/icp (master)
>   $ pwd
>   /c/work/icp
>
> I think that shell is provided by MinGW/MSYS - so does that mean that
> the regexp check
> above might not be correct in all contexts on Windows?

Isn’t it rather provided by Cygwin?

I would think that (getcwd) on Cygwin would return /c/... whereas
(getcwd) on MinGW would return C:\..., no?

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-13 16:04                       ` Ludovic Courtès
@ 2014-06-13 16:19                         ` Eli Zaretskii
  2014-06-13 16:26                           ` Neil Jerram
  2014-06-13 16:31                         ` Mike Gerwitz
  1 sibling, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-13 16:19 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> CC: Eli Zaretskii <eliz@gnu.org>, guile-devel@gnu.org
> Date: Fri, 13 Jun 2014 18:04:57 +0200
> 
> >>>> Like (string-match "^[a-zA-Z]:[/\\]" (getcwd)) ?
> >>>
> >>> Yes.
> >
> > But my Git Bash shell on Windows (at work) gives me paths like /<drive
> > letter>/...
> > For example:
> >
> >   nj@PC3946 /c/work/icp (master)
> >   $ pwd
> >   /c/work/icp

Git Bash is an MSYS program.  MSYS is a fork of an old version of
Cygwin, and the pseudo-Posix file names it returns is one of its
features, designed to make Posix shell scripts work without choking on
Windows file names with drive letters.

So this has nothing to do with native Windows programs produced by
MinGW, which is what we are discussing here: how to redirect to a null
device in a native MinGW compiled Guile whose 'system' procedure
invokes the Windows shell cmd.exe.

> Isn’t it rather provided by Cygwin?

Yes, Cygwin and MSYS.

> I would think that (getcwd) on Cygwin would return /c/... whereas
> (getcwd) on MinGW would return C:\..., no?

The MinGW version should produce either C:\... or C:/..., yes.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-13 16:19                         ` Eli Zaretskii
@ 2014-06-13 16:26                           ` Neil Jerram
  0 siblings, 0 replies; 67+ messages in thread
From: Neil Jerram @ 2014-06-13 16:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ludo, guile-devel

On 2014-06-13 17:19, Eli Zaretskii wrote:
>> From: ludo@gnu.org (Ludovic Courtès)
>> CC: Eli Zaretskii <eliz@gnu.org>, guile-devel@gnu.org
>> Date: Fri, 13 Jun 2014 18:04:57 +0200
>> 
>> >>>> Like (string-match "^[a-zA-Z]:[/\\]" (getcwd)) ?
>> >>>
>> >>> Yes.
>> >
>> > But my Git Bash shell on Windows (at work) gives me paths like /<drive
>> > letter>/...
>> > For example:
>> >
>> >   nj@PC3946 /c/work/icp (master)
>> >   $ pwd
>> >   /c/work/icp
> 
> Git Bash is an MSYS program.  MSYS is a fork of an old version of
> Cygwin, and the pseudo-Posix file names it returns is one of its
> features, designed to make Posix shell scripts work without choking on
> Windows file names with drive letters.
> 
> So this has nothing to do with native Windows programs produced by
> MinGW, which is what we are discussing here: how to redirect to a null
> device in a native MinGW compiled Guile whose 'system' procedure
> invokes the Windows shell cmd.exe.
> 
>> Isn’t it rather provided by Cygwin?
> 
> Yes, Cygwin and MSYS.
> 
>> I would think that (getcwd) on Cygwin would return /c/... whereas
>> (getcwd) on MinGW would return C:\..., no?
> 
> The MinGW version should produce either C:\... or C:/..., yes.

Thanks for explaining.  I didn't know of the connection between MSYS and 
Cygwin.

      Neil




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-13 16:04                       ` Ludovic Courtès
  2014-06-13 16:19                         ` Eli Zaretskii
@ 2014-06-13 16:31                         ` Mike Gerwitz
  2014-06-13 18:09                           ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Mike Gerwitz @ 2014-06-13 16:31 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Eli Zaretskii, guile-devel

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

On Fri, Jun 13, 2014 at 06:04:57PM +0200, Ludovic Courtès wrote:
> > But my Git Bash shell on Windows (at work) gives me paths like /<drive
> > letter>/...
> > For example:
> >
> >   nj@PC3946 /c/work/icp (master)
> >   $ pwd
> >   /c/work/icp
> >
> > I think that shell is provided by MinGW/MSYS - so does that mean that
> > the regexp check
> > above might not be correct in all contexts on Windows?
> 
> Isn’t it rather provided by Cygwin?
> 
> I would think that (getcwd) on Cygwin would return /c/... whereas
> (getcwd) on MinGW would return C:\..., no?

My coworkers use MinGW, and it does expose [A-Z]:\ as /[a-z]/; all paths in
MinGW are expected to be Unix-style.

I used the `uname` binary to determine if MinGW was being used with shell
scripts; it returns "MINGW32_NT-6.1". I do not know what Cygwin returns.
That said, I'm expecting that Guile's (uname) will return something wholly
different depending on what was used to compile it! Another method I used
was also checking the MSYSTEM environment variable, which is set to
"MINGW32" on those systems.

All that said, on msys:

  $ echo foo > C:\foo
  $ cat /c/foo
  foo
  $ echo foo > C:\nul
  $ cat /c/nul
  # no output

-- 
Mike Gerwitz
Free Software Hacker | GNU Maintainer
http://mikegerwitz.com
FSF Member #5804 | GPG Key ID: 0x8EE30EAB

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. c-api.test
  2014-06-13 16:31                         ` Mike Gerwitz
@ 2014-06-13 18:09                           ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-13 18:09 UTC (permalink / raw)
  To: Mike Gerwitz; +Cc: ludo, guile-devel

> Date: Fri, 13 Jun 2014 12:31:14 -0400
> From: Mike Gerwitz <mikegerwitz@gnu.org>
> Cc: Neil Jerram <neil@ossau.homelinux.net>, Eli Zaretskii <eliz@gnu.org>,
> 	guile-devel@gnu.org
> 
> My coworkers use MinGW, and it does expose [A-Z]:\ as /[a-z]/; all paths in
> MinGW are expected to be Unix-style.

They are most probably using the MSYS Bash as the shell.  They must,
because there's no decent port of Bash (or any Posix shell) as a
native Windows program.

MSYS tools exist to allow configuring and building MinGW programs
using Posix configury and Makefiles.  But MSYS is not MinGW; rather,
it is a companion to MinGW.  Some people also use the MSYS Bash as
their interactive shell, but that doesn't make that shell a MinGW
program.

> I used the `uname` binary to determine if MinGW was being used with shell
> scripts; it returns "MINGW32_NT-6.1".

What the uname binary returns is determined at compile time (with the
exception of the OS version, which is the 6.1 part in what you show).
I have here 2 MSYS uname binaries (one that I use to build MinGW
programs, the other came with msysgit), and one MinGW uname binary.
Here's what they report _on_the_same_system_:

  MINGW32_NT-5.1 HOME-C4E4A596F7 1.0.17(0.48/3/2) 2011-04-24 23:39 i686 Msys
  MINGW32_NT-5.1 HOME-C4E4A596F7 1.0.12(0.46/3/2) 2012-07-05 14:56 i686 unknown
  windows32 home-c4e4a596f7 2.5.1 2600 i686-pc Intel unknown MinGW

The first 2 are from MSYS uname binaries, the last one is from a
native MinGW binary.

> That said, I'm expecting that Guile's (uname) will return something wholly
> different depending on what was used to compile it!

The MinGW compiled Guile shows this:

  scheme@(guile-user)> (uname)
  $1 = #("Windows XP" "HOME-C4E4A596F7" "build 2600" "5.01 Service Pack 3" "i686")

Which is what I expect.  I'm not aware of an MSYS build of Guile, so I
don't know what it will return.  And I don't have Cygwin here to try
that.

Moreover, I don't quite see what the uname report has to do with the
issue at hand, which is how to use the null device portably.

> Another method I used was also checking the MSYSTEM environment
> variable, which is set to "MINGW32" on those systems.

MSYSTEM is the environment variable set by MSYS's /etc/profile file.
Its existence and value do not mean that you are in a MinGW shell.  As
I wrote above, there's no MinGW port of Bash that is bug-free enough
for it to be able to run a garden-variety configure script.  So anyone
who runs Bash on Windows is either running an MSYS Bash or a Cygwin
Bash.

> All that said, on msys:
> 
>   $ echo foo > C:\foo
>   $ cat /c/foo
>   foo
>   $ echo foo > C:\nul
>   $ cat /c/nul
>   # no output

Yes, that's MSYS for you.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-12 18:18           ` Eli Zaretskii
@ 2014-06-15 17:23             ` Eli Zaretskii
  2014-06-21 11:17               ` Eli Zaretskii
  2014-06-21 15:02               ` Ludovic Courtès
  0 siblings, 2 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-15 17:23 UTC (permalink / raw)
  To: ludo; +Cc: guile-devel

> Date: Thu, 12 Jun 2014 21:18:51 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: guile-devel@gnu.org
> 
> I still have one problem left: the Turkish character-mapping tests
> are failing.  I think that's because somehow the LC_ALL environment
> variable gets set to "C".  With the current libunistring code, that
> setting in the environment overrides what's been set by 'setlocale',
> and the Turkish language rules are not used.
> 
> I will fix this in libunistring

Done, and all the tests in i18n.test now pass on Windows.  But I also
needed another small change in i18n.c, see below.

> By the way, how do I run a single test from test-suite?

I'd still love to know the answer to this one.

> > >> --8<---------------cut here---------------start------------->8---
> > >> scheme@(guile-user)> ,m (ice-9 i18n)
> > >> scheme@(ice-9 i18n)> (locale-decimal-point (make-locale LC_ALL "fr_FR"))
> > >> $2 = ","
> > >> scheme@(ice-9 i18n)> (locale-thousands-separator (make-locale LC_ALL "fr_FR"))
> > >> $3 = " "
> > >> --8<---------------cut here---------------end--------------->8---
> > >
> > > I did try that, and saw a strange thing: the thousands separator is
> > > displayed as "\xa0".  That is very strange, because nl_langinfo does
> > > return " " for the French locale, as expected.  Why would the blank be
> > > translated into NBSP?  Can this also be due to libunistring problems?
> > 
> > NBSP is actually a better answer than just space, because it’d be unwise
> > to introduce a break in the middle of a number.
> 
> But nl_langinfo returns a blank.  So who converts that to NBSP?

Answering myself here: no one.  What I thought was a blank was
actually NBSP (which was displayed as blank by GDB), that's what the
Windows French locale returns as the thousands separator.  I guess
i18n.test shouldn't assume the separator is a blank, but should
instead use the actual character.

> > So does ‘number->locale-string’ return "123\xa0456" for you?
> 
> No, I get "123456".  I will revisit this after I finish fixing
> libunistring,

Revisited and fixed.  It turned out i18n.scm needed to be recompiled,
because it records the supported values of nl_langinfo arguments in
the .go file.  So whenever more supported values are added to
nl_langinfo (which was what I did for GROUPING), i18n.scm should be
recompiled.  (Shouldn't this happen automatically?)

> > >> >   UNRESOLVED: i18n.test: format ~h: French: 12345.5678
> > >> >   UNRESOLVED: i18n.test: format ~h: English: 12345.5678
> > >> >
> > >> > ~h is not supported on Windows.
> > >> 
> > >> ~h is implemented using ‘number->locale-string’.
> > >
> > > Maybe I'm confused, but isn't ~h about position directive in formats?
> > 
> > Yes, but that’s implemented in Scheme, in ice-9/format.scm.
> 
> Thanks for the pointer, I guess I will need to take a better look at
> that.

Once i18n.scm was recompiled, the ~h format test also started working.

There are a couple of other locale-related tests that fail on Windows,
like the one that reads Unicode text from strings.  I decided not to
fix those, because they test a fundamentally non-portable operation.

Here are the changes I needed for i18n.c to get the tests to succeed.
They have to do with non-portable assumptions about when the various
nl_langinfo constants are defined.  E.g., there's no reason to assume
that if INT_FRAC_DIGITS isn't defined, neither will be FRAC_DIGITS.
As luck would have it, MinGW has some of these, but not the others, so
the conditionals failed, and Guile failed to convert P_SIGN_POSN to
one of the symbolic values, instead leaving it at its numerical value.

--- libguile/i18n.c~2	2014-06-15 14:21:53 +0300
+++ libguile/i18n.c	2014-06-15 14:58:09 +0300
@@ -1583,9 +1583,13 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
 	  }
 #endif
 
-#if (defined FRAC_DIGITS) && (defined INT_FRAC_DIGITS)
+#if defined FRAC_DIGITS || defined INT_FRAC_DIGITS
+#ifdef FRAC_DIGITS
 	case FRAC_DIGITS:
+#endif
+#ifdef INT_FRAC_DIGITS
 	case INT_FRAC_DIGITS:
+#endif
 	  /* This is to be interpreted as a single integer.  */
 	  if (*c_result == CHAR_MAX)
 	    /* Unspecified.  */
@@ -1597,12 +1601,18 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
 	  break;
 #endif
 
-#if (defined P_CS_PRECEDES) && (defined INT_N_CS_PRECEDES)
+#if defined P_CS_PRECEDES || defined N_CS_PRECEDES ||	\
+  defined INT_P_CS_PRECEDES || defined INT_N_CS_PRECEDES || \
+  defined P_SEP_BY_SPACE || defined N_SEP_BY_SPACE
+#ifdef P_CS_PRECEDES
 	case P_CS_PRECEDES:
 	case N_CS_PRECEDES:
+#endif
+#ifdef INT_N_CS_PRECEDES
 	case INT_P_CS_PRECEDES:
 	case INT_N_CS_PRECEDES:
-#if (defined P_SEP_BY_SPACE) && (defined N_SEP_BY_SPACE)
+#endif
+#ifdef P_SEP_BY_SPACE
 	case P_SEP_BY_SPACE:
 	case N_SEP_BY_SPACE:
 #endif
@@ -1613,11 +1623,16 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
 	  break;
 #endif
 
-#if (defined P_SIGN_POSN) && (defined INT_N_SIGN_POSN)
+#if defined P_SIGN_POSN || defined N_SIGN_POSN || \
+  defined INT_P_SIGN_POSN || defined INT_N_SIGN_POSN
+#ifdef P_SIGN_POSN
 	case P_SIGN_POSN:
 	case N_SIGN_POSN:
+#endif
+#ifdef INT_P_SIGN_POSN
 	case INT_P_SIGN_POSN:
 	case INT_N_SIGN_POSN:
+#endif
 	  /* See `(libc) Sign of Money Amount' for the interpretation of the
 	     return value here.  */
 	  switch (*c_result)




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-11 13:13     ` Ludovic Courtès
  2014-06-11 15:33       ` Eli Zaretskii
@ 2014-06-19  4:53       ` Doug Evans
  2014-06-19  8:16         ` Ludovic Courtès
  1 sibling, 1 reply; 67+ messages in thread
From: Doug Evans @ 2014-06-19  4:53 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Eli Zaretskii, guile-devel

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

On Jun 11, 2014 3:14 PM, "Ludovic Courtès" <ludo@gnu.org> wrote:
>
> Eli Zaretskii <eliz@gnu.org> skribis:
>
> >> [...]
> >   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Turkish
> >
> > I don't know why these fail.
>
> Note that “UNRESOLVED” is not a failure; it means “we can’t run this
> test here, so skip it.”

Hi.
At least in GDB, I generally infer UNRESOLVED to mean "action needed"
(could be test harness bug or some such), whereas if a test can't be run in
a particular config I see UNSUPPORTED which I infer to mean "no action
needed".

[-- Attachment #2: Type: text/html, Size: 846 bytes --]

^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-19  4:53       ` Doug Evans
@ 2014-06-19  8:16         ` Ludovic Courtès
  0 siblings, 0 replies; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-19  8:16 UTC (permalink / raw)
  To: Doug Evans; +Cc: Eli Zaretskii, guile-devel

Doug Evans <xdje42@gmail.com> skribis:

> On Jun 11, 2014 3:14 PM, "Ludovic Courtès" <ludo@gnu.org> wrote:
>>
>> Eli Zaretskii <eliz@gnu.org> skribis:
>>
>> >> [...]
>> >   UNRESOLVED: i18n.test: string mapping: string-locale-downcase Turkish
>> >
>> > I don't know why these fail.
>>
>> Note that “UNRESOLVED” is not a failure; it means “we can’t run this
>> test here, so skip it.”
>
> Hi.
> At least in GDB, I generally infer UNRESOLVED to mean "action needed"
> (could be test harness bug or some such), whereas if a test can't be run in
> a particular config I see UNSUPPORTED which I infer to mean "no action
> needed".

Yes, that makes sense, but for historical reasons (perhaps a historical
misunderstanding? ;-)), Guile’s test suite has been using “UNRESOLVED”
with the meaning above.

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-15 17:23             ` Eli Zaretskii
@ 2014-06-21 11:17               ` Eli Zaretskii
  2014-06-21 15:02               ` Ludovic Courtès
  1 sibling, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-21 11:17 UTC (permalink / raw)
  To: ludo; +Cc: guile-devel

Ping!  Can the changes below please be committed?  Should I do that
myself?

TIA

> Date: Sun, 15 Jun 2014 20:23:17 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: guile-devel@gnu.org
> 
> Here are the changes I needed for i18n.c to get the tests to succeed.
> They have to do with non-portable assumptions about when the various
> nl_langinfo constants are defined.  E.g., there's no reason to assume
> that if INT_FRAC_DIGITS isn't defined, neither will be FRAC_DIGITS.
> As luck would have it, MinGW has some of these, but not the others, so
> the conditionals failed, and Guile failed to convert P_SIGN_POSN to
> one of the symbolic values, instead leaving it at its numerical value.
> 
> --- libguile/i18n.c~2	2014-06-15 14:21:53 +0300
> +++ libguile/i18n.c	2014-06-15 14:58:09 +0300
> @@ -1583,9 +1583,13 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
>  	  }
>  #endif
>  
> -#if (defined FRAC_DIGITS) && (defined INT_FRAC_DIGITS)
> +#if defined FRAC_DIGITS || defined INT_FRAC_DIGITS
> +#ifdef FRAC_DIGITS
>  	case FRAC_DIGITS:
> +#endif
> +#ifdef INT_FRAC_DIGITS
>  	case INT_FRAC_DIGITS:
> +#endif
>  	  /* This is to be interpreted as a single integer.  */
>  	  if (*c_result == CHAR_MAX)
>  	    /* Unspecified.  */
> @@ -1597,12 +1601,18 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
>  	  break;
>  #endif
>  
> -#if (defined P_CS_PRECEDES) && (defined INT_N_CS_PRECEDES)
> +#if defined P_CS_PRECEDES || defined N_CS_PRECEDES ||	\
> +  defined INT_P_CS_PRECEDES || defined INT_N_CS_PRECEDES || \
> +  defined P_SEP_BY_SPACE || defined N_SEP_BY_SPACE
> +#ifdef P_CS_PRECEDES
>  	case P_CS_PRECEDES:
>  	case N_CS_PRECEDES:
> +#endif
> +#ifdef INT_N_CS_PRECEDES
>  	case INT_P_CS_PRECEDES:
>  	case INT_N_CS_PRECEDES:
> -#if (defined P_SEP_BY_SPACE) && (defined N_SEP_BY_SPACE)
> +#endif
> +#ifdef P_SEP_BY_SPACE
>  	case P_SEP_BY_SPACE:
>  	case N_SEP_BY_SPACE:
>  #endif
> @@ -1613,11 +1623,16 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
>  	  break;
>  #endif
>  
> -#if (defined P_SIGN_POSN) && (defined INT_N_SIGN_POSN)
> +#if defined P_SIGN_POSN || defined N_SIGN_POSN || \
> +  defined INT_P_SIGN_POSN || defined INT_N_SIGN_POSN
> +#ifdef P_SIGN_POSN
>  	case P_SIGN_POSN:
>  	case N_SIGN_POSN:
> +#endif
> +#ifdef INT_P_SIGN_POSN
>  	case INT_P_SIGN_POSN:
>  	case INT_N_SIGN_POSN:
> +#endif
>  	  /* See `(libc) Sign of Money Amount' for the interpretation of the
>  	     return value here.  */
>  	  switch (*c_result)
> 
> 



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-15 17:23             ` Eli Zaretskii
  2014-06-21 11:17               ` Eli Zaretskii
@ 2014-06-21 15:02               ` Ludovic Courtès
  2014-06-21 15:11                 ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-21 15:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Hi Eli,

(Sorry for the delay, I switched contexts, and I’m not using fair
scheduling I suppose.  ;-))

Eli Zaretskii <eliz@gnu.org> skribis:

> Here are the changes I needed for i18n.c to get the tests to succeed.
> They have to do with non-portable assumptions about when the various
> nl_langinfo constants are defined.  E.g., there's no reason to assume
> that if INT_FRAC_DIGITS isn't defined, neither will be FRAC_DIGITS.
> As luck would have it, MinGW has some of these, but not the others, so
> the conditionals failed, and Guile failed to convert P_SIGN_POSN to
> one of the symbolic values, instead leaving it at its numerical value.
>
> --- libguile/i18n.c~2	2014-06-15 14:21:53 +0300
> +++ libguile/i18n.c	2014-06-15 14:58:09 +0300
> @@ -1583,9 +1583,13 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
>  	  }
>  #endif
>  
> -#if (defined FRAC_DIGITS) && (defined INT_FRAC_DIGITS)
> +#if defined FRAC_DIGITS || defined INT_FRAC_DIGITS
> +#ifdef FRAC_DIGITS
>  	case FRAC_DIGITS:
> +#endif
> +#ifdef INT_FRAC_DIGITS
>  	case INT_FRAC_DIGITS:
> +#endif
>  	  /* This is to be interpreted as a single integer.  */
>  	  if (*c_result == CHAR_MAX)
>  	    /* Unspecified.  */
> @@ -1597,12 +1601,18 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
>  	  break;
>  #endif
>  
> -#if (defined P_CS_PRECEDES) && (defined INT_N_CS_PRECEDES)
> +#if defined P_CS_PRECEDES || defined N_CS_PRECEDES ||	\
> +  defined INT_P_CS_PRECEDES || defined INT_N_CS_PRECEDES || \
> +  defined P_SEP_BY_SPACE || defined N_SEP_BY_SPACE
> +#ifdef P_CS_PRECEDES
>  	case P_CS_PRECEDES:
>  	case N_CS_PRECEDES:
> +#endif
> +#ifdef INT_N_CS_PRECEDES
>  	case INT_P_CS_PRECEDES:
>  	case INT_N_CS_PRECEDES:
> -#if (defined P_SEP_BY_SPACE) && (defined N_SEP_BY_SPACE)
> +#endif
> +#ifdef P_SEP_BY_SPACE
>  	case P_SEP_BY_SPACE:
>  	case N_SEP_BY_SPACE:
>  #endif
> @@ -1613,11 +1623,16 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
>  	  break;
>  #endif
>  
> -#if (defined P_SIGN_POSN) && (defined INT_N_SIGN_POSN)
> +#if defined P_SIGN_POSN || defined N_SIGN_POSN || \
> +  defined INT_P_SIGN_POSN || defined INT_N_SIGN_POSN
> +#ifdef P_SIGN_POSN
>  	case P_SIGN_POSN:
>  	case N_SIGN_POSN:
> +#endif
> +#ifdef INT_P_SIGN_POSN
>  	case INT_P_SIGN_POSN:
>  	case INT_N_SIGN_POSN:
> +#endif
>  	  /* See `(libc) Sign of Money Amount' for the interpretation of the
>  	     return value here.  */
>  	  switch (*c_result)

This change looks OK to me.  Can you commit it yourself?

Note that we don’t have ChangeLog files.  Instead, we write
ChangeLog-style commit logs.  Please see previous commit logs to get an
idea.

Ludo’.

PS: I still have a bunch of messages from you marked as to be processed.
    I’ll try to get back to it ASAP.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-21 15:02               ` Ludovic Courtès
@ 2014-06-21 15:11                 ` Eli Zaretskii
  2014-06-21 21:23                   ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-21 15:11 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Sat, 21 Jun 2014 17:02:30 +0200
> 
> Hi Eli,
> 
> (Sorry for the delay, I switched contexts, and I’m not using fair
> scheduling I suppose.  ;-))

No sweat.

> > --- libguile/i18n.c~2	2014-06-15 14:21:53 +0300
> > +++ libguile/i18n.c	2014-06-15 14:58:09 +0300
> > @@ -1583,9 +1583,13 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
> >  	  }
> >  #endif
> >  
> > -#if (defined FRAC_DIGITS) && (defined INT_FRAC_DIGITS)
> > +#if defined FRAC_DIGITS || defined INT_FRAC_DIGITS
> > +#ifdef FRAC_DIGITS
> >  	case FRAC_DIGITS:
> > +#endif
> > +#ifdef INT_FRAC_DIGITS
> >  	case INT_FRAC_DIGITS:
> > +#endif
> >  	  /* This is to be interpreted as a single integer.  */
> >  	  if (*c_result == CHAR_MAX)
> >  	    /* Unspecified.  */
> > @@ -1597,12 +1601,18 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
> >  	  break;
> >  #endif
> >  
> > -#if (defined P_CS_PRECEDES) && (defined INT_N_CS_PRECEDES)
> > +#if defined P_CS_PRECEDES || defined N_CS_PRECEDES ||	\
> > +  defined INT_P_CS_PRECEDES || defined INT_N_CS_PRECEDES || \
> > +  defined P_SEP_BY_SPACE || defined N_SEP_BY_SPACE
> > +#ifdef P_CS_PRECEDES
> >  	case P_CS_PRECEDES:
> >  	case N_CS_PRECEDES:
> > +#endif
> > +#ifdef INT_N_CS_PRECEDES
> >  	case INT_P_CS_PRECEDES:
> >  	case INT_N_CS_PRECEDES:
> > -#if (defined P_SEP_BY_SPACE) && (defined N_SEP_BY_SPACE)
> > +#endif
> > +#ifdef P_SEP_BY_SPACE
> >  	case P_SEP_BY_SPACE:
> >  	case N_SEP_BY_SPACE:
> >  #endif
> > @@ -1613,11 +1623,16 @@ SCM_DEFINE (scm_nl_langinfo, "nl-langinf
> >  	  break;
> >  #endif
> >  
> > -#if (defined P_SIGN_POSN) && (defined INT_N_SIGN_POSN)
> > +#if defined P_SIGN_POSN || defined N_SIGN_POSN || \
> > +  defined INT_P_SIGN_POSN || defined INT_N_SIGN_POSN
> > +#ifdef P_SIGN_POSN
> >  	case P_SIGN_POSN:
> >  	case N_SIGN_POSN:
> > +#endif
> > +#ifdef INT_P_SIGN_POSN
> >  	case INT_P_SIGN_POSN:
> >  	case INT_N_SIGN_POSN:
> > +#endif
> >  	  /* See `(libc) Sign of Money Amount' for the interpretation of the
> >  	     return value here.  */
> >  	  switch (*c_result)
> 
> This change looks OK to me.  Can you commit it yourself?

Yes.  Just to be sure: I should commit it to the stable-2.0 branch,
right?

> Note that we don’t have ChangeLog files.  Instead, we write
> ChangeLog-style commit logs.  Please see previous commit logs to get an
> idea.

Will do.

> PS: I still have a bunch of messages from you marked as to be processed.
>     I’ll try to get back to it ASAP.

Thanks.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-21 15:11                 ` Eli Zaretskii
@ 2014-06-21 21:23                   ` Ludovic Courtès
  2014-06-22 16:13                     ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-21 21:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

[...]

>> This change looks OK to me.  Can you commit it yourself?
>
> Yes.  Just to be sure: I should commit it to the stable-2.0 branch,
> right?

Yes, please.  It’ll be merged to master eventually.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: MinGW vs. setlocale
  2014-06-21 21:23                   ` Ludovic Courtès
@ 2014-06-22 16:13                     ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-22 16:13 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Sat, 21 Jun 2014 23:23:33 +0200
> 
> Eli Zaretskii <eliz@gnu.org> skribis:
> 
> [...]
> 
> >> This change looks OK to me.  Can you commit it yourself?
> >
> > Yes.  Just to be sure: I should commit it to the stable-2.0 branch,
> > right?
> 
> Yes, please.  It’ll be merged to master eventually.

Done.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-06-10 16:00   ` Eli Zaretskii
@ 2014-06-30 11:15     ` Ludovic Courtès
  2014-06-30 14:56       ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-06-30 11:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Date: Mon, 09 Jun 2014 21:42:36 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> skribis:
>> 
>> > 3. load.test fails:
>> >
>> >      FAIL: load.test: search-path for "foo.scm" yields "dir1/foo.scm"
>> >
>> >    (The messages are misleading: "yields" should be "should yield".)
>> >
>> >    The test fails because:
>> >
>> >     . it compares file names with equal?, which fails when Windows
>> >       file names with mixed forward and backslashes are used, and/or
>> >       when the files differ but for letter case
>> >
>> >     . the expected result uses a relative file name of temp-dir, while
>> >       search-path returns absolute file names
>> >
>> >    Fixed by using "/" to create a file name from its parts in load.c:
>> >
>> > --- libguile/load.c~	2014-02-28 23:01:27 +0200
>> > +++ libguile/load.c	2014-06-08 13:27:24 +0300
>> > @@ -452,11 +452,15 @@ scm_c_string_has_an_ext (char *str, size
>> >    return 0;
>> >  }
>> >  
>> > +#if 0
>> >  #ifdef __MINGW32__
>> >  #define FILE_NAME_SEPARATOR_STRING "\\"
>> >  #else
>> >  #define FILE_NAME_SEPARATOR_STRING "/"
>> >  #endif
>> > +#else
>> > +#define FILE_NAME_SEPARATOR_STRING "/"
>> > +#endif
>> >  
>> >  static int
>> >  is_file_name_separator (SCM c)
>> >
>> >    I don't see any reasons to use the backslashes when constructing
>> >    Windows file names.  Unless someone can tell why using backslashes
>> >    is a good idea, I propose to remove the Windows setting of
>> >    FILE_NAME_SEPARATOR_STRING.
>> 
>> I’m confused: this was added in 4bab7f01 precisely to support MinGW or
>> Windows.  Similarly, boot-9.scm has ‘file-name-separator-string’ and
>> related stuff.  This was the result of the discussion at
>> <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10474#89>.
>
> Sorry, that's my fault: I didn't explain the problem in enough detail.
>
> There's nothing wrong with the 4bab7f01 commit per se (and you will
> see that its only part that I changed is the definition of
> FILE_NAME_SEPARATOR_STRING for MinGW).  The problem is not in that
> commit, it is elsewhere: in Scheme code, in this case in the test
> suite, that compares file names as simple strings.  Such comparisons
> fail if the file names differ by the style of directory separators:
> one uses forward slashes, the other backslashes, or some mix thereof.
>
> Now, FILE_NAME_SEPARATOR_STRING is used only for constructing file
> names from their parts.  It is not used for testing a particular
> file-name character for being a directory separator.  Therefore, we
> can discard the separate definition of FILE_NAME_SEPARATOR_STRING for
> Windows, and use "/" on all platforms.  This makes the problem of
> comparing file names easier, and in particular lets Guile pass
> load.test.  But it doesn't solve the problem entirely.

OK.  This and your other message clarify things, thanks.

> To solve this problem completely, we need a function that
> canonicalizes a file name wrt directory separators -- converts all
> backslashes to forward slashes.  Does Guile have such a function?

My understanding of your other message is that you now advocate trying
hard to stick to slashes instead of backslashes, in which case a file
name canonicalization function is practically unnecessary, right?

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-06-30 11:15     ` Ludovic Courtès
@ 2014-06-30 14:56       ` Eli Zaretskii
  2014-07-01  9:36         ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-06-30 14:56 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Mon, 30 Jun 2014 13:15:44 +0200
> 
> > To solve this problem completely, we need a function that
> > canonicalizes a file name wrt directory separators -- converts all
> > backslashes to forward slashes.  Does Guile have such a function?
> 
> My understanding of your other message is that you now advocate trying
> hard to stick to slashes instead of backslashes, in which case a file
> name canonicalization function is practically unnecessary, right?

Yes, I think so -- provided that I indeed succeeded in finding all the
places where file names and path lists are created from directories
returned by C APIs.  You are in a better position to judge that, as I
still have only a very vague idea about which part of Guile does what,
both in C and in Scheme.

In Emacs, some of the file and directory names recorded during the
build and startup come from argv[0] and from prefix-relative directory
names computed by configure.  Is there something similar in Guile, and
if so, where do I find that?




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-06-30 14:56       ` Eli Zaretskii
@ 2014-07-01  9:36         ` Ludovic Courtès
  2014-07-01 15:30           ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-07-01  9:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

> In Emacs, some of the file and directory names recorded during the
> build and startup come from argv[0] and from prefix-relative directory
> names computed by configure.  Is there something similar in Guile, and
> if so, where do I find that?

The default %load-path uses absolute directory names based on what
./configure computed.  argv[0] isn’t used to derive file names.

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-01  9:36         ` Ludovic Courtès
@ 2014-07-01 15:30           ` Eli Zaretskii
  2014-07-01 15:38             ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-01 15:30 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Tue, 01 Jul 2014 11:36:32 +0200
> 
> Eli Zaretskii <eliz@gnu.org> skribis:
> 
> > In Emacs, some of the file and directory names recorded during the
> > build and startup come from argv[0] and from prefix-relative directory
> > names computed by configure.  Is there something similar in Guile, and
> > if so, where do I find that?
> 
> The default %load-path uses absolute directory names based on what
> ./configure computed.

Thanks.  Where do I find the code which does that?  I'd like to review
it with the issue at hand in mind.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-01 15:30           ` Eli Zaretskii
@ 2014-07-01 15:38             ` Ludovic Courtès
  2014-07-02 16:04               ` Eli Zaretskii
                                 ` (3 more replies)
  0 siblings, 4 replies; 67+ messages in thread
From: Ludovic Courtès @ 2014-07-01 15:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel@gnu.org
>> Date: Tue, 01 Jul 2014 11:36:32 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> skribis:
>> 
>> > In Emacs, some of the file and directory names recorded during the
>> > build and startup come from argv[0] and from prefix-relative directory
>> > names computed by configure.  Is there something similar in Guile, and
>> > if so, where do I find that?
>> 
>> The default %load-path uses absolute directory names based on what
>> ./configure computed.
>
> Thanks.  Where do I find the code which does that?  I'd like to review
> it with the issue at hand in mind.

You can look at load.c, and in particular scm_init_load_path.

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-01 15:38             ` Ludovic Courtès
@ 2014-07-02 16:04               ` Eli Zaretskii
  2014-07-02 20:56                 ` Ludovic Courtès
  2014-07-02 20:57                 ` Ludovic Courtès
  2014-07-02 16:13               ` Fix 'dirname' and 'basename' on MS-Windows Eli Zaretskii
                                 ` (2 subsequent siblings)
  3 siblings, 2 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-02 16:04 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Tue, 01 Jul 2014 17:38:04 +0200
> 
> Eli Zaretskii <eliz@gnu.org> skribis:
> 
> >> From: ludo@gnu.org (Ludovic Courtès)
> >> Cc: guile-devel@gnu.org
> >> Date: Tue, 01 Jul 2014 11:36:32 +0200
> >> 
> >> Eli Zaretskii <eliz@gnu.org> skribis:
> >> 
> >> > In Emacs, some of the file and directory names recorded during the
> >> > build and startup come from argv[0] and from prefix-relative directory
> >> > names computed by configure.  Is there something similar in Guile, and
> >> > if so, where do I find that?
> >> 
> >> The default %load-path uses absolute directory names based on what
> >> ./configure computed.
> >
> > Thanks.  Where do I find the code which does that?  I'd like to review
> > it with the issue at hand in mind.
> 
> You can look at load.c, and in particular scm_init_load_path.

OK, thanks for the pointer.

I've reviewed the related code, and below is what I suggest to push.
(This supersedes what I sent in
http://lists.gnu.org/archive/html/guile-devel/2014-06/msg00066.html.)

--- libguile/load.c~0	2014-02-28 23:01:27 +0200
+++ libguile/load.c	2014-07-02 19:00:50 +0300
@@ -277,6 +277,37 @@ SCM_DEFINE (scm_parse_path_with_ellipsis
 }
 #undef FUNC_NAME
 
+static char *
+getenv_path (const char *name)
+{
+  char *val = getenv (name);
+
+#ifdef __MINGW32__
+  if (val)
+    {
+      char *p = val;
+
+      /* Replace backslashes with forward slashes, so that Scheme code
+	 always gets d:/foo/bar style file names.  This avoids
+	 multiple subtle problems with comparing file names as strings
+	 and with redirections in /bin/sh command lines.  Note that
+	 this destructively modifies the environment variables, so
+	 both scm_getenv and subprocesses will see the values with
+	 forward slashes.  But that is OK, since these variables are
+	 Guile-specific, and having scm_getenv return the same value
+	 as used by the callers of getenv_path is good for
+	 consistency and file-name comparison.  */
+      while (*p)
+	{
+	  if (*p == '\\')
+	    *p = '/';
+	  p++;
+	}
+    }
+#endif
+
+  return val;
+}
 
 /* Initialize the global variable %load-path, given the value of the
    SCM_SITE_DIR and SCM_LIBRARY_DIR preprocessor symbols and the
@@ -289,7 +320,7 @@ scm_init_load_path ()
   SCM cpath = SCM_EOL;
 
 #ifdef SCM_LIBRARY_DIR
-  env = getenv ("GUILE_SYSTEM_PATH");
+  env = getenv_path ("GUILE_SYSTEM_PATH");
   if (env && strcmp (env, "") == 0)
     /* special-case interpret system-path=="" as meaning no system path instead
        of '("") */
@@ -302,7 +333,7 @@ scm_init_load_path ()
                        scm_from_locale_string (SCM_GLOBAL_SITE_DIR),
                        scm_from_locale_string (SCM_PKGDATA_DIR));
 
-  env = getenv ("GUILE_SYSTEM_COMPILED_PATH");
+  env = getenv_path ("GUILE_SYSTEM_COMPILED_PATH");
   if (env && strcmp (env, "") == 0)
     /* like above */
     ; 
@@ -345,14 +376,30 @@ scm_init_load_path ()
       cachedir[0] = 0;
 
     if (cachedir[0])
-      *scm_loc_compile_fallback_path = scm_from_locale_string (cachedir);
+      {
+#ifdef __MINGW32__
+	/* We don't use getenv_path for FALLBACK_DIR because those
+	   variables are not Guile-specific, so we want to leave them
+	   intact in the environment.  This is especially relevant for
+	   LOCALAPPDATA and APPDATA.  */
+	char *p = cachedir;
+
+	while (*p)
+	  {
+	    if (*p == '\\')
+	      *p = '/';
+	    p++;
+	  }
+#endif
+	*scm_loc_compile_fallback_path = scm_from_locale_string (cachedir);
+      }
   }
 
-  env = getenv ("GUILE_LOAD_PATH");
+  env = getenv_path ("GUILE_LOAD_PATH");
   if (env)
     path = scm_parse_path_with_ellipsis (scm_from_locale_string (env), path);
 
-  env = getenv ("GUILE_LOAD_COMPILED_PATH");
+  env = getenv_path ("GUILE_LOAD_COMPILED_PATH");
   if (env)
     cpath = scm_parse_path_with_ellipsis (scm_from_locale_string (env), cpath);
 
@@ -452,11 +499,10 @@ scm_c_string_has_an_ext (char *str, size
   return 0;
 }
 
-#ifdef __MINGW32__
-#define FILE_NAME_SEPARATOR_STRING "\\"
-#else
+/* Defined as "/" for Unix and Windows alike, so that file names
+   constructed by the functions in this module wind up with Unix-style
+   forward slashes as directory separators.  */
 #define FILE_NAME_SEPARATOR_STRING "/"
-#endif
 
 static int
 is_file_name_separator (SCM c)
@@ -877,7 +923,7 @@ canonical_suffix (SCM fname)
 
   /* CANON should be absolute.  */
   canon = scm_canonicalize_path (fname);
-  
+
 #ifdef __MINGW32__
   {
     size_t len = scm_c_string_length (canon);



--- libguile/filesys.c~0	2014-02-28 23:01:27 +0200
+++ libguile/filesys.c	2014-06-29 18:13:30 +0300
@@ -1235,6 +1235,19 @@ SCM_DEFINE (scm_getcwd, "getcwd", 0, 0,
       errno = save_errno;
       SCM_SYSERROR;
     }
+#ifdef __MINGW32__
+  if (rv)
+    {
+      char *p = wd;
+
+      while (*p)
+	{
+	  if (*p == '\\')
+	    *p = '/';
+	  p++;
+	}
+    }
+#endif
   result = scm_from_locale_stringn (wd, strlen (wd));
   free (wd);
   return result;


--- module/ice-9/boot-9.scm~	2014-02-15 01:00:33 +0200
+++ module/ice-9/boot-9.scm	2014-06-29 18:15:07 +0300
@@ -1657,7 +1657,7 @@
        (or (char=? c #\/)
            (char=? c #\\)))
 
-     (define file-name-separator-string "\\")
+     (define file-name-separator-string "/")
 
      (define (absolute-file-name? file-name)
        (define (file-name-separator-at-index? idx)



--- libguile/init.c~0	2014-02-28 23:01:27 +0200
+++ libguile/init.c	2014-07-02 18:51:04 +0300
@@ -310,6 +310,17 @@ scm_boot_guile (int argc, char ** argv,
 {
   void *res;
   struct main_func_closure c;
+#ifdef __MINGW32__
+  /* Convert backslashes in argv[0] to forward slashes.  */
+  char *p = argv[0];
+
+  while (*p)
+    {
+      if (*p == '\\')
+	*p = '/';
+      p++;
+    }
+#endif
 
   c.main_func = main_func;
   c.closure = closure;





^ permalink raw reply	[flat|nested] 67+ messages in thread

* Fix 'dirname' and 'basename' on MS-Windows
  2014-07-01 15:38             ` Ludovic Courtès
  2014-07-02 16:04               ` Eli Zaretskii
@ 2014-07-02 16:13               ` Eli Zaretskii
  2014-07-09 14:22                 ` Ludovic Courtès
  2014-07-02 16:16               ` Provide reasonable stack limit " Eli Zaretskii
  2014-07-02 16:30               ` Bug in scm_getaffinity Eli Zaretskii
  3 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-02 16:13 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

These 2 functions don't deal correctly with Windows file names with
drive letters and with UNCs.  The patch below fixes that.

Incidentally, isn't the line in scm_basename marked below wrong?

  if (i == end)
    {
      if (len > 0 && is_file_name_separator (scm_c_string_ref (filename, 0)))
        return scm_c_substring (filename, 0, 1);
      else
	return scm_dot_string;  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
    }
  else
    return scm_c_substring (filename, i+1, end+1);

It is responsible for the following strange results:

   (basename ".foo" ".foo")  => "."
   (basename "_foo" "_foo")  => "."

Also, isn't the following result wrong as well?

   (basename "/")  => "/"

I think all of these should return the empty string, "".

Here's the proposed patch for supporting Windows file names.

--- libguile/filesys.c~1	2014-06-29 16:13:30 +0300
+++ libguile/filesys.c	2014-07-02 14:03:08 +0300
@@ -448,6 +448,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"
@@ -1518,24 +1530,60 @@ SCM_DEFINE (scm_dirname, "dirname", 1, 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;
     }
@@ -1553,6 +1601,9 @@ SCM_DEFINE (scm_basename, "basename", 1,
 #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 prefix_len = 0;
 
   SCM_VALIDATE_STRING (1, filename);
   len = scm_i_string_length (filename);
@@ -1564,11 +1615,17 @@ SCM_DEFINE (scm_basename, "basename", 1,
       SCM_VALIDATE_STRING (2, suffix);
       j = scm_i_string_length (suffix) - 1;
     }
+  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 = 2;
+
   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;
   end = i;
-  while (i >= 0 && j >= 0 
+  while (i >= prefix_len && j >= 0
 	 && (scm_i_string_ref (filename, i)
 	     == scm_i_string_ref (suffix, j)))
     {
@@ -1577,12 +1634,20 @@ SCM_DEFINE (scm_basename, "basename", 1,
     }
   if (j == -1)
     end = 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 == end)
     {
-      if (len > 0 && is_file_name_separator (scm_c_string_ref (filename, 0)))
-        return scm_c_substring (filename, 0, 1);
+      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;
     }



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Provide reasonable stack limit on MS-Windows
  2014-07-01 15:38             ` Ludovic Courtès
  2014-07-02 16:04               ` Eli Zaretskii
  2014-07-02 16:13               ` Fix 'dirname' and 'basename' on MS-Windows Eli Zaretskii
@ 2014-07-02 16:16               ` Eli Zaretskii
  2014-07-02 21:02                 ` Ludovic Courtès
  2014-07-02 16:30               ` Bug in scm_getaffinity Eli Zaretskii
  3 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-02 16:16 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

The default value used by debug.c when getrlimit is not available is
too small (the default stack size of MinGW programs is 2MB).  This
small patch fixes that:

--- libguile/debug.c~0	2014-02-15 01:00:33 +0200
+++ libguile/debug.c	2014-07-02 17:40:51 +0300
@@ -27,6 +27,11 @@
 #include <sys/resource.h>
 #endif
 
+#ifdef __MINGW32__
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>
+#endif
+
 #include "libguile/_scm.h"
 #include "libguile/async.h"
 #include "libguile/eval.h"
@@ -228,7 +233,7 @@ scm_local_eval (SCM exp, SCM env)
 static void
 init_stack_limit (void)
 {
-#ifdef HAVE_GETRLIMIT
+#if defined HAVE_GETRLIMIT
   struct rlimit lim;
   if (getrlimit (RLIMIT_STACK, &lim) == 0)
       {
@@ -242,6 +247,16 @@ init_stack_limit (void)
           SCM_STACK_LIMIT = bytes * 8 / 10 / sizeof (scm_t_bits);
       }
   errno = 0;
+#elif defined __MINGW32__
+  MEMORY_BASIC_INFORMATION m;
+  uintptr_t bytes;
+
+  if (VirtualQuery ((LPCVOID) &m, &m, sizeof m))
+    {
+      bytes = (DWORD_PTR) m.BaseAddress + m.RegionSize
+	       - (DWORD_PTR) m.AllocationBase;
+      SCM_STACK_LIMIT = bytes * 8 / 10 / sizeof (scm_t_bits);
+    }
 #endif
 }
 




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Bug in scm_getaffinity
  2014-07-01 15:38             ` Ludovic Courtès
                                 ` (2 preceding siblings ...)
  2014-07-02 16:16               ` Provide reasonable stack limit " Eli Zaretskii
@ 2014-07-02 16:30               ` Eli Zaretskii
  2014-07-02 21:04                 ` Ludovic Courtès
  3 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-02 16:30 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

While working on the MS-Windows emulation of this function, I bumped
into something that looks like a bug in its subroutine:

  static SCM
  cpu_set_to_bitvector (const cpu_set_t *cs)
  {
    SCM bv;
    size_t cpu;

    bv = scm_c_make_bitvector (sizeof (*cs), SCM_BOOL_F);

    for (cpu = 0; cpu < sizeof (*cs); cpu++)
      {
	if (CPU_ISSET (cpu, cs))
	  /* XXX: This is inefficient but avoids code duplication.  */
	  scm_c_bitvector_set_x (bv, cpu, SCM_BOOL_T);
      }

I think using 'sizeof (*cs)' is incorrect here, we need to use
CPU_SETSIZE instead.  The cpu_set_t data type could be an array of bit
masks, in which case counting only bytes in it is wrong: the result is
too small.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-02 16:04               ` Eli Zaretskii
@ 2014-07-02 20:56                 ` Ludovic Courtès
  2014-07-02 20:57                 ` Ludovic Courtès
  1 sibling, 0 replies; 67+ messages in thread
From: Ludovic Courtès @ 2014-07-02 20:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel@gnu.org
>> Date: Tue, 01 Jul 2014 17:38:04 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> skribis:
>> 
>> >> From: ludo@gnu.org (Ludovic Courtès)
>> >> Cc: guile-devel@gnu.org
>> >> Date: Tue, 01 Jul 2014 11:36:32 +0200
>> >> 
>> >> Eli Zaretskii <eliz@gnu.org> skribis:
>> >> 
>> >> > In Emacs, some of the file and directory names recorded during the
>> >> > build and startup come from argv[0] and from prefix-relative directory
>> >> > names computed by configure.  Is there something similar in Guile, and
>> >> > if so, where do I find that?
>> >> 
>> >> The default %load-path uses absolute directory names based on what
>> >> ./configure computed.
>> >
>> > Thanks.  Where do I find the code which does that?  I'd like to review
>> > it with the issue at hand in mind.
>> 
>> You can look at load.c, and in particular scm_init_load_path.
>
> OK, thanks for the pointer.

Looks good.

> I've reviewed the related code, and below is what I suggest to push.
> (This supersedes what I sent in
> http://lists.gnu.org/archive/html/guile-devel/2014-06/msg00066.html.)

Overall looks good.  Some comments:

> --- libguile/load.c~0	2014-02-28 23:01:27 +0200
> +++ libguile/load.c	2014-07-02 19:00:50 +0300
> @@ -277,6 +277,37 @@ SCM_DEFINE (scm_parse_path_with_ellipsis
>  }
>  #undef FUNC_NAME
>  
> +static char *
> +getenv_path (const char *name)

What about making it ‘canonicalize_file_name_separators’, and change
callers to do:

  thing = canonicalize_file_name_separators (getenv ("PATH"))

Also please add a “docstring” above the function.

[...]

> --- libguile/filesys.c~0	2014-02-28 23:01:27 +0200
> +++ libguile/filesys.c	2014-06-29 18:13:30 +0300
> @@ -1235,6 +1235,19 @@ SCM_DEFINE (scm_getcwd, "getcwd", 0, 0,
>        errno = save_errno;
>        SCM_SYSERROR;
>      }
> +#ifdef __MINGW32__
> +  if (rv)
> +    {
> +      char *p = wd;
> +
> +      while (*p)
> +	{
> +	  if (*p == '\\')
> +	    *p = '/';
> +	  p++;
> +	}
> +    }
> +#endif

Maybe this could be replaced by an unconditional call to
canonicalize_file_name_separators?

In that case, that function actually needs to be called
scm_i_canonicalize_file_name_separators and declared SCM_INTERNAL.

> --- libguile/init.c~0	2014-02-28 23:01:27 +0200
> +++ libguile/init.c	2014-07-02 18:51:04 +0300
> @@ -310,6 +310,17 @@ scm_boot_guile (int argc, char ** argv,
>  {
>    void *res;
>    struct main_func_closure c;
> +#ifdef __MINGW32__
> +  /* Convert backslashes in argv[0] to forward slashes.  */
> +  char *p = argv[0];
> +
> +  while (*p)
> +    {
> +      if (*p == '\\')
> +	*p = '/';
> +      p++;
> +    }
> +#endif

Ditto.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-02 16:04               ` Eli Zaretskii
  2014-07-02 20:56                 ` Ludovic Courtès
@ 2014-07-02 20:57                 ` Ludovic Courtès
  2014-07-03 15:10                   ` Eli Zaretskii
  1 sibling, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-07-02 20:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel@gnu.org
>> Date: Tue, 01 Jul 2014 17:38:04 +0200

[...]

>> You can look at load.c, and in particular scm_init_load_path.
>
> OK, thanks for the pointer.
>
> I've reviewed the related code, and below is what I suggest to push.
> (This supersedes what I sent in
> http://lists.gnu.org/archive/html/guile-devel/2014-06/msg00066.html.)

Also, could you add tests for that?  Namely, a ‘search-path’ use that
currently returns a file name with backslashes, and will now return a
file name with forward slashes.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Provide reasonable stack limit on MS-Windows
  2014-07-02 16:16               ` Provide reasonable stack limit " Eli Zaretskii
@ 2014-07-02 21:02                 ` Ludovic Courtès
  2014-07-03 16:28                   ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-07-02 21:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

> The default value used by debug.c when getrlimit is not available is
> too small (the default stack size of MinGW programs is 2MB).  This
> small patch fixes that:

Looks good to me, please push.

But it’s really problematic that we’re accumulating all this while a
bunch of Gnulib modules would have done it; more work initially, but
then more benefits.  (Did I say that already?  ;-))

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Bug in scm_getaffinity
  2014-07-02 16:30               ` Bug in scm_getaffinity Eli Zaretskii
@ 2014-07-02 21:04                 ` Ludovic Courtès
  2014-07-03 16:31                   ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-07-02 21:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

> While working on the MS-Windows emulation of this function, I bumped
> into something that looks like a bug in its subroutine:
>
>   static SCM
>   cpu_set_to_bitvector (const cpu_set_t *cs)
>   {
>     SCM bv;
>     size_t cpu;
>
>     bv = scm_c_make_bitvector (sizeof (*cs), SCM_BOOL_F);
>
>     for (cpu = 0; cpu < sizeof (*cs); cpu++)
>       {
> 	if (CPU_ISSET (cpu, cs))
> 	  /* XXX: This is inefficient but avoids code duplication.  */
> 	  scm_c_bitvector_set_x (bv, cpu, SCM_BOOL_T);
>       }
>
> I think using 'sizeof (*cs)' is incorrect here, we need to use
> CPU_SETSIZE instead.  The cpu_set_t data type could be an array of bit
> masks, in which case counting only bytes in it is wrong: the result is
> too small.

Indeed, good catch.  Could you commit the obvious fix?

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-02 20:57                 ` Ludovic Courtès
@ 2014-07-03 15:10                   ` Eli Zaretskii
  2014-07-03 17:11                     ` Ludovic Courtès
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-03 15:10 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Wed, 02 Jul 2014 22:57:41 +0200
> 
> Eli Zaretskii <eliz@gnu.org> skribis:
> 
> >> From: ludo@gnu.org (Ludovic Courtès)
> >> Cc: guile-devel@gnu.org
> >> Date: Tue, 01 Jul 2014 17:38:04 +0200
> 
> [...]
> 
> >> You can look at load.c, and in particular scm_init_load_path.
> >
> > OK, thanks for the pointer.
> >
> > I've reviewed the related code, and below is what I suggest to push.
> > (This supersedes what I sent in
> > http://lists.gnu.org/archive/html/guile-devel/2014-06/msg00066.html.)
> 
> Also, could you add tests for that?  Namely, a ‘search-path’ use that
> currently returns a file name with backslashes, and will now return a
> file name with forward slashes.

Is the below OK?

--- libguile/load.c~0	2014-02-28 23:01:27 +0200
+++ libguile/load.c	2014-07-03 09:58:29 +0300
@@ -277,6 +277,41 @@ SCM_DEFINE (scm_parse_path_with_ellipsis
 }
 #undef FUNC_NAME
 
+/* On Posix hosts, just return PATH unaltered.  On Windows,
+   destructively replace all backslashes in PATH with Unix-style
+   forward slashes, so that Scheme code always gets d:/foo/bar style
+   file names.  This avoids multiple subtle problems with comparing
+   file names as strings, and with redirections in /bin/sh command
+   lines.
+
+   Note that, if PATH is result of a call to 'getenv', this
+   destructively modifies the environment variables, so both
+   scm_getenv and subprocesses will afterwards see the values with
+   forward slashes.  That is OK as long as applied to Guile-specific
+   environment variables, since having scm_getenv return the same
+   value as used by the callers of this function is good for
+   consistency and file-name comparison.  Avoid using this function on
+   values returned by 'getenv' for general-purpose environment
+   variables; instead, make a copy of the value and work on that.  */
+SCM_INTERNAL char *
+scm_i_mirror_backslashes (char *path)
+{
+#ifdef __MINGW32__
+  if (path)
+    {
+      char *p = path;
+
+      while (*p)
+	{
+	  if (*p == '\\')
+	    *p = '/';
+	  p++;
+	}
+    }
+#endif
+
+  return path;
+}
 
 /* Initialize the global variable %load-path, given the value of the
    SCM_SITE_DIR and SCM_LIBRARY_DIR preprocessor symbols and the
@@ -289,7 +324,7 @@ scm_init_load_path ()
   SCM cpath = SCM_EOL;
 
 #ifdef SCM_LIBRARY_DIR
-  env = getenv ("GUILE_SYSTEM_PATH");
+  env = scm_i_mirror_backslashes (getenv ("GUILE_SYSTEM_PATH"));
   if (env && strcmp (env, "") == 0)
     /* special-case interpret system-path=="" as meaning no system path instead
        of '("") */
@@ -302,7 +337,7 @@ scm_init_load_path ()
                        scm_from_locale_string (SCM_GLOBAL_SITE_DIR),
                        scm_from_locale_string (SCM_PKGDATA_DIR));
 
-  env = getenv ("GUILE_SYSTEM_COMPILED_PATH");
+  env = scm_i_mirror_backslashes (getenv ("GUILE_SYSTEM_COMPILED_PATH"));
   if (env && strcmp (env, "") == 0)
     /* like above */
     ; 
@@ -345,14 +380,17 @@ scm_init_load_path ()
       cachedir[0] = 0;
 
     if (cachedir[0])
-      *scm_loc_compile_fallback_path = scm_from_locale_string (cachedir);
+      {
+	scm_i_mirror_backslashes (cachedir);
+	*scm_loc_compile_fallback_path = scm_from_locale_string (cachedir);
+      }
   }
 
-  env = getenv ("GUILE_LOAD_PATH");
+  env = scm_i_mirror_backslashes (getenv ("GUILE_LOAD_PATH"));
   if (env)
     path = scm_parse_path_with_ellipsis (scm_from_locale_string (env), path);
 
-  env = getenv ("GUILE_LOAD_COMPILED_PATH");
+  env = scm_i_mirror_backslashes (getenv ("GUILE_LOAD_COMPILED_PATH"));
   if (env)
     cpath = scm_parse_path_with_ellipsis (scm_from_locale_string (env), cpath);
 
@@ -452,11 +490,10 @@ scm_c_string_has_an_ext (char *str, size
   return 0;
 }
 
-#ifdef __MINGW32__
-#define FILE_NAME_SEPARATOR_STRING "\\"
-#else
+/* Defined as "/" for Unix and Windows alike, so that file names
+   constructed by the functions in this module wind up with Unix-style
+   forward slashes as directory separators.  */
 #define FILE_NAME_SEPARATOR_STRING "/"
-#endif
 
 static int
 is_file_name_separator (SCM c)
@@ -877,7 +914,7 @@ canonical_suffix (SCM fname)
 
   /* CANON should be absolute.  */
   canon = scm_canonicalize_path (fname);
-  
+
 #ifdef __MINGW32__
   {
     size_t len = scm_c_string_length (canon);


--- libguile/load.h~0	2013-03-19 00:30:13 +0200
+++ libguile/load.h	2014-07-03 09:59:17 +0300
@@ -44,6 +44,7 @@ SCM_INTERNAL void scm_init_load_path (vo
 SCM_INTERNAL void scm_init_load (void);
 SCM_INTERNAL void scm_init_load_should_auto_compile (void);
 SCM_INTERNAL void scm_init_eval_in_scheme (void);
+SCM_INTERNAL char *scm_i_mirror_backslashes (char *path);
 
 #endif  /* SCM_LOAD_H */
 


--- libguile/filesys.c~0	2014-02-28 23:01:27 +0200
+++ libguile/filesys.c	2014-07-03 10:03:25 +0300
@@ -51,6 +51,7 @@
 
 #include "libguile/validate.h"
 #include "libguile/filesys.h"
+#include "libguile/load.h"	/* for scm_i_mirror_backslashes */
 
 \f
 #ifdef HAVE_IO_H
@@ -1235,6 +1248,9 @@ SCM_DEFINE (scm_getcwd, "getcwd", 0, 0,
       errno = save_errno;
       SCM_SYSERROR;
     }
+  /* On Windows, convert backslashes in current directory to forward
+     slashes.  */
+  scm_i_mirror_backslashes (wd);
   result = scm_from_locale_stringn (wd, strlen (wd));
   free (wd);
   return result;


--- libguile/init.c~0	2014-02-28 23:01:27 +0200
+++ libguile/init.c	2014-07-03 10:02:03 +0300
@@ -311,6 +311,9 @@ scm_boot_guile (int argc, char ** argv,
   void *res;
   struct main_func_closure c;
 
+  /* On Windows, convert backslashes in argv[0] to forward
+     slashes.  */
+  scm_i_mirror_backslashes (argv[0]);
   c.main_func = main_func;
   c.closure = closure;
   c.argc = argc;


--- module/ice-9/boot-9.scm~	2014-02-15 01:00:33 +0200
+++ module/ice-9/boot-9.scm	2014-06-29 16:15:07 +0300
@@ -1657,7 +1657,7 @@
        (or (char=? c #\/)
            (char=? c #\\)))
 
-     (define file-name-separator-string "\\")
+     (define file-name-separator-string "/")
 
      (define (absolute-file-name? file-name)
        (define (file-name-separator-at-index? idx)


--- test-suite/tests/ports.test~2	2014-06-29 16:06:51 +0300
+++ test-suite/tests/ports.test	2014-07-03 10:55:30 +0300
@@ -1866,6 +1865,17 @@
     (with-fluids ((%file-port-name-canonicalization 'absolute))
       (port-filename (open-input-file (%search-load-path "ice-9/q.scm"))))))
 
+(with-test-prefix "file name separators"
+
+  (pass-if "no backslash separators in Windows file names"
+    ;; In Guile 2.0.11 and earlier, %load-path on Windows could
+    ;; include file names with backslashes, and `getcwd' on Windows
+    ;; would always return a directory name with backslashes.
+    (or (not (file-name-separator? #\\))
+	(with-load-path (cons (getcwd) %load-path)
+	  (not (string-index (%search-load-path (basename (test-file)))
+			     #\\))))))
+
 (delete-file (test-file))
 
 ;;; Local Variables:




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Provide reasonable stack limit on MS-Windows
  2014-07-02 21:02                 ` Ludovic Courtès
@ 2014-07-03 16:28                   ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-03 16:28 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Wed, 02 Jul 2014 23:02:05 +0200
> 
> Eli Zaretskii <eliz@gnu.org> skribis:
> 
> > The default value used by debug.c when getrlimit is not available is
> > too small (the default stack size of MinGW programs is 2MB).  This
> > small patch fixes that:
> 
> Looks good to me, please push.

Done.

> But it’s really problematic that we’re accumulating all this while a
> bunch of Gnulib modules would have done it; more work initially, but
> then more benefits.  (Did I say that already?  ;-))

Yes, you did say that.  (There are no getrlimit module in Gnulib,
FWIW.)




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Bug in scm_getaffinity
  2014-07-02 21:04                 ` Ludovic Courtès
@ 2014-07-03 16:31                   ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-03 16:31 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Wed, 02 Jul 2014 23:04:38 +0200
> 
> >   static SCM
> >   cpu_set_to_bitvector (const cpu_set_t *cs)
> >   {
> >     SCM bv;
> >     size_t cpu;
> >
> >     bv = scm_c_make_bitvector (sizeof (*cs), SCM_BOOL_F);
> >
> >     for (cpu = 0; cpu < sizeof (*cs); cpu++)
> >       {
> > 	if (CPU_ISSET (cpu, cs))
> > 	  /* XXX: This is inefficient but avoids code duplication.  */
> > 	  scm_c_bitvector_set_x (bv, cpu, SCM_BOOL_T);
> >       }
> >
> > I think using 'sizeof (*cs)' is incorrect here, we need to use
> > CPU_SETSIZE instead.  The cpu_set_t data type could be an array of bit
> > masks, in which case counting only bytes in it is wrong: the result is
> > too small.
> 
> Indeed, good catch.  Could you commit the obvious fix?

Done.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-03 15:10                   ` Eli Zaretskii
@ 2014-07-03 17:11                     ` Ludovic Courtès
  2014-07-03 18:09                       ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-07-03 17:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: guile-devel@gnu.org
>> Date: Wed, 02 Jul 2014 22:57:41 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> skribis:
>> 
>> >> From: ludo@gnu.org (Ludovic Courtès)
>> >> Cc: guile-devel@gnu.org
>> >> Date: Tue, 01 Jul 2014 17:38:04 +0200
>> 
>> [...]
>> 
>> >> You can look at load.c, and in particular scm_init_load_path.
>> >
>> > OK, thanks for the pointer.
>> >
>> > I've reviewed the related code, and below is what I suggest to push.
>> > (This supersedes what I sent in
>> > http://lists.gnu.org/archive/html/guile-devel/2014-06/msg00066.html.)
>> 
>> Also, could you add tests for that?  Namely, a ‘search-path’ use that
>> currently returns a file name with backslashes, and will now return a
>> file name with forward slashes.
>
> Is the below OK?

Looks good to me, OK to push.

[...]

> --- test-suite/tests/ports.test~2	2014-06-29 16:06:51 +0300
> +++ test-suite/tests/ports.test	2014-07-03 10:55:30 +0300
> @@ -1866,6 +1865,17 @@
>      (with-fluids ((%file-port-name-canonicalization 'absolute))
>        (port-filename (open-input-file (%search-load-path "ice-9/q.scm"))))))
>  
> +(with-test-prefix "file name separators"
> +
> +  (pass-if "no backslash separators in Windows file names"
> +    ;; In Guile 2.0.11 and earlier, %load-path on Windows could
> +    ;; include file names with backslashes, and `getcwd' on Windows
> +    ;; would always return a directory name with backslashes.
> +    (or (not (file-name-separator? #\\))
> +	(with-load-path (cons (getcwd) %load-path)
> +	  (not (string-index (%search-load-path (basename (test-file)))
> +			     #\\))))))

No tabs in Scheme files please (if you use Emacs, there’s a
.dir-locals.el that should set it up correctly.)

Thanks!

Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-03 17:11                     ` Ludovic Courtès
@ 2014-07-03 18:09                       ` Eli Zaretskii
  2014-07-07 20:53                         ` Mark H Weaver
  0 siblings, 1 reply; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-03 18:09 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Thu, 03 Jul 2014 19:11:41 +0200
> 
> > Is the below OK?
> 
> Looks good to me, OK to push.

Done.

> > --- test-suite/tests/ports.test~2	2014-06-29 16:06:51 +0300
> > +++ test-suite/tests/ports.test	2014-07-03 10:55:30 +0300
> > @@ -1866,6 +1865,17 @@
> >      (with-fluids ((%file-port-name-canonicalization 'absolute))
> >        (port-filename (open-input-file (%search-load-path "ice-9/q.scm"))))))
> >  
> > +(with-test-prefix "file name separators"
> > +
> > +  (pass-if "no backslash separators in Windows file names"
> > +    ;; In Guile 2.0.11 and earlier, %load-path on Windows could
> > +    ;; include file names with backslashes, and `getcwd' on Windows
> > +    ;; would always return a directory name with backslashes.
> > +    (or (not (file-name-separator? #\\))
> > +	(with-load-path (cons (getcwd) %load-path)
> > +	  (not (string-index (%search-load-path (basename (test-file)))
> > +			     #\\))))))
> 
> No tabs in Scheme files please (if you use Emacs, there’s a
> .dir-locals.el that should set it up correctly.)

.dir-locals don't affect patch application by Patch.

Btw, there are quite a few *.test files with tabs.  I untabified the
ones I modified recently (even if tabs were in places I didn't
modify), but there are more left.  JFYI.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-03 18:09                       ` Eli Zaretskii
@ 2014-07-07 20:53                         ` Mark H Weaver
  2014-07-08  2:37                           ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Mark H Weaver @ 2014-07-07 20:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Ludovic Courtès, guile-devel

Eli Zaretskii <eliz@gnu.org> writes:
> Btw, there are quite a few *.test files with tabs.  I untabified the
> ones I modified recently (even if tabs were in places I didn't
> modify), but there are more left.  JFYI.

Please, in the future let's discuss this before you push large-scale
whitespace changes like this.  In the past such proposals have been
rejected.  They make merges difficult.

    Thanks,
      Mark



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Windows file name separators
  2014-07-07 20:53                         ` Mark H Weaver
@ 2014-07-08  2:37                           ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-08  2:37 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: ludo, guile-devel

> From: Mark H Weaver <mhw@netris.org>
> Cc: ludo@gnu.org (Ludovic Courtès),  guile-devel@gnu.org
> Date: Mon, 07 Jul 2014 16:53:04 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Btw, there are quite a few *.test files with tabs.  I untabified the
> > ones I modified recently (even if tabs were in places I didn't
> > modify), but there are more left.  JFYI.
> 
> Please, in the future let's discuss this before you push large-scale
> whitespace changes like this.  In the past such proposals have been
> rejected.  They make merges difficult.

That's why I pushed changes only in the files where tabs were caused
by my recent modifications.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Fix 'dirname' and 'basename' on MS-Windows
  2014-07-02 16:13               ` Fix 'dirname' and 'basename' on MS-Windows Eli Zaretskii
@ 2014-07-09 14:22                 ` Ludovic Courtès
  2014-07-09 14:53                   ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Ludovic Courtès @ 2014-07-09 14:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: guile-devel

Eli Zaretskii <eliz@gnu.org> skribis:

> These 2 functions don't deal correctly with Windows file names with
> drive letters and with UNCs.  The patch below fixes that.
>
> Incidentally, isn't the line in scm_basename marked below wrong?
>
>   if (i == end)
>     {
>       if (len > 0 && is_file_name_separator (scm_c_string_ref (filename, 0)))
>         return scm_c_substring (filename, 0, 1);
>       else
> 	return scm_dot_string;  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
>     }
>   else
>     return scm_c_substring (filename, i+1, end+1);
>
> It is responsible for the following strange results:
>
>    (basename ".foo" ".foo")  => "."
>    (basename "_foo" "_foo")  => "."
>
> Also, isn't the following result wrong as well?
>
>    (basename "/")  => "/"
>
> I think all of these should return the empty string, "".

(I think I forgot about this message, sorry.)

It seems that Gnulib’s dirname-lgpl and basename-lgpl modules do what
you want.  Could you confirm?

If that’s the case, I’ll import them.  If you want to commit
Window-specific tests, that’s even better.

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Fix 'dirname' and 'basename' on MS-Windows
  2014-07-09 14:22                 ` Ludovic Courtès
@ 2014-07-09 14:53                   ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-09 14:53 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

> From: ludo@gnu.org (Ludovic Courtès)
> Cc: guile-devel@gnu.org
> Date: Wed, 09 Jul 2014 16:22:02 +0200
> 
> It seems that Gnulib’s dirname-lgpl and basename-lgpl modules do what
> you want.  Could you confirm?

Yes, they do the job.  But since we want to support UNCs, we need to
define DOUBLE_SLASH_IS_DISTINCT_ROOT to a non-zero value, because
Gnulib doesn't.

> If that’s the case, I’ll import them.  If you want to commit
> Window-specific tests, that’s even better.

There are no tests of these functions currently.  Do you mean you want
a Window-only test?

Thanks.




^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Fix 'dirname' and 'basename' on MS-Windows
@ 2014-07-09 15:16 Nelson H. F. Beebe
  2014-07-09 16:49 ` Eli Zaretskii
  0 siblings, 1 reply; 67+ messages in thread
From: Nelson H. F. Beebe @ 2014-07-09 15:16 UTC (permalink / raw)
  To: guile-devel; +Cc: beebe

Eli Zaretskii <eliz@gnu.org> comments on misbehavior (or unexpected
behavior) of guile's (basename ...) function:

>    (basename ".foo" ".foo")  => "."
>    (basename "_foo" "_foo")  => "."
>
> Also, isn't the following result wrong as well?
>
>    (basename "/")  => "/"

According to built-in documentation:

	guile> (help basename)
	`basename' is a primitive procedure in the (guile) module.

	 -- Scheme Procedure: basename filename [suffix]
	     Return the base name of the file name FILENAME. The base name is
	     the file name without any directory components.  If SUFFIX is
	     provided, and is equal to the end of BASENAME, it is removed also.

So, let us see what these produce:

	guile> (basename ".foo" ".foo")
	"."

	guile> (basename "_foo" "_foo")
	"."

The documentation clearly indicates that the matching suffix is
removed, in which case, the result should be a empty string.  The
function therefore does not follow its documentation, and one or the
other are wrong.

However, the Unix (and POSIX) basename and dirname commands have been
around since at least 1979 (I found them in my Unix 7th edition
manuals from that year), and I think it would be wise to follow the
POSIX standard for their implementation:

	% basename /tmp/x/y/z/foo.bar
	foo.bar

	% basename /tmp/x/y/z/foo.bar .bar
	foo

	% basename /tmp/x/y/z/foo.bar bar
	foo.

	% basename foo.bar .bar
	foo

	% basename .bar .bar
	.bar

The possibly-surprising behaviour of that last example is due to the
wording in POSIX (IEEE Std 1003.1-2001):

>> ...
>> 6. If the suffix operand is present, is not identical to the
>>    characters remaining in string, and is identical to a suffix of the
>>    characters remaining in string, the suffix suffix shall be removed
>>    from string. Otherwise, string is not modified by this step. It
>>    shall not be considered an error if suffix is not found in string.
>> ...

The phrase `is not identical to the characters remaining in string'
means that ".bar" is the result, rather than "".

Also notice that POSIX defines a basename() library function, but it
takes only one argument, and thus does not have the same behavior as
the basename command when the latter has two arguments.  Because guile
offers a choice of 1 or 2 arguments, its basename function was
presumably modeled on the POSIX command, rather than the POSIX library
function.

Also, in guile documentation, would it not be better to replace "file
name", "base name", FILENAME, and BASENAME with the standard POSIX
terminology "pathname" and "filename"?

	/tmp/x/y/z/foo.bar	# a pathname
	/tmp/x/y/z		# the path to (or directory of) that pathname
	foo.bar			# the filename of that pathname

POSIX says this about those names:
         
>> ...
>> 3.2     Absolute Pathname
>> 
>>         A pathname beginning with a single or more than two
>>         slashes; see also Section 3.266
>> ...
        
>> ...
>> 3.40    Basename
>> 
>>         The final, or only, filename in a pathname.
>> ...

>> ...
>> 3.169      Filename
>> 
>> 	   A name consisting of 1 to {NAME_MAX} bytes used to name a
>> 	   file. The characters composing the name may be selected
>> 	   from the set of all character values excluding the slash
>> 	   character and the null byte. The filenames dot and dot-dot
>> 	   have special meaning. A filename is sometimes referred to
>> 	   as a ``pathname component''.
>> ...
>>

>> ...
>> 3.266    Pathname
>> 
>>          A character string that is used to identify a file. In the
>>          context of IEEE Std 1003.1-2001, a pathname consists of, at
>>          most, {PATH_MAX} bytes, including the terminating null
>>          byte. It has an optional beginning slash, followed by zero or
>>          more filenames separated by slashes. A pathname may
>>          optionally contain one or more trailing slashes. Multiple
>>          successive slashes are considered to be the same as one
>>          slash.
>> ...

>> ...
>> 3.319    Relative Pathname
>> 
>>          A pathname not beginning with a slash.
>> ...
>>         

>> ...
>> 4.11      Pathname Resolution
>> 
>> ... long complex text omitted ...
>> 
>> A pathname consisting of a single slash shall resolve to the root
>> directory of the process. A null pathname shall not be successfully
>> resolved. A pathname that begins with two successive slashes may be
>> interpreted in an implementation-defined manner, although more than
>> two leading slashes shall be treated as a single slash.
>> ...

-------------------------------------------------------------------------------
- Nelson H. F. Beebe                    Tel: +1 801 581 5254                  -
- University of Utah                    FAX: +1 801 581 4148                  -
- Department of Mathematics, 110 LCB    Internet e-mail: beebe@math.utah.edu  -
- 155 S 1400 E RM 233                       beebe@acm.org  beebe@computer.org -
- Salt Lake City, UT 84112-0090, USA    URL: http://www.math.utah.edu/~beebe/ -
-------------------------------------------------------------------------------



^ permalink raw reply	[flat|nested] 67+ messages in thread

* Re: Fix 'dirname' and 'basename' on MS-Windows
  2014-07-09 15:16 Fix 'dirname' and 'basename' on MS-Windows Nelson H. F. Beebe
@ 2014-07-09 16:49 ` Eli Zaretskii
  0 siblings, 0 replies; 67+ messages in thread
From: Eli Zaretskii @ 2014-07-09 16:49 UTC (permalink / raw)
  To: Nelson H. F. Beebe; +Cc: guile-devel

> Date: Wed, 9 Jul 2014 09:16:35 -0600 (MDT)
> From: "Nelson H. F. Beebe" <beebe@math.utah.edu>
> Cc: beebe@math.utah.edu
> 
> >> 6. If the suffix operand is present, is not identical to the
> >>    characters remaining in string, and is identical to a suffix of the
> >>    characters remaining in string, the suffix suffix shall be removed
> >>    from string. Otherwise, string is not modified by this step. It
> >>    shall not be considered an error if suffix is not found in string.
> >> ...
> 
> The phrase `is not identical to the characters remaining in string'
> means that ".bar" is the result, rather than "".

Any idea why does this exception make sense?

> Also, in guile documentation, would it not be better to replace "file
> name", "base name", FILENAME, and BASENAME with the standard POSIX
> terminology "pathname" and "filename"?

GNU Coding Standards frown upon using "path" for anything that is not
a PATH-style directory list.  In GNU terminology, "filename" can be
both full (a.k.a. "absolute") and relative file names.



^ permalink raw reply	[flat|nested] 67+ messages in thread

end of thread, other threads:[~2014-07-09 16:49 UTC | newest]

Thread overview: 67+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-08 15:04 MinGW issues in Guile 2.0.11 Eli Zaretskii
2014-06-09 15:47 ` Eli Zaretskii
2014-06-09 18:01 ` Mark H Weaver
2014-06-09 18:25   ` Eli Zaretskii
2014-06-09 19:30 ` MinGW vs. setlocale Ludovic Courtès
2014-06-10 16:17   ` Eli Zaretskii
2014-06-11 13:13     ` Ludovic Courtès
2014-06-11 15:33       ` Eli Zaretskii
2014-06-12  8:39         ` Ludovic Courtès
2014-06-12 18:18           ` Eli Zaretskii
2014-06-15 17:23             ` Eli Zaretskii
2014-06-21 11:17               ` Eli Zaretskii
2014-06-21 15:02               ` Ludovic Courtès
2014-06-21 15:11                 ` Eli Zaretskii
2014-06-21 21:23                   ` Ludovic Courtès
2014-06-22 16:13                     ` Eli Zaretskii
2014-06-19  4:53       ` Doug Evans
2014-06-19  8:16         ` Ludovic Courtès
2014-06-09 19:32 ` MinGW vs. c-api.test Ludovic Courtès
2014-06-10  9:05   ` Neil Jerram
2014-06-10 11:42     ` Ludovic Courtès
2014-06-10 15:32     ` Eli Zaretskii
2014-06-10 15:56       ` David Kastrup
2014-06-10 18:00         ` Eli Zaretskii
2014-06-11  0:36           ` dsmich
2014-06-11  2:52             ` Eli Zaretskii
2014-06-10 11:44   ` Ludovic Courtès
2014-06-10 15:39     ` Eli Zaretskii
2014-06-11 12:37       ` Ludovic Courtès
2014-06-11 15:08         ` Eli Zaretskii
2014-06-12  8:29           ` Ludovic Courtès
2014-06-12 17:16             ` Eli Zaretskii
2014-06-12 19:48               ` Ludovic Courtès
2014-06-12 19:59                 ` Eli Zaretskii
2014-06-12 21:20                   ` Ludovic Courtès
2014-06-13  9:15                     ` Neil Jerram
2014-06-13 16:04                       ` Ludovic Courtès
2014-06-13 16:19                         ` Eli Zaretskii
2014-06-13 16:26                           ` Neil Jerram
2014-06-13 16:31                         ` Mike Gerwitz
2014-06-13 18:09                           ` Eli Zaretskii
2014-06-09 19:42 ` Windows file name separators Ludovic Courtès
2014-06-10 16:00   ` Eli Zaretskii
2014-06-30 11:15     ` Ludovic Courtès
2014-06-30 14:56       ` Eli Zaretskii
2014-07-01  9:36         ` Ludovic Courtès
2014-07-01 15:30           ` Eli Zaretskii
2014-07-01 15:38             ` Ludovic Courtès
2014-07-02 16:04               ` Eli Zaretskii
2014-07-02 20:56                 ` Ludovic Courtès
2014-07-02 20:57                 ` Ludovic Courtès
2014-07-03 15:10                   ` Eli Zaretskii
2014-07-03 17:11                     ` Ludovic Courtès
2014-07-03 18:09                       ` Eli Zaretskii
2014-07-07 20:53                         ` Mark H Weaver
2014-07-08  2:37                           ` Eli Zaretskii
2014-07-02 16:13               ` Fix 'dirname' and 'basename' on MS-Windows Eli Zaretskii
2014-07-09 14:22                 ` Ludovic Courtès
2014-07-09 14:53                   ` Eli Zaretskii
2014-07-02 16:16               ` Provide reasonable stack limit " Eli Zaretskii
2014-07-02 21:02                 ` Ludovic Courtès
2014-07-03 16:28                   ` Eli Zaretskii
2014-07-02 16:30               ` Bug in scm_getaffinity Eli Zaretskii
2014-07-02 21:04                 ` Ludovic Courtès
2014-07-03 16:31                   ` Eli Zaretskii
  -- strict thread matches above, loose matches on Subject: below --
2014-07-09 15:16 Fix 'dirname' and 'basename' on MS-Windows Nelson H. F. Beebe
2014-07-09 16:49 ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).