Hi all,

As Vasilij already reported, there are two failing tests under mingw.
They initially fail for the same reason - they try to delete files
that have been opened but not closed, which is an error under Windows.

After fixing this, the posix-tests fail due to the fact that Windows
doesn't fully support file modes.  It doesn't know about execute
permissions, and doesn't support making files write-only or having
files with no permissions.

To fix that, we can conceptually "right-extend" the user permissions
over group and others after adding read permissions (as Windows does
not have files without read permissions) and dropping execute
permissions.  In code, I cond-expand on Windows, shift the group and
other bits out to obtain the user bits, and then dispatch on the values
to get read and write access or read-only access.

With these patches in place, the tests pass once again on mingw-msys.

Why we never noticed before?  The POSIX tests were extended in ffe553,
and the read-lines-test was added in cfa1e7, both of which were added
somewhere between 5.2.0 and 5.3.0rc1.  And on *nix these tests work
fine, of course...

Cheers,
Peter
From 00f5fafd86f625c25231688a1b975c69bff630d1 Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Thu, 2 Sep 2021 08:03:39 +0200
Subject: [PATCH 1/2] Ensure all ports are closed in tests when deleting the
 file

Under Windows, read-lines-test.scm and posix-tests.scm would
fail with "permission denied" upon deletion of the output file,
but that's because there was still an open file handle to it
(and on Windows you can't delete open files).

In read-lines-test.scm, it is my favorite Scheme footgun:
call-with-input-file doesn't close its port when the dynamic
extent is exited via other ways than a regular procedure return.

In posix-tests.scm it was a more mundane error - we opened a
file using the descriptor-returning POSIX procedure file-open,
but never closed the descriptor.
---
 tests/posix-tests.scm      | 2 +-
 tests/read-lines-tests.scm | 7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/posix-tests.scm b/tests/posix-tests.scm
index 361f55c1..26f495e7 100644
--- a/tests/posix-tests.scm
+++ b/tests/posix-tests.scm
@@ -98,7 +98,7 @@
                          (let ((mode (file-creation-mode)))
                            (set! (file-creation-mode) umask)
                            (delete-file* "posix-tests.out")
-                           (file-open "posix-tests.out" open/creat given ...)
+                           (file-close (file-open "posix-tests.out" open/creat given ...))
                            (assert (equal? (file-permissions "posix-tests.out") expected))
                            (set! (file-creation-mode) mode))))))
   ;; default file mode
diff --git a/tests/read-lines-tests.scm b/tests/read-lines-tests.scm
index 28cdd223..e107c26f 100644
--- a/tests/read-lines-tests.scm
+++ b/tests/read-lines-tests.scm
@@ -46,8 +46,11 @@ EOF
 (test-error
  "with an invalid second argument (max)"
  (call-with-input-file input-test-file
-  (lambda (port)
-    (read-lines port 2.0))))
+   (lambda (port)
+     (dynamic-wind
+	 void
+	 (lambda () (read-lines port 2.0))
+	 (lambda () (close-input-port port))))))
 
 (test-end "read-lines")
 
-- 
2.31.1

From 9ce043d50ea6e7005bf2a4111134fada84fac5e3 Mon Sep 17 00:00:00 2001
From: Peter Bex <pe...@more-magic.net>
Date: Thu, 2 Sep 2021 09:44:13 +0200
Subject: [PATCH 2/2] Fix posix-tests file permission test on Windows

In Windows, there are three problems with this test:

- Deleting read-only files gives a permission error.
- Permissions don't know about "group" and "other" permissions - the
  permissions are always extended from the user so the rest match it.
- It is impossible to give write-only or "no" permissions on a file.

So we make files writable before attempting to delete them, and
we provide a mapping for expected permissions so that we only look
at the user part of the file mode, and always have it include
"readable" as a permission.

See also
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/umask
and
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/remove-wremove
---
 tests/posix-tests.scm | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/tests/posix-tests.scm b/tests/posix-tests.scm
index 26f495e7..1c695d9d 100644
--- a/tests/posix-tests.scm
+++ b/tests/posix-tests.scm
@@ -1,4 +1,5 @@
-(import (chicken pathname)
+(import (chicken bitwise)
+	(chicken pathname)
         (chicken file)
         (chicken file posix)
         (chicken platform)
@@ -89,17 +90,38 @@
 (assert (not (get-environment-variable "FOO")))
 
 ;; file creation and umask interaction
+(define (permission-expectation original-expectation)
+  (cond-expand
+    ;; In Windows, all files are always readable.  You cannot give
+    ;; write-only permissions or no permissions.  Also, there's no
+    ;; concept of "group" and "other", so we must take the user
+    ;; permissions and extend those over the rest.  Finally, it
+    ;; doesn't have an "execute" bit, so ignore that too.
+    (windows (case (arithmetic-shift original-expectation -6)
+	       ((6 7 3 2) #o666)
+	       (else #o444)))
+    (else original-expectation)))
+
+;; For windows, the file must be writable before it can be deleted!
+(define (delete-maybe-readonly-file filename)
+  (cond-expand
+    (windows (when (file-exists? filename)
+	       (set-file-permissions! filename #o666)))
+    (else))
+  (delete-file* filename))
+
 (letrec-syntax ((test (syntax-rules ()
                         ((test umask expected)
                          (test umask "expected" expected "given"))
                         ((test umask given expected)
                          (test umask "expected" expected "given" given))
                         ((test umask "expected" expected "given" given ...)
-                         (let ((mode (file-creation-mode)))
+                         (let ((mode (file-creation-mode))
+			       (exp-perm (permission-expectation expected)))
                            (set! (file-creation-mode) umask)
-                           (delete-file* "posix-tests.out")
+                           (delete-maybe-readonly-file "posix-tests.out")
                            (file-close (file-open "posix-tests.out" open/creat given ...))
-                           (assert (equal? (file-permissions "posix-tests.out") expected))
+                           (assert (equal? (file-permissions "posix-tests.out") exp-perm))
                            (set! (file-creation-mode) mode))))))
   ;; default file mode
   (test #o000 #o666)
-- 
2.31.1

Attachment: signature.asc
Description: PGP signature

Reply via email to