bug#32246: 27.0.50; files-tests failure on Cygwin

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

I'm getting a failure of
files-tests-file-name-non-special-file-name-case-insensitive-p on
Cygwin:

Test files-tests-file-name-non-special-file-name-case-insensitive-p 
backtrace:
   signal(ert-test-failed (((should (equal (file-name-case-insensitive-
   ert-fail(((should (equal (file-name-case-insensitive-p nospecial) (f
   (if (unwind-protect (setq value-532 (apply fn-530 args-531)) (setq f
   (let (form-description-534) (if (unwind-protect (setq value-532 (app
   (let ((value-532 'ert-form-evaluation-aborted-533)) (let (form-descr
   (let* ((fn-530 #'equal) (args-531 (condition-case err (let ((signal-
   (progn (let* ((fn-530 #'equal) (args-531 (condition-case err (let ((
   (unwind-protect (progn (let* ((fn-530 #'equal) (args-531 (condition-
   (let* ((temporary-file-directory (file-truename temporary-file-direc
   (closure (t) nil (let* ((temporary-file-directory (file-truename tem
   ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
   ert-run-test(#s(ert-test :name files-tests-file-name-non-special-fil
   ert-run-or-rerun-test(#s(ert--stats :selector (not (tag :unstable))
   ert-run-tests((not (tag :unstable)) #f(compiled-function (event-type
   ert-run-tests-batch((not (tag :unstable)))
   ert-run-tests-batch-and-exit((not (tag :unstable)))
   eval((ert-run-tests-batch-and-exit '(not (tag :unstable))))
   command-line-1(("-L" ":../../master/test" "-l" "ert" "-l" "lisp/file
   command-line()
   normal-top-level()
Test files-tests-file-name-non-special-file-name-case-insensitive-p 
condition:
     (ert-test-failed
      ((should
        (equal
         (file-name-case-insensitive-p nospecial)
         (file-name-case-insensitive-p tmpfile)))
       :form
       (equal t nil)
       :value nil :explanation
       (different-atoms t nil)))
    FAILED  40/78 
files-tests-file-name-non-special-file-name-case-insensitive-p (0.003135 
sec)

A bisection shows that the problem started with the following commit:

commit e75c57f10ee9418599398361b0676f48d265fb12
Author: Michael Albinus <michael.albinus@gmx.de>
Date:   Sun Jun 3 14:30:41 2018 +0200

     Extend file-name-non-special

Please let me know what I can do to help track this down.

Ken


In GNU Emacs 27.0.50 (build 21, x86_64-unknown-cygwin, GTK+ Version 3.22.28)
  of 2018-07-22 built on moufang
Repository revision: f3f67cf0b9a6388d423e048a7aafad2eb531b5a5
Windowing system distributor 'The Cygwin/X Project', version 11.0.12000000
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GSETTINGS NOTIFY ACL
GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11 THREADS LCMS2

Important settings:
   value of $LANG: en_US.UTF-8
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
easymenu mml-sec password-cache epa derived epg epg-config gnus-util
rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader cl-loaddefs cl-lib sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils elec-pair time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
gfilenotify lcms2 dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 94354 9921)
  (symbols 48 20128 1)
  (miscs 40 39 93)
  (strings 32 28382 1874)
  (string-bytes 1 749578)
  (vectors 16 14231)
  (vector-slots 8 503648 13212)
  (floats 8 48 86)
  (intervals 56 207 0)
  (buffers 992 12))

bug#32246: 27.0.50; files-tests failure on Cygwin

[Michael Albinus]

Ken Brown <kbrown@cornell.edu> writes:

Hi Ken,

> I'm getting a failure of
> files-tests-file-name-non-special-file-name-case-insensitive-p on
> Cygwin:

Will check next days, when I have acquired a Windows machine temporarily.

> Please let me know what I can do to help track this down.

Could you, pls, provide me a recent Cygwin Emacs?

> Ken

Best regards, Michael.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

Hi Michael,

On 7/22/2018 2:23 PM, Michael Albinus wrote:
> Ken Brown <kbrown@cornell.edu> writes:
> 
> Hi Ken,
> 
>> I'm getting a failure of
>> files-tests-file-name-non-special-file-name-case-insensitive-p on
>> Cygwin:
> 
> Will check next days, when I have acquired a Windows machine temporarily.

Thanks.

>> Please let me know what I can do to help track this down.
> 
> Could you, pls, provide me a recent Cygwin Emacs?

I've just uploaded one to my private Cygwin repository at

   http://sanibeltranquility.com/cygwin/index.html

There are instructions at that site.  Let me know if anything is unclear.

Thanks.

Ken

bug#32246: 27.0.50; files-tests failure on Cygwin

[Michael Albinus]

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

> I've just uploaded one to my private Cygwin repository at

Thanks for this. However, I haven't been able to reproduce the
problem. I've tried it interactively, running ert over the tests in
files-tests.el. And I've tried it in a Gnome terminal, running

--8<---------------cut here---------------start------------->8---
$ emacs -Q -batch -l /cygdrive/c/Users/lb01177/Downloads/files-tests.el --eval '(ert-run-tests-batch-and-exit (quote t))'
--8<---------------cut here---------------end--------------->8---

In both cases, the error didn't happen.

Finally, I've tried it in bash, called directly from the MS Windows:

--8<---------------cut here---------------start------------->8---
$ /cygdrive/c/cygwin64/bin/emacs-nox -Q -batch -l /cygdrive/c/Users/lb01177/Downloads/files-tests.el --eval '(ert-run-tests-batch-and-exit (quote t))'
--8<---------------cut here---------------end--------------->8---

In this case I've got two errors

--8<---------------cut here---------------start------------->8---
2 unexpected results:
   FAILED  files-tests--copy-directory
   FAILED  files-tests--make-directory
--8<---------------cut here---------------end--------------->8---

But they are different from what you have reported.

How do you get this error? Note that I haven't the downloaded Emacs git
repository under cagwin, and so I cannot call make. But I would like to
know your environment, is it a desktop manager, or a terminal in an X11
environment, or just called from MS Windows.

> Thanks.
>
> Ken

Best regards, Michael.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

On 7/25/2018 8:00 AM, Michael Albinus wrote:
> Hi Ken,

Hi Michael,

> Thanks for this. However, I haven't been able to reproduce the
> problem. I've tried it interactively, running ert over the tests in
> files-tests.el. And I've tried it in a Gnome terminal, running
> 
> --8<---------------cut here---------------start------------->8---
> $ emacs -Q -batch -l /cygdrive/c/Users/lb01177/Downloads/files-tests.el --eval '(ert-run-tests-batch-and-exit (quote t))'
> --8<---------------cut here---------------end--------------->8---
> 
> In both cases, the error didn't happen.
> 
> How do you get this error? Note that I haven't the downloaded Emacs git
> repository under cagwin, and so I cannot call make. But I would like to
> know your environment, is it a desktop manager, or a terminal in an X11
> environment, or just called from MS Windows.

Thanks for testing.

I left out a crucial piece of information in my bug report (sorry!): I 
always set up my Cygwin systems to be case sensitive, as explained here:

 
https://cygwin.com/cygwin-ug-net/using-specialnames.html#pathnames-casesensitive

If you don't do this, then all file systems are case insensitive.  And 
even if you do this, file names under /cygdrive are case insensitive, as 
explained at the link above.  So I'm not surprised that you couldn't 
reproduce the error.

Apart from that, my normal way of running tests is simply 'make check' 
in my build directory.  I work either in a Cygwin terminal (mintty) or 
an xterm window under X11.  Here's what happens when I run the tests the 
way you did, from my build directory in a Cygwin terminal (and avoiding 
/cygdrive):

$ src/emacs -Q -batch -l ../master/test/lisp/files-tests.el --eval 
'(ert-run-tests-batch-and-exit (quote t))'
Running 78 tests (2018-07-25 09:33:31-0400, selector ‘t’)

[...]

Ran 78 tests, 76 results as expected, 1 unexpected, 1 skipped 
(2018-07-25 09:33:34-0400, 2.591031 sec)
1 expected failures

1 unexpected results:
    FAILED  files-tests-file-name-non-special-file-name-case-insensitive-p

1 skipped results:
   SKIPPED  files-test-bug-18141

Best regards,

Ken

bug#32246: 27.0.50; files-tests failure on Cygwin

[Michael Albinus]

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

> I left out a crucial piece of information in my bug report (sorry!): I
> always set up my Cygwin systems to be case sensitive, as explained
> here:
>
> https://cygwin.com/cygwin-ug-net/using-specialnames.html#pathnames-casesensitive
>
> If you don't do this, then all file systems are case insensitive.  And
> even if you do this, file names under /cygdrive are case insensitive,
> as explained at the link above.  So I'm not surprised that you
> couldn't reproduce the error.

Well, file_name_case_insensitive_p of fileio.c doesn't care:

--8<---------------cut here---------------start------------->8---
#if defined CYGWIN || defined DOS_NT
  return true;
#else
  return false;
#endif
--8<---------------cut here---------------end--------------->8---

Do you believe this is worth fixing? If yes, I would let it to you.

Otherwise, I will add (skip-unless (not (eq system-type 'cygwin))) to
files-tests-file-name-non-special-file-name-case-insensitive-p.

> Best regards,
>
> Ken

Best regards, Michael.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

On 7/25/2018 12:46 PM, Michael Albinus wrote:
> Ken Brown <kbrown@cornell.edu> writes:
> 
> Hi Ken,

Hi Michael,

>> I left out a crucial piece of information in my bug report (sorry!): I
>> always set up my Cygwin systems to be case sensitive, as explained
>> here:
>>
>> https://cygwin.com/cygwin-ug-net/using-specialnames.html#pathnames-casesensitive
>>
>> If you don't do this, then all file systems are case insensitive.  And
>> even if you do this, file names under /cygdrive are case insensitive,
>> as explained at the link above.  So I'm not surprised that you
>> couldn't reproduce the error.
> 
> Well, file_name_case_insensitive_p of fileio.c doesn't care:
> 
> --8<---------------cut here---------------start------------->8---
> #if defined CYGWIN || defined DOS_NT
>    return true;
> #else
>    return false;
> #endif
> --8<---------------cut here---------------end--------------->8---

No, that code is a fallback that is only relevant if the call to 
pathconf with _PC_CASE_INSENSITIVE fails or can't be made.  See the 
comment at the beginning of the definition of 
file_name_case_insensitive_p, and also the comment before the definition.

Best regards,

Ken

bug#32246: 27.0.50; files-tests failure on Cygwin

[Michael Albinus]

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

> No, that code is a fallback that is only relevant if the call to
> pathconf with _PC_CASE_INSENSITIVE fails or can't be made.  See the
> comment at the beginning of the definition of
> file_name_case_insensitive_p, and also the comment before the
> definition.

OK, so I will continue to debug.

> Best regards,
>
> Ken

Best regards, Michael.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi Ken,

>> No, that code is a fallback that is only relevant if the call to
>> pathconf with _PC_CASE_INSENSITIVE fails or can't be made.  See the
>> comment at the beginning of the definition of
>> file_name_case_insensitive_p, and also the comment before the
>> definition.
>
> OK, so I will continue to debug.

It looks that on cygwin, file-name-case-insensitive-p returns different
values for existing and non-existing files:

--8<---------------cut here---------------start------------->8---
M-! touch /tmp/foo

(file-name-case-insensitive-p "/tmp/foo")
=> nil

(file-name-case-insensitive-p "/tmp/bar")
=> t
--8<---------------cut here---------------end--------------->8---

The doc of file-name-case-insensitive-p does not require that FILENAME exists.

>> Best regards,
>>
>> Ken

Best regards, Michael.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

On 7/26/2018 9:56 AM, Michael Albinus wrote:
> Michael Albinus <michael.albinus@gmx.de> writes:

Hi Michael,

> It looks that on cygwin, file-name-case-insensitive-p returns different
> values for existing and non-existing files:
> 
> --8<---------------cut here---------------start------------->8---
> M-! touch /tmp/foo
> 
> (file-name-case-insensitive-p "/tmp/foo")
> => nil
> 
> (file-name-case-insensitive-p "/tmp/bar")
> => t
> --8<---------------cut here---------------end--------------->8---

Right.  If the file doesn't exist, then the call to pathconf fails with 
ENOENT, as required by POSIX 
(http://pubs.opengroup.org/onlinepubs/009695299/functions/fpathconf.html). 
  The fallback code in file_name_case_insensitive_p then kicks in and 
returns true on Cygwin.

> The doc of file-name-case-insensitive-p does not require that FILENAME exists.

I think the doc is ambiguous on that.  The first sentence of the doc in 
the elisp manual says, "Sometimes file names or their parts need to be 
compared as strings, in which case it’s important to know whether the 
underlying filesystem is case-insensitive."  This certainly suggests 
that the file should exist, but there's no indication of what the 
function returns for a non-existent file.  And the code doesn't attempt 
to detect this case.

I don't know what the right solution is.

Thanks for tracking down the problem.

Best regards,

Ken

bug#32246: 27.0.50; files-tests failure on Cygwin

[Michael Albinus]

Ken Brown <kbrown@cornell.edu> writes:

> Hi Michael,

Hi Ken,

>> The doc of file-name-case-insensitive-p does not require that FILENAME exists.
>
> I think the doc is ambiguous on that.  The first sentence of the doc
> in the elisp manual says, "Sometimes file names or their parts need to
> be compared as strings, in which case it’s important to know whether
> the underlying filesystem is case-insensitive."  This certainly
> suggests that the file should exist, but there's no indication of what
> the function returns for a non-existent file.  And the code doesn't
> attempt to detect this case.
>
> I don't know what the right solution is.

The doc speaks from "the underlying filesystem". So I believe *this*
must be checked. For non-existing files, the respective directory (up to
the part which exist) should be checked. In case of
"/non-existing-dir/non-existing-file", "/" must be checked.

> Best regards,
>
> Ken

Best regards, Michael.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> Date: Thu, 26 Jul 2018 10:54:38 -0400
> Cc: 32246@debbugs.gnu.org
> 
> I think the doc is ambiguous on that.  The first sentence of the doc in 
> the elisp manual says, "Sometimes file names or their parts need to be 
> compared as strings, in which case it’s important to know whether the 
> underlying filesystem is case-insensitive."  This certainly suggests 
> that the file should exist, but there's no indication of what the 
> function returns for a non-existent file.  And the code doesn't attempt 
> to detect this case.
> 
> I don't know what the right solution is.

The usual solution is to pass the root of the volume to pathconf.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> The usual solution is to pass the root of the volume to pathconf.

How do you determine the root of the volume?

Best regards, Michael.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

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

On 7/26/2018 11:03 AM, Michael Albinus wrote:
> The doc speaks from "the underlying filesystem". So I believe *this*
> must be checked. For non-existing files, the respective directory (up to
> the part which exist) should be checked. In case of
> "/non-existing-dir/non-existing-file", "/" must be checked.

Thanks, that makes sense.  Patch attached.

Ken

[-- Attachment #2: 0001-Fix-file-name-case-insensitive-p-on-non-existent-fil.patch --]
[-- Type: text/plain, Size: 1412 bytes --]

From 8f8c1050509f88f38f6c2b1681c969b1e9922413 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Fri, 27 Jul 2018 14:24:01 -0400
Subject: [PATCH] Fix file-name-case-insensitive-p on non-existent files

* src/fileio.c (Ffile_name_case_insensitive_p): If the file
doesn't exist, move up the filesystem tree until an existing
directory is found.  Then test that directory for
case-insensitivity.  (Bug#32246)
---
 src/fileio.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/fileio.c b/src/fileio.c
index b92492c93a..2dcfb73b0d 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2296,6 +2296,21 @@ The arg must be a string.  */)
   if (!NILP (handler))
     return call2 (handler, Qfile_name_case_insensitive_p, filename);
 
+  /* If the file doesn't exist, move up the filesystem tree until we
+     reach an existing directory or the root.  */
+  if (NILP (Ffile_exists_p (filename)))
+    {
+      filename = Ffile_name_directory (filename);
+      while (NILP (Ffile_exists_p (filename)))
+	{
+	  Lisp_Object newname = expand_and_dir_to_file (filename);
+	  /* Avoid infinite loop if the root is reported as non-existing
+	     (impossible?).  */
+	  if (!NILP (Fstring_equal (newname, filename)))
+	    break;
+	  filename = newname;
+	}
+    }
   filename = ENCODE_FILE (filename);
   return file_name_case_insensitive_p (SSDATA (filename)) ? Qt : Qnil;
 }
-- 
2.17.0

bug#32246: 27.0.50; files-tests failure on Cygwin

[Michael Albinus]

Ken Brown <kbrown@cornell.edu> writes:

Hi Ken,

> Thanks, that makes sense.  Patch attached.

LGTM.

I cannot build for cygwin myself. So if this patch fixes the error for
you, you might commit.

There's the other proposal from Eli, use the root of the volume. This
would work also, but I don't know how to find the root of a volume
easily.

> Ken

Best regards, Michael.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

On 7/27/2018 3:30 PM, Michael Albinus wrote:

Hi Michael,

> LGTM.
> 
> I cannot build for cygwin myself. So if this patch fixes the error for
> you, you might commit.

Yes, it fixes the error.  But I'll wait for Eli's approval before 
committing.

> There's the other proposal from Eli, use the root of the volume. This
> would work also, but I don't know how to find the root of a volume
> easily.

I don't either.  And this would be especially tricky on Cygwin, where 
I'm not even sure what "volume" should mean.  For example, /proc and 
/cygdrive are case-insensitive on my system, even though I've set my 
system up to be case-sensitive otherwise.  So it seems that /proc and 
/cygdrive should be viewed as separate file systems.

Best regards,

Ken

bug#32246: 27.0.50; files-tests failure on Cygwin

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Fri, 27 Jul 2018 21:30:24 +0200
> Cc: 32246@debbugs.gnu.org
> 
> There's the other proposal from Eli, use the root of the volume. This
> would work also, but I don't know how to find the root of a volume
> easily.

Ken's patch actually does something very similar, so unless Cygwin
makes it easy to find the volume root, I think Ken's patch is OK.

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

On 7/27/2018 5:10 PM, Eli Zaretskii wrote:
> Ken's patch actually does something very similar, so unless Cygwin
> makes it easy to find the volume root, I think Ken's patch is OK.

Pushed as commit 81d6418e6b.  Closing.

Ken

bug#32246: 27.0.50; files-tests failure on Cygwin

[Ken Brown]

On 7/27/2018 5:32 PM, Ken Brown wrote:
> Pushed as commit 81d6418e6b.  Closing.

Really closing.