From: Steve Hay Date: Mon, 17 Mar 2008 14:36:54 +0000 (+0000) Subject: RE: [PATCH revised] Fix ExtUtils::Install under Cygwin X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=038ae9a45711aea142f721498a4a61353b40c4e4;p=p5sagit%2Fp5-mst-13.2.git RE: [PATCH revised] Fix ExtUtils::Install under Cygwin From: "Steve Hay" Message-ID: <1B32FF956ABF414C9BCE5E487A1497E70176BD61@ukmail02.planit.group> "OK, so how about the attached. This fixes up -w for all compilers so that it is symmetrical with chmod(), and adds a note to perltodo on fixing POSIX::access() and chdir()." The whole long thread started here: http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2008-03/msg00056.html p4raw-id: //depot/perl@33566 --- diff --git a/lib/ExtUtils/Install.pm b/lib/ExtUtils/Install.pm index 1b144b5..4d033aa 100644 --- a/lib/ExtUtils/Install.pm +++ b/lib/ExtUtils/Install.pm @@ -38,11 +38,11 @@ ExtUtils::Install - install files from here to there =head1 VERSION -1.50 +1.50_01 =cut -$VERSION = '1.50'; +$VERSION = '1.50_01'; $VERSION = eval $VERSION; =pod @@ -358,7 +358,8 @@ Abstract a -w check that tries to use POSIX::access() if possible. sub _have_write_access { my $dir=shift; unless (defined $has_posix) { - $has_posix= (!$Is_cygwin && eval 'local $^W; require POSIX; 1') || 0; + $has_posix= (!$Is_cygwin && !$Is_Win32 + && eval 'local $^W; require POSIX; 1') || 0; } if ($has_posix) { return POSIX::access($dir, POSIX::W_OK()); diff --git a/pod/perlport.pod b/pod/perlport.pod index 41c4e0a..f8fb1fe 100644 --- a/pod/perlport.pod +++ b/pod/perlport.pod @@ -1581,6 +1581,11 @@ C<-r>, C<-w>, and C<-x> have a limited meaning only; directories and applications are executable, and there are no uid/gid considerations. C<-o> is not supported. (S) +C<-w> only inspects the read-only file attribute (FILE_ATTRIBUTE_READONLY), +which determines whether the directory can be deleted, not whether it can +be written to. Directories always have read and write access unless denied +by discretionary access control lists (DACLs). (S) + C<-r>, C<-w>, C<-x>, and C<-o> tell whether the file is accessible, which may not reflect UIC-based file protections. (VMS) diff --git a/pod/perltodo.pod b/pod/perltodo.pod index 8d7aceb..227fd6b 100644 --- a/pod/perltodo.pod +++ b/pod/perltodo.pod @@ -484,6 +484,31 @@ warnings are also currently suppressed by adding -D_CRT_NONSTDC_NO_DEPRECATE. It might be nice to do as Microsoft suggest here too, although, unlike the secure functions issue, there is presumably little or no benefit in this case. +=head2 Fix POSIX::access() and chdir() on Win32 + +These functions currently take no account of DACLs and therefore do not behave +correctly in situations where access is restricted by DACLs (as opposed to the +read-only attribute). + +Furthermore, POSIX::access() behaves differently for directories having the +read-only attribute set depending on what CRT library is being used. For +example, the _access() function in the VC6 and VC7 CRTs (wrongly) claim that +such directories are not writable, whereas in fact all directories are writable +unless access is denied by DACLs. (In the case of directories, the read-only +attribute actually only means that the directory cannot be deleted.) This CRT +bug is fixed in the VC8 and VC9 CRTs (but, of course, the directory may still +not actually be writable if access is indeed denied by DACLs). + +For the chdir() issue, see ActiveState bug #74552: +http://bugs.activestate.com/show_bug.cgi?id=74552 + +Therefore, DACLs should be checked both for consistency across CRTs and for +the correct answer. + +(Note that perl's -w operator should not be modified to check DACLs. It has +been written so that it reflects the state of the read-only attribute, even +for directories (whatever CRT is being used), for symmetry with chmod().) + =head2 strcat(), strcpy(), strncat(), strncpy(), sprintf(), vsprintf() Maybe create a utility that checks after each libperl.a creation that diff --git a/win32/win32.c b/win32/win32.c index a5723d6..5a0dde3 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -1510,9 +1510,22 @@ win32_stat(const char *path, Stat_t *sbuf) errno = ENOTDIR; return -1; } + if (S_ISDIR(sbuf->st_mode)) { + /* Ensure the "write" bit is switched off in the mode for + * directories with the read-only attribute set. Borland (at least) + * switches it on for directories, which is technically correct + * (directories are indeed always writable unless denied by DACLs), + * but we want stat() and -w to reflect the state of the read-only + * attribute for symmetry with chmod(). */ + DWORD r = GetFileAttributesA(path); + if (r != 0xffffffff && (r & FILE_ATTRIBUTE_READONLY)) { + sbuf->st_mode &= ~S_IWRITE; + } + } #ifdef __BORLANDC__ - if (S_ISDIR(sbuf->st_mode)) - sbuf->st_mode |= S_IWRITE | S_IEXEC; + if (S_ISDIR(sbuf->st_mode)) { + sbuf->st_mode |= S_IEXEC; + } else if (S_ISREG(sbuf->st_mode)) { int perms; if (l >= 4 && path[l-4] == '.') {