dagolden / Path-Tiny

File path utility
41 stars 58 forks source link

Using lines() on a read-only file on AIX does not work #110

Closed zzdavidg closed 10 years ago

zzdavidg commented 10 years ago

Using lines() on a read-only file on AIX does not work.

This is because lines() internally asks for a 'locked' filehandle my $fh = $self->filehandle( { locked => 1 }, "<", $binmode );

and inside filehandle, if a lock is requested on AIX in read-only mode - unlike all other platforms where it would acquire a shared lock - filehandle instead opens the file in read-write mode and asks for an exclusive lock

    elsif ( $^O eq 'aix' && $opentype eq "<" ) {
        # AIX can only lock write handles, so upgrade to RW and LOCK_EX
        $opentype = "+<";
        $lock     = Fcntl::LOCK_EX();
    }
    else {
        $lock = $opentype eq "<" ? Fcntl::LOCK_SH() : Fcntl::LOCK_EX();
    }

Now this is documented in your documentation under caveats, but the fact that lines() will then only work on files with read and write permission on AIX is not.

Now I don't specifically know the history behind the caveat - as on my AIX 6.1 the man page for flock says

http://www-01.ibm.com/support/knowledgecenter/api/content/ssw_aix_61/com.ibm.aix.basetrf1/lockfx.htm

"The flock subroutine locks and unlocks entire files. This is a limited interface maintained for BSD compatibility, although its behavior differs from BSD in a few subtle ways. To apply a shared lock, the file must be opened for reading. To apply an exclusive lock, the file must be opened for writing. "

I checked back to AIX 5.2 and its flock documentation is the same (http://publib16.boulder.ibm.com/pseries/en_US/libs/basetrf1/basetrf1.pdf)

So it appears that at least AIX 5.2 through 6.1 locking works the same as Path::Tiny expects without the need for the AIX caveat. I didn't check 7.1 or AIX 4.

My specific case is I'm trying to use Perl::Critic 1.121 on AIX 6.1. Running perlcritic fails when Pod::Wordlist tries to load its wordlist file, installed read-only by File::ShareDir::Install, via a Path::Tiny object returned by File::ShareDir::ProjectDistDir. The line that fails is

dist_file('Pod-Spell', 'wordlist')->lines_utf8({ chomp => 1 })

The error is

Couldn't require Perl::Critic::Policy::Documentation::PodSpelling : Error open (+<:raw:encoding(UTF-8)) on '/u01/perl/perlmod/auto/share/dist/Pod-Spell/wordlist': Permission denied at /u01/perl/perlmod/Pod/Wordlist.pm line 22.

As the openmode being used (+<) requires the file to have user write permission.

I've worked around the issue by chmod'ing the file to have write permission, but I suspect Path::Tiny really shouldn't assume users have write permission to the file to use lines().

dagolden commented 10 years ago

The history is in issue #71. Maybe the best we can do on AIX is to only do the locking if write permission exists and do an unsafe read otherwise. What do you think?

dagolden commented 10 years ago

Whoops. Hadn't meant to close it until I got feedback.

zzdavidg commented 10 years ago

I'm fine with that as in my use case (perlcritic reading the wordlist for the spell checking dictionary) the locking aspect is not hugely important. I'm not an expert enough on the nuances of AIX behaviour (I'm more used to HPUX and Linux) to know how flock works on it. I was going purely off the man pages which said that LOCK_SH locks can be acquired in read-only mode (which seems to be what #71 says doesn't work).

I would suspect that most users of Path::Tiny would prefer the openmode requested to be honoured and the locking to be forgone rather than the openmode forcing the file to be writeable just so that locking can be applied.

dagolden commented 10 years ago

IIRC, the issue is that Perl on AIX doesn't use flock but uses fcntl instead for some reason and the limitation comes from that.

Since you're happy with the resolution, I'll ship it to CPAN and close this ticket. I don't think the change will break tests, but as I don't have AIX, I'll have to rely on you/others to let me know.