Paul Cochrane wrote:
>> > But if we convert what MANIFEST provides (i.e. Unix directory
>> > separators) into whatever the current platform needs (i.e. what
>> > canonpath() does) then it should agree with whatever svn spits out.
>> > Or am I missing something?
>>
>> No, that's exactly what I think needs to be done. In the patch
>> canonpath is used with the result of the svn execution only. That's not
>> enough. @manifest_files would need to be changed too, otherwise it
>> contains the file names with forward slashes from MANIFEST. There's
>> also a regex $lf_files_regexp that seems to filter files, and the part
>> that uses it would need to be aware of the changed separator, too.
>>
>> > Essentially my patch is just a less fragile version of the patch you
>> > submitted to get this test to work on Windows. (at least, I don't
>> > think I'm changing the functionality that much).
>>
>> I simple changed the backward slashes to forward slashes, thus forward
>> slashes everywhere.
>
> Which was what *I* intended to do with my patch, but after staring at
> it long enough, I realised that's not what *it* was saying! :-)
> Ooops.
Oh, I see. Sorry I didn't get this right.
> I'd like to avoid converting the entire MANIFEST from Unix to Windows
> directory separators just so that we can have the hash we're
> generating in t/distro/file_metadata.t has consistent keys;
> converting to explicitly Unix should be enough (and converting the
> whole MANIFEST would make the test even slower than it already is).
> Even more so considering that there is a ticket in RT trying to get
> rid of MANIFEST. Anyway, attached is another patch, which hopefully
> does the right thing now. If everyone's happy with the patch, I'll
> apply and commit it to trunk.
>
> Ron, would it be ok if you could check the patch to see if it works on
> Win32? Thanks heaps in advance.
There's one piece missing: You'd need to splitdir/catdir $directories
too, otherwise it's left as-is.
I have attached a modified patch, and prove-ed it works with r18945 on
Win32.
Ron
Index: t/distro/file_metadata.t
===================================================================
--- t/distro/file_metadata.t (revision 18945)
+++ t/distro/file_metadata.t (working copy)
@@ -8,7 +8,8 @@
use Test::More;
use File::Basename qw( fileparse );
-use File::Spec::Functions qw( catfile );
+use File::Spec::Functions qw( catfile splitpath splitdir );
+use File::Spec::Unix;
use Parrot::Config;
use Parrot::Revision;
use ExtUtils::Manifest qw( maniread );
@@ -270,13 +271,22 @@
# This RE may be a little wonky.
if ( $result =~ m{(.*) - (.*)} ) {
- my ( $file, $attribute ) = ( $1, $2 );
+ my ( $full_path, $attribute ) = ( $1, $2 );
- # file names are reported with backslashes on Windows,
- # but we want forward slashes
- $file =~ s!\\!/!g if $^O eq 'MSWin32';
+ # split the path
+ my ( $volume, $directories, $file ) =
+ splitpath $full_path;
+ my @directories = splitdir $directories;
- $results{$file} = $attribute;
+ # put it back together as a unix path (to match MANIFEST)
+ $full_path = File::Spec::Unix->catpath(
+ $volume,
+ File::Spec::Unix->catdir(@directories),
+ $file
+ );
+
+ # store the attribute into the results hash
+ $results{$full_path} = $attribute;
}
}
