Subject: [clisp-list] File error in clisp 2.28/cygwin testsuite execution

From: Karsten <[email-masked]>
Date: Sat Mar 9 08:00:02 2002

Hello,

the testcase that fails with clisp 2.28 /cygwin is the following (I added 
the handler-case so that it can be interactively used)

(handler-case
     (progn
       (with-open-file (s "/tmp/foo35.tmp" :direction :output))
       (delete-file "/tmp/foo35.tmp/bar"))
   (file-error (error) :file-error)
   (condition (condition) (values :other condition))
   (:no-error (dont-care) (declare (ignore dont-care)) :no-error)
   )

It returns :no-error, although the test-case assumes that a file error is 
raised (as it does in clisp 2.28 win32 native)

I wonder a bit why this is assumed

The with-open-file will fail, if the directory /tmp does no exist. On my 
machine it exists for cygwin and linux, but not for windows.
The delete-file fails on my machine only for linux (redhat), but not for 
windows nor for cygwin.

Karsten

---

From: Sam Steingold <[email-masked]>
Date: Sat Mar 9 13:31:03 2002

> * In message <[email-masked]>
> * On the subject of "[clisp-list] File error in clisp 2.28/cygwin testsuite execution"
> * Sent on Sat, 09 Mar 2002 16:58:47 +0100
> * Honorable Karsten <[email-masked]> writes:
>
> the testcase that fails with clisp 2.28 /cygwin is the following (I
> added the handler-case so that it can be interactively used)
> 
> (handler-case
>      (progn
> It returns :no-error, although the test-case assumes that a file error
> is raised (as it does in clisp 2.28 win32 native)
> 

the tests were originally designed on UNIX, so the existence of /tmp is
always assumed throughout the tests.

WITH-OPEN-FILE is a touch(1) - it creates a plain file (non-dir), and
DELETE-FILE accesses a non-existent dir - which should generate a
FILE-ERROR.

> The with-open-file will fail, if the directory /tmp does no exist. On
> my machine it exists for cygwin and linux, but not for windows.  The
> delete-file fails on my machine only for linux (redhat), but not for
> windows nor for cygwin.

I see.  delete-file should fail in the
<pathname.d7653::assure_dir_exists()> call
I wonder why it does not - could you please debug that?

-- 
Sam Steingold (http://www.podval.org/~sds) running RedHat7.2 GNU/Linux
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
The early bird may get the worm, but the second mouse gets the cheese.

---

From: Peter Burwood <[email-masked]>
Date: Sun Mar 10 13:33:02 2002

> I wonder why it does not - could you please debug that?

The problem with cygwin seems to be down to realpath and readlink.
realpath is called with "/tmp/foo35.tmp/." and returns
"/tmp/foo35.tmp" with errno EINVAL.  Under Linux, the return is NULL
and the errno is ENOTDIR for this call.

Thus Linux will cause a file error to be raised by assure_dir_exists,
as when realpath returns NULL, then assure_dir_exists generates a
FILE-ERROR when errno != ENOENT or tolerantp is false.

Now, realpath in this example determines its result and errno from the
internal call to readlink and the SUSv2 distinguishes some errno as
follows (ENOENT and ENOTDIR are the same for readlink and realpath):

  [EINVAL]
    The path argument names a file that is not a symbolic link. 

  [ENOENT]
    A component of file_name does not name an existing file or
    file_name points to an empty string.

  [ENOTDIR]
    A component of the path prefix is not a directory.

So, it appears to be a bug in Clisp's realpath assuming that readlink
will return ENOTDIR in favour of EINVAL.  Fortunately, we can identify
this case in realpath and I've attached a patch.  In fact, I've
supplied an alternative patch in case you don't like the first.  They
have both been tested on Cygwin.

2002-03-10  Peter Burwood  <[email-masked]>

	* pathname.d (realpath): Favour ENOTDIR rather than EINVAL for Cygwin.

Index: pathname.d
===================================================================
RCS file: /cvsroot/clisp/clisp/src/pathname.d,v
retrieving revision 1.170
diff -p -u -2 -r1.170 pathname.d
--- pathname.d	28 Feb 2002 19:49:30 -0000	1.170
+++ pathname.d	10 Mar 2002 20:33:08 -0000
@@ -198,4 +198,8 @@ local char* realpath (const char* path, 
               # directory, not a symbolic link
               to_ptr[-1] = '/'; # insert the '/' again
+            } else if (!S_ISLNK(statbuf.st_mode)) {
+              # something else, but not a directory or symbolic link.
+              errno = ENOTDIR;
+              return NULL;
             } else
               #endif

Index: pathname.d
===================================================================
RCS file: /cvsroot/clisp/clisp/src/pathname.d,v
retrieving revision 1.170
diff -p -u -2 -r1.170 pathname.d
--- pathname.d	28 Feb 2002 19:49:30 -0000	1.170
+++ pathname.d	10 Mar 2002 20:38:21 -0000
@@ -241,4 +241,11 @@ local char* realpath (const char* path, 
                   to_ptr = from_ptr;
                 } else {
+                  #if defined(UNIX_CYGWIN32)
+                    # When we've got here, we know the file is not a link due to
+                    # readlinks and not a directory, as S_ISDIR has been checked.
+                    # Thus, treat EINVAL as ENOTDIR because CLISP prefers ENOTDIR.
+                  if (errno == EINVAL)
+                    errno = ENOTDIR;
+                  #endif
                   #if defined(UNIX_IRIX)
                   if ((errno == EINVAL) || (errno == ENXIO))

I did have a quick try with Cygwin's realpath and that was looking
hopeful as realpath ("/tmp/foo35.tmp/.") returned NULL and errno
ENOENT.  Unfortunately, realpath ("/tmp/foo/bar/."), where /tmp is
empty, returns "/tmp/foo/bar" and doesn't change errno.

Perhaps this next bit should be another email...

I do question the consistency of Clisp here w.r.t. when an error is or
is not generated.  That is, it seems that delete-file can be called
with other arguments and raise no error at all.

  (with-open-file (s "/tmp/foo.txt" :direction :output))
  (delete-file "/tmp/foo.txt")
     -> #P/tmp/foo.txt"        ; no error, it existed.
  (delete-file "/tmp/foo.txt")
     -> NIL                    ; still no error, but now it doesn't

According the CLHS, on delete-file

  delete-file filespec => t

  It is implementation-dependent whether an attempt to delete a
  nonexistent file is considered to be successful.

  delete-file returns true if it succeeds, or signals an error of type
  file-error if it does not.

Which means that delete-file cannot return NIL.  Clisp is
non-conformant here, as is Clisp returning the pathname of the deleted
file.

A patch to delete-file is below.  I've chosen to make delete-file on a
nonexistent file be an error.  You may disagree as it does change
behaviour.  Please examine this patch carefully, because I'm not sure
on the STACK code.

Peter

2002-03-10  Peter Burwood  <[email-masked]>

	* pathname.d (delete_file): CLtL2 compatibility. DELETE-FILE
          returns T or raises an error.

Index: pathname.d
===================================================================
RCS file: /cvsroot/clisp/clisp/src/pathname.d,v
retrieving revision 1.170
diff -p -u -2 -r1.170 pathname.d
--- pathname.d	28 Feb 2002 19:49:30 -0000	1.170
+++ pathname.d	10 Mar 2002 21:21:47 -0000
@@ -7653,23 +7657,22 @@ LISPFUNN(delete_file,1) {
   var object namestring = assure_dir_exists(false,true); # filename for the operating system
   if (eq(namestring,nullobj)) {
-    # path to the file does not exist ==> return NIL
-    skipSTACK(1); value1 = NIL; mv_count=1; return;
+    # path to the file does not exist
+    fehler_dir_not_exists(STACK_0);
   }
   check_delete_open(STACK_0);
   # delete file:
  #ifdef FILE_EXISTS_TRIVIAL
-  if (!file_exists(namestring)) { # file does not exist -> value NIL
-    skipSTACK(1); value1 = NIL; mv_count=1; return;
-  }
+  if (!file_exists(namestring)) { fehler_file_not_exists(); }
  #endif
   with_sstring_0(namestring,O(pathname_encoding),namestring_asciz, {
     if (!delete_file_if_exists(namestring_asciz)) {
-      # file does not exist -> value NIL
-      FREE_DYNAMIC_ARRAY(namestring_asciz); skipSTACK(1);
-      value1 = NIL; mv_count=1; return;
+      # file does not exist
+      FREE_DYNAMIC_ARRAY(namestring_asciz);
+      fehler_file_not_exists();
     }
   });
-  # file existed, was deleted -> pathname (/=NIL) as value
-  value1 = popSTACK(); mv_count=1;
+  # file exists and was deleted -> t
+  skipSTACK(1); # forget pathname
+  value1 = T; mv_count=1;
 }

---

From: Sam Steingold <[email-masked]>
Date: Sun Mar 10 21:00:08 2002

> * In message <[email-masked]>
> * On the subject of "Re: [clisp-list] File error in clisp 2.28/cygwin testsuite execution"
> * Sent on 10 Mar 2002 21:30:59 +0000
> * Honorable Peter Burwood <[email-masked]> writes:
>

committed -- thanks!
[Bruno, this fixes a cygwin-specific bug]

incidentally, why is CLISP implementing its own realpath?
is the system one known to be deficient in any way?

> non-conformant here,
yes.

> as is Clisp returning the pathname of the deleted file.

well, the pathname is "true", so this complies with the third paragraph
you site (but not with the first one).
I think returning the path of the deleted file is a useful extension
which cannot possibly break anything, but if you wish you may disable
it when *ansi* is T.

> behaviour.

the main problem with CLISP is that it barfs when the dir is bad, but
returns NIL when the dir is good but the file is not there.
as you said, this is inconsistent.

I think that signaling an error should mean that "mission cannot be
accomplished" (e.g., permission problems).
I want CLISP to return the path of the file if it was found and
deleted (or T if *ansi* is T), T if the file was not found (for any
reason whatsoever) and signal an error when the file was found but could
not be deleted.
Could you please modify your patch to do this?
(or convince me that I am wrong :-)

-- 
Sam Steingold (http://www.podval.org/~sds) running RedHat7.2 GNU/Linux
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
You can have it good, soon or cheap.  Pick two...

---

From: Peter Burwood <[email-masked]>
Date: Mon Mar 11 15:13:31 2002

> is the system one known to be deficient in any way?

Well the Cygwin version is not equivalent to the CLISP one and is
arguably broken. Perhaps years ago when realpath was written it was
unusual to find it in the library. I presume a suitable set of tests
could be written for configure to decide whether the OS implementation
is sufficient.

> you site (but not with the first one).

Yes, I ignored this, because of the t earlier.

> which cannot possibly break anything, but if you wish you may disable

I included this change because I was changing the other exit points of
delete-file. Really, this extension was okay with me.

> reason whatsoever) and signal an error when the file was found but could

Returning T if the file was not found would change current behaviour.
I also consider it misleading as no file was deleted, so delete-file
returning true would be a lie, yet it is allowed by the CLHS.  I
wonder why the CLHS is written so that delete-file returns only T and
not T or NIL?

The wish "T if the file was not found (for any reason whatsover)"
would also mean that the the only the Cygwin implementation pre my fix
in realpath was conformant and the test in excepsit.tst would need
changing from FILE-ERROR to T.

That is

  (with-open-file (s "/tmp/foo35.tmp" :direction :output))
  (delete-file "/tmp/foo35.tmp/bar")
    -> T

since there is no permissions error.  Are you happy with T here?

> (or convince me that I am wrong :-)

Well, Jorg is unhappy about the ansi addition and I abstain from it.
I also think my points above need considering before making another
change. Once we've agreed on the correct behaviour, I'll make the change.

I propose

  delete-file filespec => generalised boolean

    returns the pathname if the file was successfully deleted.
    signals FILE-ERROR if there was a permissions failure.

Which of the alternatives would you like

    returns T if the file was not found and pathname-directory was valid.
    signals FILE-ERROR if the pathname-directory was invalid.

or

    returns T if the file was not found

I cannot see why an invalid directory should be treated specially and
return FILE-ERROR while file not found returns T, so I prefer the
latter.

Peter

---

From: Sam Steingold <[email-masked]>
Date: Mon Mar 11 18:25:02 2002

> * In message <[email-masked]>
> * Sent on 11 Mar 2002 23:10:51 +0000

yes.

>     returns T if the file was not found

no.

> return FILE-ERROR while file not found returns T, so I prefer the

the invariant is that if (delete-file filespec) returned T, then the
file filespec does not exist.
thus, is we deleted it, we can return its truename.
if we looked and it was not there, we return T.
but if we looked and could not get there, we have to barf because we do
not really know what went wrong.
E.g., /foo/bar/baz: if we cannot stat bar or foo, we cannot find out
whether baz is there.

another thing is that it is a good idea not to deviate from other CL
implementations gratuitously.

CMUCL appears to barf whenever no delete action was performed for
whatever reason.

what about ACL? LW? CCL?

-- 
Sam Steingold (http://www.podval.org/~sds) running RedHat7.2 GNU/Linux
Keep Jerusalem united! <http://www.onejerusalem.org/Petition.asp>
Read, think and remember! <http://www.iris.org.il> <http://www.memri.org/>
Life is like a diaper -- short and loaded.