RE: [PATCH revised] Fix ExtUtils::Install under Cygwin

From: "Steve Hay" <SteveHay@planit.com>
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 (file)
--- 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 (file)
--- 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<Mac OS>)
 
+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<Win32>)
+
 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 (file)
--- 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 (file)
--- 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] == '.') {