Bug #105935 for File-Path: New warning in CPAN.pm with File-Path-2.11

Sat Jul 18 15:01:14 2015 SREZIC [...] cpan.org - Ticket created

CC:	ANDK [...] cpan.org
Subject:	New warning in CPAN.pm with File-Path-2.11

After File-Path was upgraded to the new version 2.11 CPAN.pm started to emit a new warning in CPAN::Distribution: ... CPAN: Compress::Zlib loaded ok (v2.068) Checksum for /home/cpansand/.cpan/sources/authors/id/E/ET/ETHER/Dist-Zilla-Plugin-StaticInstall-0.005.tar.gz ok tmp-59267 for tmp-59267: Datei oder Verzeichnis nicht gefunden at /usr/perl5.20.0/lib/5.20.0/CPAN/Distribution.pm line 468. CPAN: File::Temp loaded ok (v0.2304) ... ("Datei oder Verzeichnis nicht gefunden" = "File or directory not found") I did not check if this is a regression in File::Path or legal new warning. CC to Andreas.

Sun Jul 19 08:37:30 2015 jkeenan [...] cpan.org - Correspondence added

On Sat Jul 18 15:01:14 2015, SREZIC wrote: Show quoted text

> After File-Path was upgraded to the new version 2.11 CPAN.pm started > to emit a new warning in CPAN::Distribution: > > ... > CPAN: Compress::Zlib loaded ok (v2.068) > Checksum for /home/cpansand/.cpan/sources/authors/id/E/ET/ETHER/Dist- > Zilla-Plugin-StaticInstall-0.005.tar.gz ok > tmp-59267 for tmp-59267: Datei oder Verzeichnis nicht gefunden at > /usr/perl5.20.0/lib/5.20.0/CPAN/Distribution.pm line 468. > CPAN: File::Temp loaded ok (v0.2304) > ... > > ("Datei oder Verzeichnis nicht gefunden" = "File or directory not > found") > > I did not check if this is a regression in File::Path or legal new > warning. CC to Andreas.

At least on my Ubuntu Linux system, that error message is associated with $! having a value of 2. So this suggests that a warning that incorporates $! is being exercised. But that's as much as I can say for now. More details would be appreciated. Thank you very much.

Sun Jul 19 08:37:31 2015 The RT System itself - Status changed from 'new' to 'open'

Sun Jul 19 19:30:33 2015 ether [...] cpan.org - Correspondence added

RT-Send-CC:

ANDK [...] cpan.org

On 2015-07-19 05:37:30, JKEENAN wrote: Show quoted text

> At least on my Ubuntu Linux system, that error message is associated > with $! having a value of 2. So this suggests that a warning that > incorporates $! is being exercised. > > But that's as much as I can say for now. More details would be > appreciated. > > Thank you very much.

errno 2: ENOENT: No such file or directory But this isn't really new information, given the original error message :)

Sun Jul 19 19:31:43 2015 ether [...] cpan.org - Correspondence added

On 2015-07-19 16:30:33, ETHER wrote: Show quoted text

> On 2015-07-19 05:37:30, JKEENAN wrote: >

> > At least on my Ubuntu Linux system, that error message is associated > > with $! having a value of 2. So this suggests that a warning that > > incorporates $! is being exercised. > > > > But that's as much as I can say for now. More details would be > > appreciated. > > > > Thank you very much.

> > > errno 2: ENOENT: No such file or directory > > But this isn't really new information, given the original error message :)

This looks to be the same issue: http://perl5.git.perl.org/perl.git/commitdiff/bfab0499

Sun Jul 19 19:54:03 2015 jkeenan [...] cpan.org - Correspondence added

On Sun Jul 19 19:31:43 2015, ETHER wrote: Show quoted text

> On 2015-07-19 16:30:33, ETHER wrote:

> > On 2015-07-19 05:37:30, JKEENAN wrote: > >

> > > At least on my Ubuntu Linux system, that error message is > > > associated > > > with $! having a value of 2. So this suggests that a warning that > > > incorporates $! is being exercised. > > > > > > But that's as much as I can say for now. More details would be > > > appreciated. > > > > > > Thank you very much.

> > > > > > errno 2: ENOENT: No such file or directory > > > > But this isn't really new information, given the original error > > message :)

> > This looks to be the same issue: > http://perl5.git.perl.org/perl.git/commitdiff/bfab0499

Well, in this case at least, the fact that Chris merged 2.11 into blead so quickly was a good thing, because the failure which appeared was, IMO, insufficiently precise coding (which he has now corrected). Thank you very much. Jim Keenan

Mon Jul 20 00:45:52 2015 andreas.koenig.7os6VVqR [...] franz.ak.mind.de - Correspondence added

CC:	SREZIC [...] cpan.org, ANDK [...] cpan.org
Subject:	Re: [rt.cpan.org #105935] New warning in CPAN.pm with File-Path-2.11
Date:	Mon, 20 Jul 2015 06:45:26 +0200
To:	bug-File-Path [...] rt.cpan.org
From:	Andreas Koenig <andreas.koenig.7os6VVqR [...] franz.ak.mind.de>

Show quoted text

>>>>> On Sun, 19 Jul 2015 19:54:08 -0400, "James E Keenan via RT" <bug-File-Path@rt.cpan.org> said:

Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=105935 > > On Sun Jul 19 19:31:43 2015, ETHER wrote:

>> On 2015-07-19 16:30:33, ETHER wrote:

>> > On 2015-07-19 05:37:30, JKEENAN wrote: >> >

>> > > At least on my Ubuntu Linux system, that error message is >> > > associated >> > > with $! having a value of 2. So this suggests that a warning that >> > > incorporates $! is being exercised. >> > > >> > > But that's as much as I can say for now. More details would be >> > > appreciated. >> > > >> > > Thank you very much.

>> > >> > >> > errno 2: ENOENT: No such file or directory >> > >> > But this isn't really new information, given the original error >> > message :)

>> >> This looks to be the same issue: >> http://perl5.git.perl.org/perl.git/commitdiff/bfab0499

Show quoted text

> Well, in this case at least, the fact that Chris merged 2.11 into > blead so quickly was a good thing, because the failure which > appeared was, IMO, insufficiently precise coding (which he has now > corrected).

I'm not sure I can follow. Before I start to raise an eyebrow, are you saying that rmtree($dir); is less precice than rmtree($dir) if -d $dir; ? Even given that the former has never issued a warning if $dir did not exist? -- andreas

Mon Jul 20 05:42:42 2015 RICHE [...] cpan.org - Correspondence added

Show quoted text

> I'm not sure I can follow. Before I start to raise an eyebrow, are you > saying that > > rmtree($dir); > > is less precice than > > rmtree($dir) if -d $dir; > > ? Even given that the former has never issued a warning if $dir did > not > exist?

I don't mean to hijack your conversation with Jim, but in the context of that unit test the latter is certainly more precise. In a clean environment, that line doesn't look to be necessary. But if someone is running the test over and over again, then the directory could exist if the script dies in the middle and doesn't run the last line. So... there should be a condition. Simply speaking, we are talking about the principle of least surprise (especially for the general user). The documentation states: The "remove_tree" function deletes the given directories and any files and subdirectories they might contain ... However the documentation continues: .... much like the Unix command "rm -r" or the Windows commands "rmdir /s" and "rd /s". Which actually does confuse things. "rm -r" will certainly remove a file as well as a directory (although not a , whereas the "Windows equivalent" will not. So ... Jim, we should clarify that. The remove_tree / retree functions expect a directory. If a user's routine passes in a non-existent directory (or a non-directory), what should be the expected behavior? To me, being silent about it is far more surprising -- and could be really frustrating for a general user thinking it's working fine when it's not.

Mon Jul 20 15:43:04 2015 andreas.koenig.7os6VVqR [...] franz.ak.mind.de - Correspondence added

CC:	SREZIC [...] cpan.org, ANDK [...] cpan.org
Subject:	Re: [rt.cpan.org #105935] New warning in CPAN.pm with File-Path-2.11
Date:	Mon, 20 Jul 2015 21:42:43 +0200
To:	bug-File-Path [...] rt.cpan.org
From:	Andreas Koenig <andreas.koenig.7os6VVqR [...] franz.ak.mind.de>

Show quoted text

>>>>> On Mon, 20 Jul 2015 05:42:48 -0400, "Richard Elberger via RT" <bug-File-Path@rt.cpan.org> said:

Show quoted text

> I don't mean to hijack your conversation with Jim, but in the > context of that unit test the latter is certainly more precise. In a > clean environment, that line doesn't look to be necessary. But if > someone is running the test over and over again, then the directory > could exist if the script dies in the middle and doesn't run the > last line. So... there should be a condition.

Thank you for chiming in. Show quoted text

> Simply speaking, we are talking about the principle of least > surprise (especially for the general user). The documentation > states:

Show quoted text

> The "remove_tree" function deletes the given directories and any files > and subdirectories they might contain ...

The question boils down to: when the documentation of old code does not perfectly well match the implementation, what should be changed, the documentation or the code? This can often be argued both ways. If the code is crap, fix it. If the code is good, fix the documentation. If the code has a record of keeping people happy for over 15 years, fix the docs. I'm sure not everybody was always happy about it, but if there is an itch to change code that old, one probably has to do some homework of arguing before editing the code. Show quoted text

> However the documentation continues:

Show quoted text

> .... much like the Unix command "rm -r" or the Windows > commands "rmdir /s" and "rd /s".

The docs are vague, fixing them is not a bad idea. Changing the code is. Show quoted text

> Which actually does confuse things. "rm -r" will certainly remove a > file as well as a directory (although not a , whereas the "Windows > equivalent" will not. So ... Jim, we should clarify that.

Better clarify in the docs what the code actually does: if possible without changing the code. If not possible for whatever reason, discuss the change widely, link the docs and the Changes file to the discussion that took place. If code is old, feelings towards a principle of least surprise should probably be weighed in more carefully and held against support for those who actually use this code for 20 years. Show quoted text

> The remove_tree / retree functions expect a directory. If a user's > routine passes in a non-existent directory (or a non-directory), > what should be the expected behavior?

Show quoted text

> To me, being silent about it is far more surprising -- and could be > really frustrating for a general user thinking it's working fine > when it's not.

It has been that way for, err, [...let me look it up...] Between perl-5.001n and perl-5.002 you can find this change: commit 4633a7c4bad06b471d9310620b7fe8ddd158cccd Author: Larry Wall <lwall@scalpel.netlabs.com> Date: Tue Nov 21 10:01:00 1995 +1200 Unless I'm misreading things (I did not test it): since then it's basically just the way that now has been changed. -- andreas

Wed Jul 22 16:31:09 2015 RICHE [...] cpan.org - Correspondence added

Hello Andreas, Show quoted text

> > The "remove_tree" function deletes the given directories and any > > files > > and subdirectories they might contain ...

> > The question boils down to: when the documentation of old code does > not > perfectly well match the implementation, what should be changed, the > documentation or the code? > > This can often be argued both ways. If the code is crap, fix it. If > the > code is good, fix the documentation. If the code has a record of > keeping > people happy for over 15 years, fix the docs. I'm sure not everybody > was > always happy about it, but if there is an itch to change code that > old, > one probably has to do some homework of arguing before editing the > code.

This was a result of an RT I fixed at the NYC hackathon. It was found the error handling was inconsistent and there was no inline code documentation explaining why. In such case, it was identified as an oversight. https://rt.cpan.org/Ticket/Display.html?id=39949 Show quoted text

>

> > However the documentation continues:

>

> > .... much like the Unix command "rm -r" or the Windows > > commands "rmdir /s" and "rd /s".

> > The docs are vague, fixing them is not a bad idea. Changing the code > is.

This has been clarified on HEAD and it also now states an error occurs on malformed named directory (malformed including empty string). Show quoted text

>

> > Which actually does confuse things. "rm -r" will certainly remove a > > file as well as a directory (although not a , whereas the "Windows > > equivalent" will not. So ... Jim, we should clarify that.

> > Better clarify in the docs what the code actually does: if possible > without changing the code. If not possible for whatever reason, > discuss > the change widely, link the docs and the Changes file to the > discussion > that took place. If code is old, feelings towards a principle of least > surprise should probably be weighed in more carefully and held against > support for those who actually use this code for 20 years. >

It is now clarified. Note the fix was mentioned in the Changes file for 2.10_001. Show quoted text

> > The remove_tree / retree functions expect a directory. If a user's > > routine passes in a non-existent directory (or a non-directory), > > what should be the expected behavior?

>

> > To me, being silent about it is far more surprising -- and could be > > really frustrating for a general user thinking it's working fine > > when it's not.

> > It has been that way for, err, [...let me look it up...] > > Between perl-5.001n and perl-5.002 you can find this change: > > commit 4633a7c4bad06b471d9310620b7fe8ddd158cccd > Author: Larry Wall <lwall@scalpel.netlabs.com> > Date: Tue Nov 21 10:01:00 1995 +1200 > > Unless I'm misreading things (I did not test it): since then it's > basically just the way that now has been changed.

Because it hasn't been changed for 20 years does not mean people are not happy about it. Certainly, neither of us can quantify this fact so I don't think it's a point that carries weight. As you are surely aware, there are now several options for File::Path like functionality. Most users will simply move on if they find its functionality flaky. The bottom line is: try doing "rm -r ''" on the command line. If you get a silent response across a majority of operating environments, I will surely reconsider. In such case, for now, this RT will be rejected.

Wed Jul 22 16:31:11 2015 RICHE [...] cpan.org - Status changed from 'open' to 'rejected'

Thu Jul 23 17:03:42 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-07-20 05:42:42, RICHE wrote: Show quoted text

> > I'm not sure I can follow. Before I start to raise an eyebrow, are > > you > > saying that > > > > rmtree($dir); > > > > is less precice than > > > > rmtree($dir) if -d $dir; > > > > ? Even given that the former has never issued a warning if $dir did > > not > > exist?

> > I don't mean to hijack your conversation with Jim, but in the context > of that unit test the latter is certainly more precise. In a clean > environment, that line doesn't look to be necessary. But if someone is > running the test over and over again, then the directory could exist > if the script dies in the middle and doesn't run the last line. So... > there should be a condition.

Sorry, I have to reopen this issue. There must not be a condition. With the requirement for a condition you create a race condition. Show quoted text

> Simply speaking, we are talking about the principle of least surprise > (especially for the general user). The documentation states: > > The "remove_tree" function deletes the given directories and any files > and subdirectories they might contain ... > > > However the documentation continues: > > .... much like the Unix command "rm -r" or the Windows commands "rmdir > /s" and "rd /s". > > Which actually does confuse things. "rm -r" will certainly remove a > file as well as a directory (although not a , whereas the "Windows > equivalent" will not. So ... Jim, we should clarify that. > > The remove_tree / retree functions expect a directory. If a user's > routine passes in a non-existent directory (or a non-directory), what > should be the expected behavior? > > To me, being silent about it is far more surprising -- and could be > really frustrating for a general user thinking it's working fine when > it's not.

I assert that most users are not interested in the process of actually deleting a subtree. There are interested in having a situation where the given path does not exist anymore after calling the rmtree function, regardless whether it did not exist before or not. In a Unix shell one would more often write "rm -rf" (or just "rm -r", but ignoring the exit code and redirecting stderr to /dev/null) than plain "rm -r" and checking for the exit code. Currently I don't see how this behavior can be achieved with old-style rmtree() calls, and even with new-style remove_tree() it's not clear (the ERROR HANDLING section reads like one should not use the error feature at all).

Thu Jul 23 17:03:44 2015 SREZIC [...] cpan.org - Status changed from 'rejected' to 'open'

Thu Jul 23 17:09:48 2015 jkeenan [...] cpan.org - Correspondence added

On Thu Jul 23 17:03:42 2015, SREZIC wrote: Show quoted text

> On 2015-07-20 05:42:42, RICHE wrote:

> > > I'm not sure I can follow. Before I start to raise an eyebrow, are > > > you > > > saying that > > > > > > rmtree($dir); > > > > > > is less precice than > > > > > > rmtree($dir) if -d $dir; > > > > > > ? Even given that the former has never issued a warning if $dir did > > > not > > > exist?

> > > > I don't mean to hijack your conversation with Jim, but in the context > > of that unit test the latter is certainly more precise. In a clean > > environment, that line doesn't look to be necessary. But if someone > > is > > running the test over and over again, then the directory could exist > > if the script dies in the middle and doesn't run the last line. > > So... > > there should be a condition.

> > Sorry, I have to reopen this issue. There must not be a condition. > With the requirement for a condition you create a race condition. >

Can you illustrate your point about a race condition? Thank you very much. Jim Keenan

Thu Jul 23 17:14:46 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-07-23 17:09:48, JKEENAN wrote: Show quoted text

> On Thu Jul 23 17:03:42 2015, SREZIC wrote:

> > On 2015-07-20 05:42:42, RICHE wrote:

> > > > I'm not sure I can follow. Before I start to raise an eyebrow, are > > > > you > > > > saying that > > > > > > > > rmtree($dir); > > > > > > > > is less precice than > > > > > > > > rmtree($dir) if -d $dir; > > > > > > > > ? Even given that the former has never issued a warning if $dir did > > > > not > > > > exist?

> > > > > > I don't mean to hijack your conversation with Jim, but in the context > > > of that unit test the latter is certainly more precise. In a clean > > > environment, that line doesn't look to be necessary. But if someone > > > is > > > running the test over and over again, then the directory could exist > > > if the script dies in the middle and doesn't run the last line. > > > So... > > > there should be a condition.

> > > > Sorry, I have to reopen this issue. There must not be a condition. > > With the requirement for a condition you create a race condition. > >

> > Can you illustrate your point about a race condition?

You test and do something in two steps. This is not an atomic operation. Another process which is doing the same can step in between and delete the directory earlier, causing the first process to emit a warning.

Thu Jul 23 21:06:12 2015 RICHE [...] cpan.org - Correspondence added

Show quoted text

> You test and do something in two steps. This is not an atomic > operation. Another process which is doing the same can step in between > and delete the directory earlier, causing the first process to emit a > warning.

That can be demonstrated manually on the command line using gnu fileutils, which would also emit an error. Like the documentation says, it behaves similarly to "rm -r". Also, if you mean race condition in terms of threads, it's now documented that File::Path is not thread safe. Finally, I don't know of any language that would not error or raise an exception in this case. So unless there is some fundamental reason other than history I don't see the point.

Fri Jul 24 00:58:43 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-07-23 21:06:12, RICHE wrote: Show quoted text

>

> > You test and do something in two steps. This is not an atomic > > operation. Another process which is doing the same can step in > > between > > and delete the directory earlier, causing the first process to emit a > > warning.

> > That can be demonstrated manually on the command line using gnu > fileutils, which would also emit an error. Like the documentation > says, it behaves similarly to "rm -r". > > Also, if you mean race condition in terms of threads, it's now > documented that File::Path is not thread safe.

No, I don't think about threads. I am thinking about independent processes operating on the same directory (yes, I can construct use cases, I hope you can, too). Show quoted text

> Finally, I don't know of any language that would not error or raise an > exception in this case. So unless there is some fundamental reason > other than history I don't see the point.

You are ignoring my point about "rm -rf" being more common than "rm -r".

Fri Jul 24 08:26:58 2015 RICHE [...] cpan.org - Correspondence added

Show quoted text

> You are ignoring my point about "rm -rf" being more common than "rm > -r".

That does not highlight a bug. You are actually requesting a feature here. If you want "rm -rf", and Jim and I agree to that feature, then we would implement a key that triggers that behavior, i.e. "{ force => 1}".

Fri Jul 24 09:57:07 2015 RICHE [...] cpan.org - Correspondence added

Github enhancement has been filed. https://github.com/rpcme/File-Path/issues/30 The warning will not be removed. This issue is rejected.

Fri Jul 24 09:57:14 2015 RICHE [...] cpan.org - Status changed from 'open' to 'rejected'

Mon Jul 27 02:13:09 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-07-24 08:26:58, RICHE wrote: Show quoted text

>

> > You are ignoring my point about "rm -rf" being more common than "rm > > -r".

> > > That does not highlight a bug. You are actually requesting a feature > here.

No, I am requesting the restoration of the old behavior, like it was in the last 20 years. It would be much easier to amend the documentation that rmtree() behaves like "rm -rf". Still I don't know how to use rmtree warning-free in the current version. Do we have to wait for the new option? When will it be implemented? How should users deal with the situation if they want to write code which is compatible across the different File::Path versions? Something like this: if ($File::Path::VERSION >= ???) { rmtree($path, { force => 1 }); } elsif ($File::Path::VERSION >= 2.11) { local $SIG{__WARN__} = sub {}; rmtree($path); } else { rmtree($path); } ?

Mon Jul 27 05:55:02 2015 RICHE [...] cpan.org - Correspondence added

Show quoted text

> No, I am requesting the restoration of the old behavior, like it was > in the last 20 years. It would be much easier to amend the > documentation that rmtree() behaves like "rm -rf". > > Still I don't know how to use rmtree warning-free in the current > version. Do we have to wait for the new option? When will it be > implemented? How should users deal with the situation if they want to > write code which is compatible across the different File::Path > versions? Something like this: > > if ($File::Path::VERSION >= ???) { > rmtree($path, { force => 1 }); > } elsif ($File::Path::VERSION >= 2.11) { > local $SIG{__WARN__} = sub {}; > rmtree($path); > } else { > rmtree($path); > } > > ?

You really are a proponent for technical debt, huh? Pulling the 20 year card will never work here. Behaving like "rm -rf" _is_ a feature. File::Path was _never_ warning OR error free. It was just the case that the directory could not be stat that was missed. Like I said, the 'force' option will be considered, and it will probably be implemented for the next version.

Mon Jul 27 19:01:03 2015 rjbs [...] cpan.org - Correspondence added

I wrote a longish response to this on another ticket by mistake, but I'm not going to repost it here, because subsequent to that, I got a somewhat better handle on what is going on, and so I'll start afresh. It has long been the case that you can write: my $removed = rmtree(...); ...and any errors removing anything are warned. If you don't want to have them warned, you capture them. Andreas and Slaven are complaining, not about wanting "rm -rf" but specifically about the case where there is nothing to delete. That is, if you can't delete something because of permissions: bad. If you can't delete something because it does not exist: oh well. I think this is probably pretty reasonable, given how things have worked forever. You can go see my misdirected pean on this on #39949 if you really want to, but basically: people will be angry at these new warnings, and this ticket is just the tip of the iceberg. I tried to understand exactly what change was involved here, and went to the PR in question: https://github.com/dland/File-Path/pull/5 It's kind of a hot mess. 139 commits, etc. Fortunately, this one jumps out, and I think it's the issue: https://github.com/rpcme/File-Path/commit/4db118d2f7955e6201881ab55fadee5d15cda071#diff-7f234144055528e185f3aed8a75f0932L279 So, the question is: is the code supposed to work as it was originally written, or was it a bug that errors were not given when no entity existed? I believe it was supposed to work as written. I don't believe the error on that was forgotten, but rather intentionally omitted. It lets you ignore "you wanted it gone, and it already is!" but not "you wanted it gone, but I couldn't remove some of it." There is one problem, though. In this case, you're fine: my $count = rmtree($dir); If $count is 0, it failed. In this case, though: my $count = rmtree($dir1, $dir2); $count might be 100, and that could mean that $dir2 was deleted or not, because it a recursive count, and you quite likely don't know how many items are in each directory. At this point, I think we're at a niche concern. Adding an option to test for the existence of the directory seems reasonable, but I would not make it the default, given the longstanding behavior to the contrary. Finally, as suggested, "rmtree($x) if -d $x" is not atomic, and not a real way to avoid this unwanted warning. -- rjbs

Mon Jul 27 19:01:05 2015 rjbs [...] cpan.org - Status changed from 'rejected' to 'open'

Mon Jul 27 19:31:05 2015 RICHE [...] cpan.org - Correspondence added

On Mon Jul 27 19:01:03 2015, RJBS wrote: Show quoted text

> I wrote a longish response to this on another ticket by mistake, but > I'm not going to repost it here, because subsequent to that, I got a > somewhat better handle on what is going on, and so I'll start afresh. > > It has long been the case that you can write: > > my $removed = rmtree(...); > > ...and any errors removing anything are warned. If you don't want to > have them warned, you capture them. > > Andreas and Slaven are complaining, not about wanting "rm -rf" but > specifically about the case where there is nothing to delete. That > is, if you can't delete something because of permissions: bad. If you > can't delete something because it does not exist: oh well. > > I think this is probably pretty reasonable, given how things have > worked forever. You can go see my misdirected pean on this on #39949 > if you really want to, but basically: people will be angry at these > new warnings, and this ticket is just the tip of the iceberg. > > I tried to understand exactly what change was involved here, and went > to the PR in question: https://github.com/dland/File-Path/pull/5 > > It's kind of a hot mess. 139 commits, etc. Fortunately, this one > jumps out, and I think it's the issue: > > https://github.com/rpcme/File- > Path/commit/4db118d2f7955e6201881ab55fadee5d15cda071#diff- > 7f234144055528e185f3aed8a75f0932L279 > > So, the question is: is the code supposed to work as it was > originally written, or was it a bug that errors were not given when no > entity existed? > > I believe it was supposed to work as written. I don't believe the > error on that was forgotten, but rather intentionally omitted. It > lets you ignore "you wanted it gone, and it already is!" but not "you > wanted it gone, but I couldn't remove some of it." > > There is one problem, though. In this case, you're fine: > > my $count = rmtree($dir); > > If $count is 0, it failed. In this case, though: > > my $count = rmtree($dir1, $dir2); > > $count might be 100, and that could mean that $dir2 was deleted or > not, because it a recursive count, and you quite likely don't know how > many items are in each directory. > > At this point, I think we're at a niche concern. Adding an option to > test for the existence of the directory seems reasonable, but I would > not make it the default, given the longstanding behavior to the > contrary. > > Finally, as suggested, "rmtree($x) if -d $x" is not atomic, and not a > real way to avoid this unwanted warning.

fwiw the release repos is now https://github.com/rpcme/File-Path... I am sorry you even looked at that PR, I had completely forgotten about it. I sympathize that there are users that may get perturbed by this change. However, I want to make it be consistent, not the land of special cases. So, would users react the same way if we didn't carp on every other operation? that only _error can be evaluated for issues?

Tue Jul 28 02:49:31 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-07-27 05:55:02, RICHE wrote: Show quoted text

>

> > No, I am requesting the restoration of the old behavior, like it was > > in the last 20 years. It would be much easier to amend the > > documentation that rmtree() behaves like "rm -rf". > > > > Still I don't know how to use rmtree warning-free in the current > > version. Do we have to wait for the new option? When will it be > > implemented? How should users deal with the situation if they want to > > write code which is compatible across the different File::Path > > versions? Something like this: > > > > if ($File::Path::VERSION >= ???) { > > rmtree($path, { force => 1 }); > > } elsif ($File::Path::VERSION >= 2.11) { > > local $SIG{__WARN__} = sub {}; > > rmtree($path); > > } else { > > rmtree($path); > > } > > > > ?

> > > You really are a proponent for technical debt, huh? Pulling the 20 > year card will never work here.

This has nothing to do with technical debt. The previous behavior of rmtree() was not wrong, quite the contrary, it's probably the more common use case to not warn here. It was just underdocumented, true, and adjusting the documentation would be the better change: keeping backward compatibility, and keeping the more common use case to be the default. Show quoted text

> Behaving like "rm -rf" _is_ a feature. File::Path was _never_ warning > OR error free. It was just the case that the directory could not be > stat that was missed. > > Like I said, the 'force' option will be considered, and it will > probably be implemented for the next version.

Tue Jul 28 16:36:39 2015 RICHE [...] cpan.org - Correspondence added

Show quoted text

> This has nothing to do with technical debt. The previous behavior of > rmtree() was not wrong, quite the contrary, it's probably the more > common use case to not warn here. It was just underdocumented, true, > and adjusting the documentation would be the better change: keeping > backward compatibility, and keeping the more common use case to be the > default.

Undocumented tribal knowledge is technical debt, which is exactly what led to this event. Bottom line is consistency and spirit of the module's purpose -- _not_ "it's been done this way forever". Once the research has been done it will permanently reside as inline comments to this module so I, and future maintainers, can benefit from it. I appreciate your inputs. However, the approach and tone I have received until RJBS's response was not reasonable. Only his response has made me reconsider. Until then, I suggest we both turn down the agressiveness and grandstanding a notch.

Thu Aug 06 19:08:02 2015 jkeenan [...] cpan.org - Reference to ticket #39949 added

Fri Sep 04 12:37:22 2015 SREZIC [...] cpan.org - Correspondence added

My list so far with affected modules (either just warnings or even test failures): - CPAN.pm - Setup-File -> https://rt.cpan.org/Ticket/Display.html?id=106715 - File-Rsync-Mirror-Recent-0.3.4 - Pod-Inherit-0.91 - App-makedpkg-0.05

Mon Sep 07 13:34:41 2015 ribasushi [...] leporine.io - Correspondence added

On Fri Sep 04 18:37:22 2015, SREZIC wrote: Show quoted text

> My list so far with affected modules (either just warnings or even > test failures): > > - CPAN.pm > - Setup-File -> https://rt.cpan.org/Ticket/Display.html?id=106715 > - File-Rsync-Mirror-Recent-0.3.4 > - Pod-Inherit-0.91 > - App-makedpkg-0.05

CPAN.pm being broken *alone* is beyond enough reason to unconditionally revert to the old behavior. What needs to be done to have this mess resolved within the CPAN index ASAP?

Mon Sep 07 13:35:23 2015 ribasushi [...] leporine.io - Cc RIBASUSHI added

Sun Sep 20 06:16:43 2015 SREZIC [...] cpan.org - Reference by https://github.com/nichtich/App-makedpkg/issues/3 added

Sun Sep 20 06:17:09 2015 SREZIC [...] cpan.org - Reference by ticket #106715 added

Wed Sep 23 15:40:19 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-09-04 12:37:22, SREZIC wrote: Show quoted text

> My list so far with affected modules (either just warnings or even > test failures): > > - CPAN.pm > - Setup-File -> https://rt.cpan.org/Ticket/Display.html?id=106715 > - File-Rsync-Mirror-Recent-0.3.4 > - Pod-Inherit-0.91 > - App-makedpkg-0.05

Two more: - App-MtAws-1.120 - Test-Kantan-0.40

Thu Sep 24 16:18:36 2015 SREZIC [...] cpan.org - Dependency by ticket #107304 added

Thu Oct 01 14:44:04 2015 rjbs [...] cpan.org - Correspondence added

I didn't realize this hadn't been sorted out yet. Rich, do you have an ETA for an update? If you're swamped, I will take care of it, just let me know. -- rjbs

Fri Oct 09 08:11:10 2015 RICHE [...] cpan.org - Correspondence added 60 min

On Thu Oct 01 14:44:04 2015, RJBS wrote: Show quoted text

> I didn't realize this hadn't been sorted out yet. > > Rich, do you have an ETA for an update? If you're swamped, I will > take care of it, just let me know.

https://github.com/rpcme/File-Path/releases/tag/2.12

Fri Oct 09 08:11:19 2015 RICHE [...] cpan.org - Status changed from 'open' to 'resolved'

Fri Oct 09 08:11:19 2015 RICHE [...] cpan.org - Taken

Fri Oct 09 08:12:50 2015 RICHE [...] cpan.org - Correspondence added

On Fri Oct 09 08:11:10 2015, RICHE wrote: Show quoted text

> On Thu Oct 01 14:44:04 2015, RJBS wrote:

> > I didn't realize this hadn't been sorted out yet. > > > > Rich, do you have an ETA for an update? If you're swamped, I will > > take care of it, just let me know.

> > > https://github.com/rpcme/File-Path/releases/tag/2.12

The uploaded file File-Path-2.12.tar.gz has entered CPAN as file: $CPAN/authors/id/R/RI/RICHE/File-Path-2.12.tar.gz size: 27469 bytes md5: 75e983ccb2523bd16af93582de10443c No action is required on your part Request entered by: RICHE (Richard Elberger) Request entered on: Fri, 09 Oct 2015 12:11:42 GMT Request completed: Fri, 09 Oct 2015 12:11:53 GMT

Fri Oct 09 08:42:35 2015 rjbs [...] cpan.org - Correspondence added

On 2015-10-09 08:12:50, RICHE wrote: Show quoted text

> > File-Path-2.12.tar.gz > > has entered CPAN as > > file: $CPAN/authors/id/R/RI/RICHE/File-Path-2.12.tar.gz > size: 27469 bytes > md5: 75e983ccb2523bd16af93582de10443c >

Exciting, thanks! -- rjbs

Sun Oct 11 08:37:43 2015 SREZIC [...] cpan.org - Correspondence added

On 2015-10-09 08:11:10, RICHE wrote: Show quoted text

> On Thu Oct 01 14:44:04 2015, RJBS wrote:

> > I didn't realize this hadn't been sorted out yet. > > > > Rich, do you have an ETA for an update? If you're swamped, I will > > take care of it, just let me know.

> > > https://github.com/rpcme/File-Path/releases/tag/2.12

Thanks!

Sun Oct 11 08:38:19 2015 SREZIC [...] cpan.org - Fixed in 2.12 added