Re: patch@34561 VMS exec handling / cwd realpath fixes
On Wed, Oct 29, 2008 at 10:21 PM, John E. Malmberg [EMAIL PROTECTED] wrote: Resubmitting for review. I removed the setup_cmddsc, and the unneeded readdir stuff, so this patch is what is needed for getting the symbolic links to be encoded and decoded properly in VMS.C and for CWD in pathtools. I've applied the changes to Perl_my_symlink as #34667. The Cwd changes should go through the PathTools RT queue, but I'll take them for a spin and see how they do.
Re: patch@34561 VMS exec handling / cwd realpath fixes
On Thu, Oct 30, 2008 at 11:03 AM, Craig A. Berry [EMAIL PROTECTED] wrote: On Wed, Oct 29, 2008 at 10:21 PM, John E. Malmberg [EMAIL PROTECTED] wrote: Resubmitting for review. I removed the setup_cmddsc, and the unneeded readdir stuff, so this patch is what is needed for getting the symbolic links to be encoded and decoded properly in VMS.C and for CWD in pathtools. I've applied the changes to Perl_my_symlink as #34667. That patch was badly mangled. It looked like a diff of a diff rather than a diff of code and it also messed up the curly braces. Hopefully fixed in 34668. That'll teach me to apply things I think I've already tested.
Re: patch@34561 VMS exec handling / cwd realpath fixes
Craig A. Berry wrote: On Thu, Oct 30, 2008 at 11:03 AM, Craig A. Berry [EMAIL PROTECTED] wrote: On Wed, Oct 29, 2008 at 10:21 PM, John E. Malmberg [EMAIL PROTECTED] wrote: Resubmitting for review. I removed the setup_cmddsc, and the unneeded readdir stuff, so this patch is what is needed for getting the symbolic links to be encoded and decoded properly in VMS.C and for CWD in pathtools. I've applied the changes to Perl_my_symlink as #34667. That patch was badly mangled. It looked like a diff of a diff rather than a diff of code and it also messed up the curly braces. Hopefully fixed in 34668. That'll teach me to apply things I think I've already tested. Sorry about that, I missed cleaning up that file before preparing the individual diffs. -John Personal Opinion Only
Re: patch@34561 VMS exec handling / cwd realpath fixes
Ok, Resubmitting for review. I removed the setup_cmddsc, and the unneeded readdir stuff, so this patch is what is needed for getting the symbolic links to be encoded and decoded properly in VMS.C and for CWD in pathtools. -John [EMAIL PROTECTED] Personal Opinion Only --- /rsync_root/perl/lib/cwd.pm Sun Oct 19 05:26:56 2008 +++ lib/cwd.pm Wed Oct 22 23:56:34 2008 @@ -647,23 +647,15 @@ return $ENV{'DEFAULT'} unless @_; my $path = shift; -if (-l $path) { -my $link_target = readlink($path); -die Can't resolve link $path: $! unless defined $link_target; - -return _vms_abs_path($link_target); -} - -if (defined VMS::Filespec::vms_realpath) { -my $path = $_[0]; +if (defined VMS::Filespec::vmsrealpath) { if ($path =~ m#(?=\^)/# ) { # Unix format -return VMS::Filespec::vms_realpath($path); +return VMS::Filespec::unixrealpath($path); } # VMS format - my $new_path = VMS::Filespec::vms_realname($path); + my $new_path = VMS::Filespec::vmsrealpath($path); # Perl expects directories to be in directory format $new_path = VMS::Filespec::pathify($new_path) if -d $path; @@ -673,6 +665,13 @@ # Fallback to older algorithm if correct ones are not # available. +if (-l $path) { +my $link_target = readlink($path); +die Can't resolve link $path: $! unless defined $link_target; + +return _vms_abs_path($link_target); +} + # may need to turn foo.dir into [.foo] my $pathified = VMS::Filespec::pathify($path); $path = $pathified if defined $pathified; --- /rsync_root/perl/ext/cwd/t/cwd.tTue Jan 22 22:17:18 2008 +++ ext/cwd/t/cwd.t Wed Oct 22 23:32:36 2008 @@ -173,6 +173,15 @@ $ENV{PERL_CORE} ? $Test_Dir : File::Spec-catdir('t', $Test_Dir) ) ); +if ($^O eq 'VMS') { + # Not easy to predict the physical volume name + $want = $ENV{PERL_CORE} ? $Test_Dir : File::Spec-catdir('t', $Test_Dir); + + # So just use the relative volume name + $want =~ s/^\[//; + + $want = quotemeta($want); +} like($abs_path, qr|$want$|i); like($fast_abs_path, qr|$want$|i); --- /rsync_root/perl/vms/vms.c Wed Oct 22 19:26:20 2008 +++ vms/vms.c Wed Oct 29 22:04:08 2008 @@ -12990,14 +12990,41 @@ /* * A thin wrapper around decc$symlink to make sure we follow the * standard and do not create a symlink with a zero-length name. + * + * Also in ODS-2 mode, existing tests assume that the link target + * will be converted to UNIX format. */ -/*{{{ int my_symlink(const char *path1, const char *path2)*/ -int my_symlink(const char *path1, const char *path2) { - if (!path2 || !*path2) { +/*{{{ int my_symlink(pTHX_ const char *contents, const char *link_name)*/ +int Perl_my_symlink(pTHX_ const char *contents, const char *link_name) { + if (!link_name || !*link_name) { SETERRNO(ENOENT, SS$_NOSUCHFILE); return -1; } - return symlink(path1, path2); + + if (decc_efs_charset) { + return symlink(contents, link_name); + } else { + int sts; + char * utarget; + + /* Unless we are in ODS-5 mode, convert the symlink target to UNIX */ + /* because in order to work, the symlink target must be in UNIX format */ + + /* As symbolic links can hold things other than files, we will only do */ + /* the conversion in in ODS-2 mode */ + + Newx(utarget, VMS_MAXRSS + 1, char); + if (do_tounixspec(contents, utarget, 0, NULL) == NULL) { + + /* This should not fail, as an untranslatable filename */ + /* should be passed through */ + utarget = (char *)contents; + } + sts = symlink(utarget, link_name); + Safefree(utarget); + return sts; + } + } /*}}}*/ @@ -13203,7 +13230,100 @@ if (haslower) __mystrtolower(rslt); } } - } ++ } else { ++ ++ /* Now for some hacks to deal with backwards and forward */ ++ /* compatibilty */ ++ if (!decc_efs_charset) { ++ ++ /* 1. ODS-2 mode wants to do a syntax only translation */ ++ rslt = do_rmsexpand(filespec, outbuf, ++ 0, NULL, 0, NULL, utf8_fl); ++ ++ } else { ++ if (decc_filename_unix_report) { ++ char * dir_name; ++ char * vms_dir_name; ++ char * file_name; ++ ++ /* 2. ODS-5 / UNIX report mode should return a failure */ ++ /*if the parent directory also does not exist */ ++ /*Otherwise, get the real path for the parent */ ++ /*and add the child to it. ++ ++ /* basename / dirname only available for VMS 7.0+ */ ++
Re: patch@34561 VMS exec handling / cwd realpath fixes
Craig A. Berry wrote: On Oct 24, 2008, at 9:48 PM, John E. Malmberg wrote: Craig A. Berry wrote: I don't think there's any reason for the opendir/readdir part of the patch. Running ext/File/Glob/t/basic.t under the debugger and doing a SET TRACE/CALL/NOSYS shows definitively that it's already using the routines in vms.c: %DEBUG-I-DYNIMGSET, setting image PL_FILE__GLOB trace on calls at BSD_GLOB\g_opendir\%LINE 73519+88 in THREAD 1 73519: return(PerlDir_open(buf)); %DEBUG-I-DYNIMGSET, setting image DBGPERLSHR trace on calls at VMS\Perl_opendir\%LINE 120525+16 in THREAD 1 120525: Newx(dir, VMS_MAXRSS, char); Is that with a threaded build? Yes. Ok. I will back out that from my local copy for my next rebuild. -John
Re: patch@34561 VMS exec handling / cwd realpath fixes
On Oct 24, 2008, at 9:48 PM, John E. Malmberg wrote: Craig A. Berry wrote: I don't think there's any reason for the opendir/readdir part of the patch. Running ext/File/Glob/t/basic.t under the debugger and doing a SET TRACE/CALL/NOSYS shows definitively that it's already using the routines in vms.c: %DEBUG-I-DYNIMGSET, setting image PL_FILE__GLOB trace on calls at BSD_GLOB\g_opendir\%LINE 73519+88 in THREAD 1 73519: return(PerlDir_open(buf)); %DEBUG-I-DYNIMGSET, setting image DBGPERLSHR trace on calls at VMS\Perl_opendir\%LINE 120525+16 in THREAD 1 120525: Newx(dir, VMS_MAXRSS, char); Is that with a threaded build? Yes. Craig A. Berry mailto:[EMAIL PROTECTED] ... getting out of a sonnet is much more difficult than getting in. Brad Leithauser
Re: patch@34561 VMS exec handling / cwd realpath fixes
On Oct 23, 2008, at 7:35 PM, John E. Malmberg wrote: Craig A. Berry wrote: At first glance the readdir part doesn't look right; there shouldn't need to be another completely different way of setting up the macros for one extension, but I don't know offhand why the existing macros would not be visible in File::Glob. According to the comments in iperlsys.h, it is the place that the macros need to be in order to make the wrappers visible to Perl. I don't think there's any reason for the opendir/readdir part of the patch. Running ext/File/Glob/t/basic.t under the debugger and doing a SET TRACE/CALL/NOSYS shows definitively that it's already using the routines in vms.c: %DEBUG-I-DYNIMGSET, setting image PL_FILE__GLOB trace on calls at BSD_GLOB\g_opendir\%LINE 73519+88 in THREAD 1 73519: return(PerlDir_open(buf)); %DEBUG-I-DYNIMGSET, setting image DBGPERLSHR trace on calls at VMS\Perl_opendir\%LINE 120525+16 in THREAD 1 120525: Newx(dir, VMS_MAXRSS, char); and a bit later: trace on calls at BSD_GLOB\glob3\%LINE 73312+16 in THREAD 1 73312: while ((dp = (*readdirfunc)(dirp))) { trace on calls at BSD_GLOB\my_readdir\%LINE 72577+16 in THREAD 1 72577: return PerlDir_read(d); trace on calls at BSD_GLOB\my_readdir\%LINE 72577+48 in THREAD 1 72577: return PerlDir_read(d); %DEBUG-I-DYNIMGSET, setting image DBGPERLSHR trace on calls at PERLAPI\Perl_Gthr_key_ptr\%LINE 72551+28 in THREAD 1 72551: PERLVAR(Gthr_key, perl_key) /* key to retrieve per-thread struct */ %DEBUG-I-DYNIMGSET, setting image PL_FILE__GLOB trace on calls at BSD_GLOB\my_readdir\%LINE 72577+96 in THREAD 1 72577: return PerlDir_read(d); %DEBUG-I-DYNIMGSET, setting image DBGPERLSHR trace on calls at VMS\Perl_readdir\%LINE 120685+20 in THREAD 1 Craig A. Berry mailto:[EMAIL PROTECTED] ... getting out of a sonnet is much more difficult than getting in. Brad Leithauser
Re: patch@34561 VMS exec handling / cwd realpath fixes
Craig A. Berry wrote: On Oct 23, 2008, at 7:35 PM, John E. Malmberg wrote: Craig A. Berry wrote: At first glance the readdir part doesn't look right; there shouldn't need to be another completely different way of setting up the macros for one extension, but I don't know offhand why the existing macros would not be visible in File::Glob. According to the comments in iperlsys.h, it is the place that the macros need to be in order to make the wrappers visible to Perl. I don't think there's any reason for the opendir/readdir part of the patch. Running ext/File/Glob/t/basic.t under the debugger and doing a SET TRACE/CALL/NOSYS shows definitively that it's already using the routines in vms.c: %DEBUG-I-DYNIMGSET, setting image PL_FILE__GLOB trace on calls at BSD_GLOB\g_opendir\%LINE 73519+88 in THREAD 1 73519: return(PerlDir_open(buf)); %DEBUG-I-DYNIMGSET, setting image DBGPERLSHR trace on calls at VMS\Perl_opendir\%LINE 120525+16 in THREAD 1 120525: Newx(dir, VMS_MAXRSS, char); Is that with a threaded build? When I stepped into the PerlDir_open() in PL_FILE__GLOB, using a threaded Perl, it appeared to step into the a VMS threaded shared image instead of DBGPERLSHR.EXE -John [EMAIL PROTECTED] Personal Opinion Only
Re: patch@34561 VMS exec handling / cwd realpath fixes
On Thu, Oct 23, 2008 at 12:38 AM, John E. Malmberg [EMAIL PROTECTED] wrote: Craig, Ken, Can you review this patch? I'll take a look, though I can't promise to do so immediately. At first glance the readdir part doesn't look right; there shouldn't need to be another completely different way of setting up the macros for one extension, but I don't know offhand why the existing macros would not be visible in File::Glob. There are really three separate patches here, the symlink one, the readdir one, and the Cwd one. It would be helpful in the future if things like this could be submitted as three separate patches in three separate messages so they can be more easily reviewed, considered, tested, integrated, or (worst case) rolled back individually without affecting the unrelated parts. For Cwd, the patches should be submitted here: http://rt.cpan.org/Dist/Display.html?Name=PathTools though for VMS-specific things I do appreciate a CC to vmsperl. Thanks for your work, and for your patience while I try to find time to digest it.
Re: patch@34561 VMS exec handling / cwd realpath fixes
Craig A. Berry wrote: On Thu, Oct 23, 2008 at 12:38 AM, John E. Malmberg [EMAIL PROTECTED] wrote: Craig, Ken, Can you review this patch? I'll take a look, though I can't promise to do so immediately. At first glance the readdir part doesn't look right; there shouldn't need to be another completely different way of setting up the macros for one extension, but I don't know offhand why the existing macros would not be visible in File::Glob. According to the comments in iperlsys.h, it is the place that the macros need to be in order to make the wrappers visible to Perl. iperlsys.h had macros that where hiding the macros in vmsish.h. The extra stuff in vmsish.h is for handling passing the thread context for some of the wrappers. It would be nice to clean that up so that the wrappers for the C library do not need the thread context, so that they would be cleaner. There are really three separate patches here, the symlink one, the readdir one, and the Cwd one. It would be helpful in the future if things like this could be submitted as three separate patches in three separate messages so they can be more easily reviewed, considered, tested, integrated, or (worst case) rolled back individually without affecting the unrelated parts. For Cwd, the patches should be submitted here: http://rt.cpan.org/Dist/Display.html?Name=PathTools though for VMS-specific things I do appreciate a CC to vmsperl. Thanks for your work, and for your patience while I try to find time to digest it. Sorry about combining the patches, but I am trying to get back into getting the ODS-5/Unix/GNV mode, and the exec setup_cmddsc issue were blocking it, and I was trying to avoid taking three days to validate individual patches. I thought that the readdir/bsd_glob issue might also be blocking me. The symlink change forced the cwd change so as soon as the symlink change is commited, the cwd tests will start failing. Changing cwd before the symlink change would also probably result in the cwd test failing, since the symlinks targets were being created in VMS syntax, so the realpath() routines would not have handled them as the test expects. In UNIX compatibility mode, about 130 scripts out of 1424 are currently failing on VMS. -John [EMAIL PROTECTED] Personal Opinion Only
patch@34561 VMS exec handling / cwd realpath fixes
Craig, Ken, Can you review this patch? Part 1, vms.patch In vms.c: setup_cmddsc needed a fix to handle Unix executable images when ODS-5 parsing rules are active. Those rules translate './perl' to be '[]perl.' and the code to look up images needs the trailing '.' removed. This was required for [.vms]test.com to work when the DECC$ feature settings are in UNIX compatibility mode. Perl_my_symlink, when in the default mode needs to force the symbolic link target to be in UNIX format. Targets in VMS format are not recognized by the VMS filesystem. mp_do_vmsrealpath, needs to have special handling for non-existent file specifications. Traditional VMS mode expects a syntax only translation. UNIX compatible mode needs to do a realpath() operation on the parent directory if it exists and then append the filename to it. In iperlsys.h : Investigating a failure of [.ext.file.glob.t]global.t showed that the bsd_glob module was using the CRTL opendir() and and related files instead of the ones in VMS.C vmsish.h Changes required by the above. Part 2, cwd.patch Because vms.c is now causing the symbolic link target in cwd.t to be in UNIX syntax and actually work, the test is failing as cwd.pm is using readlink() to attempt to resolve the path back to the original VMS syntax. Fix cwd.pm to use the unixrealpath and vmsrealpath routines if if they exist. Fix cwd.t to be able to detect when cwd.pm returns the correct results. I am now shifting my focus to getting as much of the Perl tests to pass when the DECC feature logicals are in UNIX compatibility mode, as this is required to natively build some of the open source projects that I am interested in. -John [EMAIL PROTECTED] Personal Opinion Only --- /rsync_root/perl/lib/cwd.pm Sun Oct 19 05:26:56 2008 +++ lib/cwd.pm Wed Oct 22 23:56:34 2008 @@ -647,23 +647,15 @@ return $ENV{'DEFAULT'} unless @_; my $path = shift; -if (-l $path) { -my $link_target = readlink($path); -die Can't resolve link $path: $! unless defined $link_target; - -return _vms_abs_path($link_target); -} - -if (defined VMS::Filespec::vms_realpath) { -my $path = $_[0]; +if (defined VMS::Filespec::vmsrealpath) { if ($path =~ m#(?=\^)/# ) { # Unix format -return VMS::Filespec::vms_realpath($path); +return VMS::Filespec::unixrealpath($path); } # VMS format - my $new_path = VMS::Filespec::vms_realname($path); + my $new_path = VMS::Filespec::vmsrealpath($path); # Perl expects directories to be in directory format $new_path = VMS::Filespec::pathify($new_path) if -d $path; @@ -673,6 +665,13 @@ # Fallback to older algorithm if correct ones are not # available. +if (-l $path) { +my $link_target = readlink($path); +die Can't resolve link $path: $! unless defined $link_target; + +return _vms_abs_path($link_target); +} + # may need to turn foo.dir into [.foo] my $pathified = VMS::Filespec::pathify($path); $path = $pathified if defined $pathified; --- /rsync_root/perl/ext/cwd/t/cwd.tTue Jan 22 22:17:18 2008 +++ ext/cwd/t/cwd.t Wed Oct 22 23:32:36 2008 @@ -173,6 +173,15 @@ $ENV{PERL_CORE} ? $Test_Dir : File::Spec-catdir('t', $Test_Dir) ) ); +if ($^O eq 'VMS') { + # Not easy to predict the physical volume name + $want = $ENV{PERL_CORE} ? $Test_Dir : File::Spec-catdir('t', $Test_Dir); + + # So just use the relative volume name + $want =~ s/^\[//; + + $want = quotemeta($want); +} like($abs_path, qr|$want$|i); like($fast_abs_path, qr|$want$|i); --- /rsync_root/perl/iperlsys.h Mon Feb 25 11:47:31 2008 +++ iperlsys.h Wed Oct 22 00:51:02 2008 @@ -433,16 +433,23 @@ #define PerlDir_mkdir(name, mode) Mkdir((name), (mode)) #ifdef VMS # define PerlDir_chdir(n) Chdir((n)) +# define PerlDir_rmdir(name) do_rmdir((name)) +# define PerlDir_close(dir) vms_closedir((dir)) +# define PerlDir_open(name) vms_opendir((name)) +# define PerlDir_read(dir)vms_readdir((dir)) +# define PerlDir_rewind(dir) vms_rewinddir((dir)) +# define PerlDir_seek(dir, loc) vms_seekdir((dir), (loc)) +# define PerlDir_tell(dir)vms_telldir((dir)) #else # define PerlDir_chdir(name) chdir((name)) +# define PerlDir_rmdir(name) rmdir((name)) +# define PerlDir_close(dir) closedir((dir)) +# define PerlDir_open(name) opendir((name)) +# define PerlDir_read(dir)readdir((dir)) +# define PerlDir_rewind(dir) rewinddir((dir)) +# define PerlDir_seek(dir, loc) seekdir((dir), (loc)) +# define