Skip Menu |

This queue is for tickets about the Module-Path CPAN distribution.

Report information
The Basics
Id: 100979
Status: rejected
Priority: 0/
Queue: Module-Path
People
Owner: NEILB [...] cpan.org
Requestors: PERLANCAR [...] cpan.org
Cc:
AdminCc:
Bug Information
Severity: Normal
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Wed Dec 17 22:31:51 2014 PERLANCAR [...] cpan.org - Ticket created
Subject: Why abs_path() at all?
I'm just wondering why Module::Path needs to go the trouble of doing abs_path() in the first place. Is there a real need for it? Can the user do it herself if she needs it? Perl doesn't do it. In fact this code (the gist of which is mentioned in the POD) doesn't work when there is a relative dir in @INC: % perl -Ilib -MModule::Path=module_path -MString::Indent -E'say module_path("String/Indent.pm"); say $INC{"String/Indent.pm"}; say module_path("String/Indent.pm") eq $INC{"String/Indent.pm"} ? 1:0' /mnt/u1/home/repos/perl-String-Indent/lib/String/Indent.pm lib/String/Indent.pm On the other hand, doing abs_path() causes all the complexities like having to check for -d, -x, -r and doing eval() to avoid error messages being printed and so on. Is it worth it?
  Thu Dec 18 17:07:25 2014 NEILB [...] cpan.org - Correspondence added
If you look at the Changes file, you'll see that starting with 0.10_01, I did a number of releases that worked on resolving symlinks, triggered by bug reports for Debian and FreeBSD. I can't remember the details, but there were two people who were happy for me to email them test releases, so I could iterate until I fixed there specific problems, while continuing to pass the basic tests. I thought about adding an option to specify whether you want symlinks resolving, but it's such a simple module concept, that I think the interface should stay as simple as possible, and it should "just work". The only downside with interface is that module_path() can return a path which doesn't look like it came from anything in @INC. But given that I wrote this for use by tool writers, where code wants to find a module then open the file and process the source, I think that's an ok trade-off.
  Thu Dec 18 17:07:26 2014 The RT System itself - Status changed from 'new' to 'open'
  Thu Dec 18 17:08:00 2014 NEILB [...] cpan.org - Status changed from 'open' to 'rejected'
  Thu Dec 18 17:08:01 2014 NEILB [...] cpan.org - Taken
  Fri Dec 19 00:26:26 2014 PERLANCAR [...] cpan.org - Correspondence added
On Thu Dec 18 17:07:25 2014, NEILB wrote: Show quoted text
> If you look at the Changes file, you'll see that starting with > 0.10_01, I did a number of releases that worked on resolving symlinks, > triggered by bug reports for Debian and FreeBSD. I can't remember the > details, but there were two people who were happy for me to email them > test releases, so I could iterate until I fixed there specific > problems, while continuing to pass the basic tests.
If I read the Github issues correctly, it was the other way around. The whole thing started here: https://github.com/neilbowers/Module-Path/issues/4 Someone (sharl) suggested that Module::Path handles symlinks (and sent a naive patch) and *then* this change broke things and only then Debian/FreeBSD bug reports came flowing in. After that the naive symlink handling code was changed to abs_path() and it still broke some systems and then you had to add eval {}. sharl complained about not finding the path in the Debian package because the @INC entry '/usr/share/perl/5.14' is a symlink to '/usr/share/perl/5.14.2' and the Debian package lists the real paths while mpath reported the symlinked path. BTW, the symlink is intentional so that when perl is upgraded to another patchlevel (e.g. 5.14.3) perl can still use the old @INC dirs. sharl could've just done this: sharl@revo:~$ realpath `mpath autodie` /usr/share/perl/5.14.2/autodie.pm Show quoted text
> I thought about adding an option to specify whether you want symlinks > resolving, but it's such a simple module concept, that I think the > interface should stay as simple as possible, and it should "just > work".
I would argue that if you want to stay as simple as possible, you should not handle symlink or doing abs_path() at all :-) Show quoted text
> The only downside with interface is that module_path() can return a > path which doesn't look like it came from anything in @INC.
Yes, which means that if you still do abs_path(), you should remove the example in the POD (and the test): module_path('Test/More.pm') eq $INC{'Test/More.pm'}, because once the @INC entry uses a non-absolute (or an absolute) path containing symlink dir somewhere in the element, the code would break. Because perl doesn't do abs_path() and Module::Path does. Show quoted text
> But given that I wrote this for use by tool writers, where code wants > to find a module then open the file and process the source, I think > that's an ok trade-off.
But even without abs_path(), "finding module and opening the file and processing the source" works perfectly fine. In the specific use cases where a user wants to abs_path(), like sharl above, he can abs_path() the found path himself.