Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File::Path::make_path dies under error conditions #15483

Closed
p5pRT opened this issue Jul 28, 2016 · 10 comments
Closed

File::Path::make_path dies under error conditions #15483

p5pRT opened this issue Jul 28, 2016 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 28, 2016

Migrated from rt.perl.org#128767 (status was 'rejected')

Searchable as RT128767$

@p5pRT
Copy link
Author

p5pRT commented Jul 28, 2016

From the.rob.dixon@gmail.com

Created by the.rob.dixon@gmail.com

Something like

  $ perl -MFile​::Path=make_path -e 'make_path q{/xxx}'

will die with

  mkdir /xxx​: Permission denied at -e line 1.

I suggest that it should behave the same way as open​:

  $ perl -E 'open ">", q{/xxx} or warn $!; say "But all's okay";'

which produces

  No such file or directory at -e line 1.
  But all's okay

No doubt there is a lot of code out there that relies on the current
behaviour, so the fix would have to be a option in the `use` line. However
I believe that a die that requires an `eval` on every call is very wrong
for such a simple file operation

Perl Info

Flags:
    category=library
    severity=low

Site configuration information for perl 5.24.0:

Configured by strawberry-perl at Tue May 10 21:33:22 2016.

Summary of my perl5 (revision 5 version 24 subversion 0) configuration:

  Platform:
    osname=MSWin32, osvers=6.3, archname=MSWin32-x64-multi-thread
    uname='Win32 strawberry-perl 5.24.0.1 #1 Tue May 10 21:30:49 2016 x64'
    config_args='undef'
    hint=recommended, useposix=true, d_sigaction=undef
    useithreads=define, usemultiplicity=define
    use64bitint=define, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags =' -s -O2 -DWIN32 -DWIN64 -DCONSERVATIVE
 -DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS
-fwrapv -fno-strict-aliasing -mms-bitfields',
    optimize='-s -O2',
    cppflags='-DWIN32'
    ccversion='', gccversion='4.9.2', gccosandvers=''
    intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678,
doublekind=3
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16,
longdblkind=3
    ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='long
long', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='g++', ldflags ='-s -L"C:\strawberry\perl\lib\CORE"
-L"C:\strawberry\c\lib"'
    libpth=C:\strawberry\c\lib C:\strawberry\c\x86_64-w64-mingw32\lib
C:\strawberry\c\lib\gcc\x86_64-w64-mingw32\4.9.2
    libs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32
-ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr
-lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
    perllibs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32
-ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr
-lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
    libc=, so=dll, useshrplib=true, libperl=libperl524.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=xs.dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-mdll -s -L"C:\strawberry\perl\lib\CORE"
-L"C:\strawberry\c\lib"'



@INC for perl 5.24.0:
    C:/Strawberry/perl/site/lib/MSWin32-x64-multi-thread
    C:/Strawberry/perl/site/lib
    C:/Strawberry/perl/vendor/lib
    C:/Strawberry/perl/lib
    .


Environment for perl 5.24.0:
    HOME (unset)
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=C:\PHP\;C:\ProgramData\Oracle\Java\javapath;C:\Program Files
(x86)\NVIDIA
Corporation\PhysX\Common;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\Strawberry\perl\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\c\bin;C:\Python27\;C:\Python27\Scripts;E:\Perl\source\public;C:\ProgramData\chocolatey\bin;C:\Program
Files\Git\cmd;C:\ffmpeg\bin;C:\SQLite;C:\PHP;C:\tools\php;C:\rakudo\bin;C:\rakudo\share\perl6\site\bin
    PERL_BADLANG (unset)
    SHELL (unset)

@p5pRT
Copy link
Author

p5pRT commented Jul 29, 2016

From zefram@fysh.org

Rob Dixon wrote​:

I believe that a die that requires an `eval` on every call is very wrong
for such a simple file operation

Exception throwing is the superior way to signal errors. Wanting to
continue with the immediately-following code after an error is much
rarer than wanting to abort some significant part of the program.

so the fix would have to be a option in the `use` line.

It's not a fix.

If such an alternate interface were to be offered, it would be invoked
either as an option in the function's parameters, or a separate function,
supplied by either the same module or a different module. You're welcome
to wrap it in your own module that provides the interface you want.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Jul 29, 2016

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jul 29, 2016

From @jkeenan

On Thu Jul 28 16​:31​:18 2016, the.rob.dixon@​gmail.com wrote​:

This is a bug report for perl from the.rob.dixon@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.24.0.

-----------------------------------------------------------------
[Please describe your issue here]

Something like

$ perl -MFile​::Path=make_path -e 'make_path q{/xxx}'

will die with

mkdir /xxx​: Permission denied at -e line 1.

I suggest that it should behave the same way as open​:

$ perl -E 'open ">", q{/xxx} or warn $!; say "But all's okay";'

which produces

No such file or directory at -e line 1.
But all's okay

No doubt there is a lot of code out there that relies on the current
behaviour, so the fix would have to be a option in the `use` line.
However
I believe that a die that requires an `eval` on every call is very
wrong
for such a simple file operation

File-Path is "upstream cpan", so, strictly speaking, new feature requests should be reported via​:

bug-File-Path@​rt.cpan.org

Rich Elberger and I are the current maintainers. When we did a lot of maintenance work in 2015 we found out that -- yes, indeed -- there is a lot of code out there that depends on File​::Path's current behavior. And there are a lot of people prepared to become very vocal about any changes to that behavior, even if the purpose of those changes is to rationalize inconsistencies among the module's three (!) different interfaces.

Hence, even apart from the objections to this feature request such as those raised by Zefram in another post to this thread/RT, there are good reasons to reject this feature request.

Moreover -- and here I speak only for myself and not for RICHE -- File-Path has acquired too many interfaces over the years and is incapable of evolving further in a rationale, consistent and well designed manner. Rather than trying to tack additional refinements onto File-Path, we should consider writing a new library -- starting first on CPAN -- that fulfills the same objectives as File-Path and could one day be shipped with Perl 5 core.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jul 29, 2016

From rich@richelberger.com

I agree with everything Jim has said here. Either a new one or File​::Path​::Tiny. There's just far too many dependencies on File​::Path and Jim and I got burnt (and schooled) by trying to fix up many things last year.

Having said that it might be worth gathering some of the techniques downstream modules have used to mitigate File​::Path shortcomings and putting those in a pod in with the module distribution.

-- rich

On Jul 29, 2016, at 09​:13, James E Keenan via RT <perlbug-followup@​perl.org> wrote​:

On Thu Jul 28 16​:31​:18 2016, the.rob.dixon@​gmail.com wrote​:
This is a bug report for perl from the.rob.dixon@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.24.0.

-----------------------------------------------------------------
[Please describe your issue here]

Something like

$ perl -MFile​::Path=make_path -e 'make_path q{/xxx}'

will die with

mkdir /xxx​: Permission denied at -e line 1.

I suggest that it should behave the same way as open​:

$ perl -E 'open ">", q{/xxx} or warn $!; say "But all's okay";'

which produces

No such file or directory at -e line 1.
But all's okay

No doubt there is a lot of code out there that relies on the current
behaviour, so the fix would have to be a option in the `use` line.
However
I believe that a die that requires an `eval` on every call is very
wrong
for such a simple file operation

File-Path is "upstream cpan", so, strictly speaking, new feature requests should be reported via​:

bug-File-Path@​rt.cpan.org

Rich Elberger and I are the current maintainers. When we did a lot of maintenance work in 2015 we found out that -- yes, indeed -- there is a lot of code out there that depends on File​::Path's current behavior. And there are a lot of people prepared to become very vocal about any changes to that behavior, even if the purpose of those changes is to rationalize inconsistencies among the module's three (!) different interfaces.

Hence, even apart from the objections to this feature request such as those raised by Zefram in another post to this thread/RT, there are good reasons to reject this feature request.

Moreover -- and here I speak only for myself and not for RICHE -- File-Path has acquired too many interfaces over the years and is incapable of evolving further in a rationale, consistent and well designed manner. Rather than trying to tack additional refinements onto File-Path, we should consider writing a new library -- starting first on CPAN -- that fulfills the same objectives as File-Path and could one day be shipped with Perl 5 core.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=128767

@p5pRT
Copy link
Author

p5pRT commented Jul 29, 2016

From the.rob.dixon@gmail.com

Thank you for your response.

Rob Dixon wrote​:

I believe that a die that requires an `eval` on every call is very
wrong
for such a simple file operation
Exception throwing is the superior way to signal errors. Wanting to
continue with the immediately-following code after an error is much
rarer than wanting to abort some significant part of the program.
I agree that, ideally, exceptions would be the correct way of dealing
with this, however

- It's not the way the rest of Perl works. `open`, and more pertinently
`mkdir` don't
throw exceptions in the same circumstance

- If you want an exception on failure then the familiar idiom "or die"
does it just fine

- `eval` is broken to the point that most people familiar with Perl
recommend that you use
the non-core `Try​::Tiny` instead

- The non-core `File​::Path​::Tiny` is often preferred over the core
module precisely because
of this. It respects the idioms of Perl and simply returns zero if the
action was impossible

so the fix would have to be a option in the `use` line.
It's not a fix.
I'm unsure what you mean here. Of course it's a fix

If such an alternate interface were to be offered, it would be invoked
either as an option in the function's parameters, or a separate
function,
supplied by either the same module or a different module. You're
welcome
to wrap it in your own module that provides the interface you want.
I'm not too worried about how it's implemented, although I don't
immediately see
what's wrong with a parameter to `import`. Thank you for your permission
to write
a wrapping module, but I don't believe I should have to do that to make
a core
module behave like the rest of Perl.

It is not for a core language module to go on a solo crusade for better
practices.
It should conform to the status quo and leave revolution and subversion
to the
non-core CPAN modules.

Kind regards,

Rob Dixon


This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

@p5pRT
Copy link
Author

p5pRT commented Jul 30, 2016

From zefram@fysh.org

Rob Dixon wrote​:

- It's not the way the rest of Perl works. `open`, and more pertinently
`mkdir` don't
throw exceptions in the same circumstance

They come from Perl 4, where it wasn't feasible to use exceptions.
It's too late to change them to use exceptions, for the same backcompat
reasons. The best we have in that respect is the autodie pragma.

I'm unsure what you mean here. Of course it's a fix

It's not a fix because there's nothing broken. Not only does the
function actually work, but in this aspect of API design it's already
of the preferred form.

I don't immediately see
what's wrong with a parameter to `import`.

It's rather fiddly to make such a parameter cause one of two different
versions of a function to be imported under the same name, and somewhat
impedes understanding. It would of course be easy to make an import
parameter throw a global switch that changes the behaviour of the single
function for all callers, but that would break compatibility wherever
the function has multiple callers in a single program.

It is not for a core language module to go on a solo crusade for better
practices.

It's far from solo. Exception throwing is generally the preferred way
for all newly-designed APIs.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Jul 30, 2016

From J.X.BIRD@mdx.ac.uk

Take me off this email list!!

On 29/07/2016 13​:50, "Rob Dixon" <the.rob.dixon@​gmail.com> wrote​:

Thank you for your response.

Rob Dixon wrote​:

I believe that a die that requires an `eval` on every call is very
wrong
for such a simple file operation
Exception throwing is the superior way to signal errors. Wanting to
continue with the immediately-following code after an error is much
rarer than wanting to abort some significant part of the program.
I agree that, ideally, exceptions would be the correct way of dealing
with this, however

- It's not the way the rest of Perl works. `open`, and more pertinently
`mkdir` don't
throw exceptions in the same circumstance

- If you want an exception on failure then the familiar idiom "or die"
does it just fine

- `eval` is broken to the point that most people familiar with Perl
recommend that you use
the non-core `Try​::Tiny` instead

- The non-core `File​::Path​::Tiny` is often preferred over the core
module precisely because
of this. It respects the idioms of Perl and simply returns zero if the
action was impossible

so the fix would have to be a option in the `use` line.
It's not a fix.
I'm unsure what you mean here. Of course it's a fix

If such an alternate interface were to be offered, it would be invoked
either as an option in the function's parameters, or a separate
function,
supplied by either the same module or a different module. You're
welcome
to wrap it in your own module that provides the interface you want.
I'm not too worried about how it's implemented, although I don't
immediately see
what's wrong with a parameter to `import`. Thank you for your permission
to write
a wrapping module, but I don't believe I should have to do that to make
a core
module behave like the rest of Perl.

It is not for a core language module to go on a solo crusade for better
practices.
It should conform to the status quo and leave revolution and subversion
to the
non-core CPAN modules.

Kind regards,

Rob Dixon

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

@p5pRT
Copy link
Author

p5pRT commented Aug 1, 2016

From @tonycoz

On Fri Jul 29 06​:13​:27 2016, jkeenan wrote​:

On Thu Jul 28 16​:31​:18 2016, the.rob.dixon@​gmail.com wrote​:

No doubt there is a lot of code out there that relies on the current
behaviour, so the fix would have to be a option in the `use` line.
However
I believe that a die that requires an `eval` on every call is very
wrong
for such a simple file operation

File-Path is "upstream cpan", so, strictly speaking, new feature
requests should be reported via​:

bug-File-Path@​rt.cpan.org

Marking rejected.

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 1, 2016

@tonycoz - Status changed from 'open' to 'rejected'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant