bug#44186: Recursive mkdir
Le mardi 27 octobre 2020 à 10:19 +0100, Leo Prikler a écrit :
> Returning #t on error won't actually
> fix them.
Do you mean that ignoring errors on mkdir when there has been a called
to mkdir-recursive just before is not OK? I agree, it's better if
mkdir-recursive fails if one element of the chain can't be created,
even if the parent exists or has been created. I updated the function.
> That would allow you
> to put mkdir-recursive into the posix module (and test it along with
> it) even if it isn't strictly POSIX.
So, that's what I did; the code is now in the posix module.
> > How do you run the tests? When I run "make check", I get 1 of 39
> > tests failed, the test-out-of-memory test. It does not even try to
> > run the ports test.
> Have a look at test-suite/Makefile.am.
(Now the relevant test is posix.test, not ports.test anymore)
I found it: I just needed to run the ./check-guile script. Now, both
recusive mkdir tests in posix.test run smoothly.
Best regards,
divoplade
From 3c43cd66b8d0d1ada76b19f0b073b7fee0c107c7 Mon Sep 17 00:00:00 2001
From: divoplade
Date: Sat, 24 Oct 2020 00:35:01 +0200
Subject: [PATCH 2/2] Use the recursive mkdir function in file output
procedures
2020-10-25 divoplade
* module/ice-9/ports.scm (open-output-file): add a mkdir keyword
to try to recursively create the directory of the output file.
* module/ice-9/ports.scm (call-with-output-file): same.
* module/ice-9/ports.scm (with-output-to-file): same.
* module/ice-9/ports.scm (with-error-to-file): same.
* doc/ref/api-io.texi: document the new keyword argument for
opening output files.
* NEWS: indicate that the open output function can now create
the filename directory if it does not exist.
---
NEWS | 6 +++-
doc/ref/api-io.texi| 38 ++---
module/ice-9/ports.scm | 75 ++
3 files changed, 71 insertions(+), 48 deletions(-)
diff --git a/NEWS b/NEWS
index 94a3f3154..09e06a7ba 100644
--- a/NEWS
+++ b/NEWS
@@ -19,7 +19,11 @@ many similar clauses whose first differentiator are constants.
** New function mkdir-recursive
This function will try and create the directory and parent directories,
-up to a directory that can be opened or the root.
+up to a directory that can be opened or the root. This behavior is
+included in open-output-file, call-with-output-file, with-output-to-file
+and with-error-to-file by adding a keyword argument `#:mkdir' which,
+when set to `#t', creates the directories before trying to open the
+file.
* Incompatible changes
diff --git a/doc/ref/api-io.texi b/doc/ref/api-io.texi
index ecbd35585..0c6beec20 100644
--- a/doc/ref/api-io.texi
+++ b/doc/ref/api-io.texi
@@ -1036,13 +1036,16 @@ for @code{open-file}. Equivalent to
@rnindex open-output-file
@deffn {Scheme Procedure} open-output-file filename @
- [#:encoding=#f] [#:binary=#f]
+ [#:encoding=#f] [#:binary=#f] [#:mkdir=#f]
Open @var{filename} for output. If @var{binary} is true, open the port
in binary mode, otherwise use text mode. @var{encoding} specifies the
-character encoding as described above for @code{open-file}. Equivalent
-to
+character encoding as described above for @code{open-file}. If
+@var{mkdir} is true, recursively create the directory of @var{filename}
+if it does not exist first. Equivalent to
@lisp
+(when @var{mkdir}
+ (mkdir-recursive (dirname @var{filename})))
(open-file @var{filename}
(if @var{binary} "wb" "w")
#:encoding @var{encoding})
@@ -1052,12 +1055,14 @@ to
@deffn {Scheme Procedure} call-with-input-file filename proc @
[#:guess-encoding=#f] [#:encoding=#f] [#:binary=#f]
@deffnx {Scheme Procedure} call-with-output-file filename proc @
-[#:encoding=#f] [#:binary=#f]
+[#:encoding=#f] [#:binary=#f] [#:mkdir=#f]
@rnindex call-with-input-file
@rnindex call-with-output-file
Open @var{filename} for input or output, and call @code{(@var{proc}
-port)} with the resulting port. Return the value returned by
-@var{proc}. @var{filename} is opened as per @code{open-input-file} or
+port)} with the resulting port. For @var{call-with-output-file}, if
+@var{mkdir} is true, create the directory of @var{filename} recursively
+if it does not exist first. Return the value returned by @var{proc}.
+@var{filename} is opened as per @code{open-input-file} or
@code{open-output-file} respectively, and an error is signaled if it
cannot be opened.
@@ -1065,22 +1070,28 @@ When @var{proc} returns, the port is closed. If @var{proc} does not
return (e.g.@: if it throws an error), then the port might not be
closed automatically, though it will be garbage collected in the usual
way if not otherwise referenced.
+
+If @var{mkdir} is true, create @var{filename}'s directory and all
+its parents.
@end deffn
@deffn {Scheme Procedure} with-input-from-file filename thunk @
[#:guess-encoding=#f] [#:encoding=#f] [#:binary=#f]
@deffnx {Scheme Procedure}
bug#44186: Recursive mkdir
Hello, Am Dienstag, den 27.10.2020, 08:01 +0100 schrieb divoplade: > I think I have made my point for the second commit. I intend this > change to be user-centric: it would be better to confuse the > developer > of a guile program than the user of said program. Anyway, this will > not > confuse anyone because creating the directory is not the default > behavior. I am very not pleased with this distinction of "user" and "developer". Even assuming there is some, it would be wiser not to confuse the latter, as then they can assure on their own terms, that the former won't be confused either. When you've reached the point, where "even" the latter can't tell you what exactly will happen, how exactly are you going to ensure the former won't be confused in the event something *does* go wrong? Spoiler warning: Returning #t on error won't actually fix them. > As for the first commit: > > Le lundi 26 octobre 2020 à 22:05 +0100, Leo Prikler a écrit : > > I'd prefer it if you didn't credit me for messing up your code. ;) > > The reason my version works, is because (ice-9 filesystem) has a > > working implementation of file-ancestors, which it uses to create > > all > > ancestors in order. The part you've copied only creates one such > > directory. With the changes you've made, the directory, that does > > get > > created, is the first one in the tree, which does not exist. I'm > > surprised, that this test passes for you, because it does not pass > > for > > me. > > Exactly, it does not pass the test, because I still can't run them. That's one way to see it. For the record, I didn't actually run your test, but instead copied your code as well as a test into a separate file rather than building all of guile. That's an easier way of prototyping. > In which file do I write the code, I feel the ports module is not the > best place. That's because it isn't. The only reason you could be led to putting it there is because of your insistence on the second patch (recall, that it is not at all needed) along with possibly a belief, that the only reason to recursively make a directory is to put files at their root, which is also wrong. As I have been suggesting multiple times already, you could potentially maybe drop your second patch without making much impact on those developers and users, who receive an error when opening a file port without ensuring the parent directory to exist. That would allow you to put mkdir-recursive into the posix module (and test it along with it) even if it isn't strictly POSIX. If that's not your cup of tea and you have more than just mkdir-recursive to add, you might want to put it into a different module. Please also note, that Guile also doesn't particularly need *your* implementation of mkdir-p (or mine for that matter). Ludovic Courtès, who you might remember being a co-maintainer along with Andy Wingo, wrote mkdir-p for GNU Guix, so whether or not it gets included here is much rather a question of whether or not they want to relicense it under the LGPL. > How do you run the tests? When I run "make check", I get 1 of 39 > tests failed, the test-out-of-memory test. It does not even try to > run the ports test. Have a look at test-suite/Makefile.am. Regards, Leo
bug#44186: Recursive mkdir
Hello,
I think I have made my point for the second commit. I intend this
change to be user-centric: it would be better to confuse the developer
of a guile program than the user of said program. Anyway, this will not
confuse anyone because creating the directory is not the default
behavior.
As for the first commit:
Le lundi 26 octobre 2020 à 22:05 +0100, Leo Prikler a écrit :
> I'd prefer it if you didn't credit me for messing up your code. ;)
> The reason my version works, is because (ice-9 filesystem) has a
> working implementation of file-ancestors, which it uses to create all
> ancestors in order. The part you've copied only creates one such
> directory. With the changes you've made, the directory, that does
> get
> created, is the first one in the tree, which does not exist. I'm
> surprised, that this test passes for you, because it does not pass
> for
> me.
Exactly, it does not pass the test, because I still can't run them.
That's one of my open questions (the other being: in which file do I
write the code, I feel the ports module is not the best place). How do
you run the tests? When I run "make check", I get 1 of 39 tests failed,
the test-out-of-memory test. It does not even try to run the ports
test.
So until I can check it, my work can only be understood as "work in
progress".
Best regards,
divoplade
From ce1eb42ab2db13235e2de71d10529daacdf9df87 Mon Sep 17 00:00:00 2001
From: divoplade
Date: Sat, 24 Oct 2020 00:35:01 +0200
Subject: [PATCH 2/2] Use the recursive mkdir function in file output
procedures
2020-10-25 divoplade
* module/ice-9/ports.scm (open-output-file): add a mkdir keyword
to try to recursively create the directory of the output file.
* module/ice-9/ports.scm (call-with-output-file): same.
* module/ice-9/ports.scm (with-output-to-file): same.
* module/ice-9/ports.scm (with-error-to-file): same.
* doc/ref/api-io.texi: document the new keyword argument for
opening output files.
* NEWS: indicate that the open output function can now create
the filename directory if it does not exist.
---
NEWS | 6 +++-
doc/ref/api-io.texi| 38 ++---
module/ice-9/ports.scm | 75 ++
3 files changed, 71 insertions(+), 48 deletions(-)
diff --git a/NEWS b/NEWS
index 94a3f3154..09e06a7ba 100644
--- a/NEWS
+++ b/NEWS
@@ -19,7 +19,11 @@ many similar clauses whose first differentiator are constants.
** New function mkdir-recursive
This function will try and create the directory and parent directories,
-up to a directory that can be opened or the root.
+up to a directory that can be opened or the root. This behavior is
+included in open-output-file, call-with-output-file, with-output-to-file
+and with-error-to-file by adding a keyword argument `#:mkdir' which,
+when set to `#t', creates the directories before trying to open the
+file.
* Incompatible changes
diff --git a/doc/ref/api-io.texi b/doc/ref/api-io.texi
index ecbd35585..0c6beec20 100644
--- a/doc/ref/api-io.texi
+++ b/doc/ref/api-io.texi
@@ -1036,13 +1036,16 @@ for @code{open-file}. Equivalent to
@rnindex open-output-file
@deffn {Scheme Procedure} open-output-file filename @
- [#:encoding=#f] [#:binary=#f]
+ [#:encoding=#f] [#:binary=#f] [#:mkdir=#f]
Open @var{filename} for output. If @var{binary} is true, open the port
in binary mode, otherwise use text mode. @var{encoding} specifies the
-character encoding as described above for @code{open-file}. Equivalent
-to
+character encoding as described above for @code{open-file}. If
+@var{mkdir} is true, recursively create the directory of @var{filename}
+if it does not exist first. Equivalent to
@lisp
+(when @var{mkdir}
+ (mkdir-recursive (dirname @var{filename})))
(open-file @var{filename}
(if @var{binary} "wb" "w")
#:encoding @var{encoding})
@@ -1052,12 +1055,14 @@ to
@deffn {Scheme Procedure} call-with-input-file filename proc @
[#:guess-encoding=#f] [#:encoding=#f] [#:binary=#f]
@deffnx {Scheme Procedure} call-with-output-file filename proc @
-[#:encoding=#f] [#:binary=#f]
+[#:encoding=#f] [#:binary=#f] [#:mkdir=#f]
@rnindex call-with-input-file
@rnindex call-with-output-file
Open @var{filename} for input or output, and call @code{(@var{proc}
-port)} with the resulting port. Return the value returned by
-@var{proc}. @var{filename} is opened as per @code{open-input-file} or
+port)} with the resulting port. For @var{call-with-output-file}, if
+@var{mkdir} is true, create the directory of @var{filename} recursively
+if it does not exist first. Return the value returned by @var{proc}.
+@var{filename} is opened as per @code{open-input-file} or
@code{open-output-file} respectively, and an error is signaled if it
cannot be opened.
@@ -1065,22 +1070,28 @@ When @var{proc} returns, the port is closed. If @var{proc} does not
return (e.g.@: if it throws an error), then the port might not be
closed automatically,
