GNU bug report logs - #44186
Recursive mkdir

Previous Next

To reply to this bug, email your comments to 44186 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: divoplade <d <at> divoplade.fr>
To: bug-guile <at> gnu.org
Cc: zimoun <zimon.toutoune <at> gmail.com>, 44130 <at> debbugs.gnu.org
Subject: Recursive mkdir
Date: Sat, 24 Oct 2020 00:00:08 +0200

[Message part 1 (text/plain, inline)]

Dear guile,

I have slightly modified the mkdir function so that it takes a third
argument to try to create the intermediate directories when calling
mkdir, in effect acting as "mkdir -p".

However, I could not make the test suite run the ports test, on which I
added the test for the new behavior. Is it expected? How do I run it?

In the mean time, here is the WIP version of the patch.

Best regards,

divoplade

Le vendredi 23 octobre 2020 à 21:36 +0200, zimoun a écrit :
> salut,
> 
> On Thu, 22 Oct 2020 at 01:29, divoplade <d <at> divoplade.fr> wrote:
> 
> > 1. Lobby guile to provide this function out of the box (this will
> > take
> > time);
> 
> This path seems the one to go.  It will take less time than running
> Guile on Windows. ;-)
> 
> The only issue is that your code will depend on Guile 3.0.x where
> x>4.  
> 
> Otherwise, why is it not possible to send a patch to Guile?

[0001-mkdir-Add-an-optional-argument-recursive-to-create-t.patch (text/x-patch, attachment)]

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: divoplade <d <at> divoplade.fr>
Cc: bug-guile <at> gnu.org
Subject: Re: Recursive mkdir
Date: Sat, 24 Oct 2020 00:07:19 +0200

Dear,

On Sat, 24 Oct 2020 at 00:00, divoplade <d <at> divoplade.fr> wrote:

> I have slightly modified the mkdir function so that it takes a third
> argument to try to create the intermediate directories when calling
> mkdir, in effect acting as "mkdir -p".
>
> However, I could not make the test suite run the ports test, on which I
> added the test for the new behavior. Is it expected? How do I run it?
>
> In the mean time, here is the WIP version of the patch.

[...]

> > Otherwise, why is it not possible to send a patch to Guile?

The discussion started here:

     <http://issues.guix.gnu.org/issue/44130>

All the best,
simon

Message #11 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: divoplade <d <at> divoplade.fr>
To: 44186 <at> debbugs.gnu.org
Subject: Re: bug#44186: Recursive mkdir
Date: Sat, 24 Oct 2020 00:46:57 +0200

[Message part 1 (text/plain, inline)]

I also added a keyword argument to the output functions so that they
can create the directory of the file if needed. What do you think?

[0002-Use-the-recursive-mkdir-function-in-file-output-proc.patch (text/x-patch, attachment)]

[0001-mkdir-Add-an-optional-argument-recursive-to-create-t.patch (text/x-patch, attachment)]

Message #14 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: Bengt Richter <bokr <at> bokr.com>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 44186 <at> debbugs.gnu.org, divoplade <d <at> divoplade.fr>
Subject: Re: bug#44186: Recursive mkdir
Date: Sat, 24 Oct 2020 01:32:14 +0200

Hi,

On +2020-10-24 00:07:19 +0200, zimoun wrote:
> Dear,
> 
> On Sat, 24 Oct 2020 at 00:00, divoplade <d <at> divoplade.fr> wrote:
> 
> > I have slightly modified the mkdir function so that it takes a third
> > argument to try to create the intermediate directories when calling
> > mkdir, in effect acting as "mkdir -p".
> >
> > However, I could not make the test suite run the ports test, on which I
> > added the test for the new behavior. Is it expected? How do I run it?
> >
> > In the mean time, here is the WIP version of the patch.
> 
> [...]
> 
> > > Otherwise, why is it not possible to send a patch to Guile?
> 
> The discussion started here:
> 
>      <http://issues.guix.gnu.org/issue/44130>
> 
> All the best,
> simon
> 
> 
> 
An alternate solution could be programmed using ffi, as documented in [1], n'est-ce pas?
What would guix best-practice guidance say about that?

[1] info guile ffi
-- 
Regards,
Bengt Richter

Message #17 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: divoplade <d <at> divoplade.fr>
To: Bengt Richter <bokr <at> bokr.com>, zimoun <zimon.toutoune <at> gmail.com>
Cc: 44186 <at> debbugs.gnu.org
Subject: Re: bug#44186: Recursive mkdir
Date: Sat, 24 Oct 2020 08:17:47 +0200

Hello,

Le samedi 24 octobre 2020 à 01:32 +0200, Bengt Richter a écrit :
> An alternate solution could be programmed using ffi, as documented in
> [1], n'est-ce pas?
To be clear, you would rather have that function as guile code rather
than extending the C function? I'm OK with that, but in which file
should I put that function? My instinct was to put the code near the
mkdir function, and that happened to be in a C file, so I went C.

> What would guix best-practice guidance say about that?
I'm not sure to follow, now this is a guile matter, guix has nothing to
do about it. I'm sorry I messed a few things up with the mailing lists
(I should have listened to them, "don't cross the streams"). Could you
elaborate?

Best regards,

divoplade

Message #20 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: Bengt Richter <bokr <at> bokr.com>
To: divoplade <d <at> divoplade.fr>
Cc: 44186 <at> debbugs.gnu.org, zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#44186: Recursive mkdir
Date: Sun, 25 Oct 2020 05:13:50 +0100

Hi divoplade,

On +2020-10-24 08:17:47 +0200, divoplade wrote:
> Hello,
> 
> Le samedi 24 octobre 2020 à 01:32 +0200, Bengt Richter a écrit :
> > An alternate solution could be programmed using ffi, as documented in
> > [1], n'est-ce pas?
> To be clear, you would rather have that function as guile code rather
> than extending the C function? I'm OK with that, but in which file
> should I put that function? My instinct was to put the code near the
> mkdir function, and that happened to be in a C file, so I went C.
>
Seems logical, and probably where I'd go, but please be careful!
Don't make a C version of this hack:
┌───────────────────────────────────────────────────────────────────────────────┐
│ (define (my-mkdir-p path . ignore) (system (string-append "mkdir -p " path))) │
└───────────────────────────────────────────────────────────────────────────────┘
You can then write (my-mkdir-p "foo-dir/bar-dir") and it'll do the job.

But it's definitely safer to skip the hack and write
┌─────────────────────────────────────┐
│ (system "mkdir -p foo-dir/bar-dir") │
└─────────────────────────────────────┘
and not give anything a chance to inject something bad via unsanitized parameters.
E.g.,
┌─────────────────────────────────────┐
│ (my-mkdir-p "here/below;tree here") │
│ here                                │
│ └── below                           │
└─────────────────────────────────────┘
It does the intended, so no suspicous change in that part, and 
my-mkdir-p looks innocent enough. (Did you notice the danger? ;)

Well, introducing something like this, but more subtle,
could be the first move by a mole  working for
<insert favorite bogeyman organization>
to compromise or disrupt/discredit GNU FLOSS competition.

Is that unhappy thought too paranoid?
I hope so, but I'm not convinced  ;/

> > What would guix best-practice guidance say about that?
> I'm not sure to follow, now this is a guile matter, guix has nothing to
> do about it. I'm sorry I messed a few things up with the mailing lists
> (I should have listened to them, "don't cross the streams"). Could you
> elaborate?
>
Sorry, you are right: it does become a guile matter, but...

... it also becomes part of guix if it's part of the guile version
a guix version depends on, so thereby it becomes a guix quality/security item
to be careful about.

So I was wondering whether guix architects have preferences
for where and how a function should be implemented, all things considered.

> Best regards,
> 
> divoplade
> 
-- 
Regards,
Bengt Richter

Message #23 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Bengt Richter <bokr <at> bokr.com>
Cc: 44186 <at> debbugs.gnu.org, bug-guile <at> gnu.org, divoplade <d <at> divoplade.fr>,
 zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#44186: Recursive mkdir
Date: Sun, 25 Oct 2020 08:45:21 +0100

Bengt Richter <bokr <at> bokr.com> writes:

> Hi divoplade,
>
> On +2020-10-24 08:17:47 +0200, divoplade wrote:
>> Hello,
>> 
>> Le samedi 24 octobre 2020 à 01:32 +0200, Bengt Richter a écrit :
>> > An alternate solution could be programmed using ffi, as documented in
>> > [1], n'est-ce pas?
>> To be clear, you would rather have that function as guile code rather
>> than extending the C function? I'm OK with that, but in which file
>> should I put that function? My instinct was to put the code near the
>> mkdir function, and that happened to be in a C file, so I went C.

We should all be writing fewer lines of C code and more Scheme :)

> Seems logical, and probably where I'd go, but please be careful!
> Don't make a C version of this hack:
> ┌───────────────────────────────────────────────────────────────────────────────┐
> │ (define (my-mkdir-p path . ignore) (system (string-append "mkdir -p " path))) │
> └───────────────────────────────────────────────────────────────────────────────┘
> You can then write (my-mkdir-p "foo-dir/bar-dir") and it'll do the job.
>
> But it's definitely safer to skip the hack and write
> ┌─────────────────────────────────────┐
> │ (system "mkdir -p foo-dir/bar-dir") │
> └─────────────────────────────────────┘
> and not give anything a chance to inject something bad via unsanitized
> parameters.

Use “system*” instead of “system” when you need to work with
user-provided values.  For mkdir-p, however, using any variant of
“system” is inelegant.

-- 
Ricardo

Message #29 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: divoplade <d <at> divoplade.fr>
To: Ricardo Wurmus <rekado <at> elephly.net>, Bengt Richter <bokr <at> bokr.com>
Cc: 44186 <at> debbugs.gnu.org, zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#44186: Recursive mkdir
Date: Sun, 25 Oct 2020 10:07:17 +0100

[Message part 1 (text/plain, inline)]

Hello!

Le dimanche 25 octobre 2020 à 08:45 +0100, Ricardo Wurmus a écrit :
> > > To be clear, you would rather have that function as guile code
> > > rather
> > > than extending the C function? I'm OK with that, but in which
> > > file
> > > should I put that function? My instinct was to put the code near
> > > the
> > > mkdir function, and that happened to be in a C file, so I went C.
> 
> We should all be writing fewer lines of C code and more Scheme :)

OK, but where do I put the function? There is a posix.scm, but this
function is not posix, and posix.scm does not seem to export functions.

By default I will put it in ports.scm, because it is useful for open-
output-directory.

Also, I still cannot run the ports tests! How do I do it? Also test-
out-of-memory fails, but it also fails on master.

Attached, the new version for you to read!

Best regards,

divoplade

[0002-Use-the-recursive-mkdir-function-in-file-output-proc.patch (text/x-patch, attachment)]

[0001-ports-Add-mkdir-recursive.patch (text/x-patch, attachment)]

Message #32 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: divoplade <d <at> divoplade.fr>
To: Ricardo Wurmus <rekado <at> elephly.net>, Bengt Richter <bokr <at> bokr.com>
Cc: 44186 <at> debbugs.gnu.org, zimoun <zimon.toutoune <at> gmail.com>
Subject: Re: bug#44186: Recursive mkdir
Date: Sun, 25 Oct 2020 11:44:46 +0100

[Message part 1 (text/plain, inline)]

So, after a bit of bikeshedding on #guile, it turns out that the
controversy moved to the second commit.

Here is the justification for it.

When a program user wants to save data to a file, if the place where to
save the file does not exist yet, there will be an error: "cannot
create file or directory". That's puzzling to the user, because, yes,
the user wants to create that file.  If the error is a little more
precise, it will be something in the line of "Please create directory
blah/blah/blah before invoking this program".

So, the user will wonder why the program was not able to create
blah/blah/blah itself, create it, and re-run the program. This is more
work for the user, work that could have been easily handled by the
program.

Good behaving programs should (recursively) create the directory before
trying to write to a file specified by the user. That include log files
for a daemon, for instance. Emacs org-mode babel tangling uses a
:mkdirp t for a similar reason. In order to simplify the development of
such programs, and in order to avoid bugs where the developer forgot to
call (mkdir-recursive (dirname output-file)) before (open-output-file,
call-with-output-file or with-output-to-file, while still keeping
compatibility of the other programs, I propose to add a keyword
argument to these functions. 

There are also worries that this keyword will not be of much use and
will clutter the implementation. I am willing to bet that this keyword
will be more used than, for instance, #:guess-encoding, which is
available in all the corresponding input functions.

I also simplified the mkdir-recursive function, to be closer to 
https://gitlab.com/leoprikler/guile-filesystem/-/blob/master/ice-9/filesystem.scm
.

[0002-Use-the-recursive-mkdir-function-in-file-output-proc.patch (text/x-patch, attachment)]

[0001-ports-Add-mkdir-recursive.patch (text/x-patch, attachment)]

Message #35 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: d <at> divoplade.fr
Cc: 44186 <at> debbugs.gnu.org
Subject: bug#44186: Recursive mkdir
Date: Mon, 26 Oct 2020 22:05:43 +0100

Hello, divoplade,

> So, after a bit of bikeshedding on #guile, it turns out that the
> controversy moved to the second commit.
Not quite, the second commit just needlessly complicates a patch, that
should not be that complicated in the first place.
When posting a patch to the mailing list, you should do your best to
ensure, that all of your code works as intended (alas, I am getting
ahead of myself here a bit).  It is easier to prove this for *just*
mkdir-p/mkdir-recursive rather than having to test all your wrappers as
well.  You have been pretty adamant, that you need those wrappers, but
they can easily be written in whichever library actually ends up using
them as well, so you're just creating more work for yourself for little
gain.  That's the point I was trying to make in the IRC.

> Here is the justification for it.
> 
> When a program user wants to save data to a file, if the place where
> to
> save the file does not exist yet, there will be an error: "cannot
> create file or directory". That's puzzling to the user, because, yes,
> the user wants to create that file.  If the error is a little more
> precise, it will be something in the line of "Please create directory
> blah/blah/blah before invoking this program".
> 
> So, the user will wonder why the program was not able to create
> blah/blah/blah itself, create it, and re-run the program. This is
> more
> work for the user, work that could have been easily handled by the
> program.
That would be a nice story in a vacuum, but in practice few systems
work like that.  Python errors in a similar way to Guile, but with a
nicer message.  So does plain Node JS.  On the part of Node with all of
its single package modules, even there the equivalent to your versions
of open-output-file is one package[1] removed from the built-in fs
module.

> Good behaving programs should (recursively) create the directory
> before
> trying to write to a file specified by the user. That include log
> files
> for a daemon, for instance. Emacs org-mode babel tangling uses a
> :mkdirp t for a similar reason. In order to simplify the development
> of
> such programs, and in order to avoid bugs where the developer forgot
> to
> call (mkdir-recursive (dirname output-file)) before (open-output-
> file,
> call-with-output-file or with-output-to-file, while still keeping
> compatibility of the other programs, I propose to add a keyword
> argument to these functions. 
The programming of such procedures would not get simpler by inlining
mkdir-p into open-file.  Instead, programs written that way would be
harder to understand, as the implicit creation of such directories
outside of functions that explicitly exist for this implicit creation
will cause (some) confusion as to whether or not implicit creation of
directories can/will take place and whether that is actually wanted in
this context and erroring is not an acceptable alternative.

The addition of mkdir-p/mkdir-recursive/make-directories alone would
already enable a writer of org-mode tangle to write whichever helper
they need for their (in my opinion rather specific) use case in 2
lines. 

> I also simplified the mkdir-recursive function, to be closer to 
> https://gitlab.com/leoprikler/guile-filesystem/-/blob/master/ice-9/filesystem.scm
> .
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.

On a somewhat related note, I am getting closer to a 0.1.0 release of
(ice-9 filesystem), which I'll then pitch to the Guix mailing lists. 
Being an outside module, that means you can use it with Guile versions
earlier than 3.0.5, which at least in my eyes sounds like a better
solution to your problem of not being able to implement org-babel in
Guile.

Regards, Leo

[1] https://www.npmjs.com/package/fse

Message #38 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: divoplade <d <at> divoplade.fr>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 44186 <at> debbugs.gnu.org
Subject: Re: bug#44186: Recursive mkdir
Date: Tue, 27 Oct 2020 08:01:23 +0100

[Message part 1 (text/plain, inline)]

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

[0002-Use-the-recursive-mkdir-function-in-file-output-proc.patch (text/x-patch, attachment)]

[0001-ports-Add-mkdir-recursive.patch (text/x-patch, attachment)]

Message #41 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: divoplade <d <at> divoplade.fr>
Cc: 44186 <at> debbugs.gnu.org
Subject: Re: bug#44186: Recursive mkdir
Date: Tue, 27 Oct 2020 10:19:50 +0100

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

Message #44 received at 44186 <at> debbugs.gnu.org (full text, mbox):

From: divoplade <d <at> divoplade.fr>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 44186 <at> debbugs.gnu.org
Subject: Re: bug#44186: Recursive mkdir
Date: Tue, 27 Oct 2020 11:21:03 +0100

[Message part 1 (text/plain, inline)]

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

[0002-Use-the-recursive-mkdir-function-in-file-output-proc.patch (text/x-patch, attachment)]

[0001-ports-Add-mkdir-recursive.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.