[perl #41569] t/distro/file_metadata.t fails on win32

[Ron Blaschke]

Hi,

Could someone please tell me the expected result of
t/distro/file_metadata.pl at revision 17389? After looking into bug
#41569 I'm getting the following on Windows (XP, SP2, VC++ 8.0, Subversion).

>prove t/distro/file_metadata.t

t/distro/file_metadata....# Collecting svn:mime-type attributes...
t/distro/file_metadata....ok 1/3# Collecting svn:keywords attributes...

t/distro/file_metadata....NOK 2# Failed test
(t/distro/file_metadata.t at l
# got: '
# languages/APL/MAINTAINER (Author Date Id Revision)
# languages/BASIC/MAINTAINER (Author Date Id Revision)
# languages/HQ9plus/MAINTAINER (Author Date Id Revision)
# languages/PIR/MAINTAINER (Author Date Id Revision)
# languages/WMLScript/MAINTAINER (Author Date Id Revision)
# languages/Zcode/MAINTAINER (Author Date Id Revision)
# languages/abc/MAINTAINER (Author Date Id Revision)
# languages/amber/MAINTAINER (Author Date Id Revision)
# languages/befunge/MAINTAINER (Author Date Id Revision)
# languages/bf/MAINTAINER (Author Date Id Revision)
# languages/c99/MAINTAINER (Author Date Id Revision)
# languages/cardinal/MAINTAINER (Author Date Id Revision)
# languages/dotnet/MAINTAINER (Author Date Id Revision)
# languages/ecmascript/MAINTAINER (Author Date Id Revision)
# languages/lazy-k/MAINTAINER (Author Date Id Revision)
# languages/lisp/MAINTAINER (Author Date Id Revision)
# languages/lua/MAINTAINER (Author Date Id Revision)
# languages/ook/MAINTAINER (Author Date Id Revision)
# languages/parakeet/MAINTAINER (Author Date Id Revision)
# languages/parrot_compiler/MAINTAINER (Author Date Id Revision)
# languages/perl6/MAINTAINER (Author Date Id Revision)
# languages/pheme/MAINTAINER (Author Date Id Revision)
# languages/punie/MAINTAINER (Author Date Id Revision)
# languages/pynie/MAINTAINER (Author Date Id Revision)
# languages/scheme/MAINTAINER (Author Date Id Revision)
# languages/tcl/runtime/builtin/auto_execok.pir (Author Date Id Revision)
# languages/tcl/runtime/builtin/auto_load.pir (Author Date Id Revision)
# languages/tcl/runtime/builtin/exec.pir (Author Date Id Revision)
# languages/tcl/runtime/builtin/fconfigure.pir (Author Date Id Revision)
# languages/tcl/runtime/builtin/glob.pir (Author Date Id Revision)
# languages/tcl/runtime/builtin/interp.pir (Author Date Id Revision)
# languages/tcl/runtime/builtin/trace.pir (Author Date Id Revision)
# languages/tcl/runtime/builtin/update.pir (Author Date Id Revision)
# languages/unlambda/MAINTAINER (Author Date Id Revision)
# languages/urm/MAINTAINER (Author Date Id Revision)
# src/pmc/metaattribute.pmc (Author Date Id Revision)
# src/pmc/metaclass.pmc (Author Date Id Revision)
# src/pmc/object.pmc (Author Date Id Revision)
# '
# expected: ''
# Collecting svn:eol-style attributes...

t/distro/file_metadata....NOK 3# Failed test
(t/distro/file_metadata.t at l
# got: '
# examples/shootout/ack.pir.output (native)
# examples/shootout/binarytrees.pir.output (native)
# examples/shootout/fannkuch.pir.output (native)
# examples/shootout/fasta.pir.output (native)
# examples/shootout/knucleotide.pir.input (native)
# examples/shootout/knucleotide.pir.output (native)
# examples/shootout/nbody.pir.output (native)
# examples/shootout/nsieve-bits-2.pir.output (native)
# examples/shootout/nsieve-bits.pir.output (native)
# examples/shootout/nsieve.pir.output (native)
# examples/shootout/partialsums-2.pir.output (native)
# examples/shootout/partialsums.pir.output (native)
# examples/shootout/pidigits.pir.output (native)
# examples/shootout/recursive-2.pir.output (native)
# examples/shootout/recursive.pir.output (native)
# examples/shootout/regexdna.pir.input (native)
# examples/shootout/regexdna.pir.output (native)
# examples/shootout/revcomp.pir.input (native)
# examples/shootout/revcomp.pir.output (native)
# examples/shootout/spectralnorm.pir.output (native)
# examples/shootout/sumcol.pir.input (native)
# examples/shootout/sumcol.pir.output (native)
# examples/shootout/takfp.pir.output (native)
# languages/APL/MAINTAINER (native)
# languages/BASIC/MAINTAINER (native)
# languages/HQ9plus/MAINTAINER (native)
# languages/PIR/MAINTAINER (native)
# languages/WMLScript/MAINTAINER (native)
# languages/Zcode/MAINTAINER (native)
# languages/abc/MAINTAINER (native)
# languages/amber/MAINTAINER (native)
# languages/befunge/MAINTAINER (native)
# languages/bf/MAINTAINER (native)
# languages/c99/MAINTAINER (native)
# languages/cardinal/MAINTAINER (native)
# languages/dotnet/MAINTAINER (native)
# languages/ecmascript/MAINTAINER (native)
# languages/lazy-k/MAINTAINER (native)
# languages/lisp/MAINTAINER (native)
# languages/lua/MAINTAINER (native)
# languages/lua/lib/alarm.pir (native)
# languages/lua/t/alarm.t (native)
# languages/lua/t/package.t (native)
# languages/lua/t/regex.t (native)
# languages/lua/t/rx_captures (native)
# languages/lua/t/rx_charclass (native)
# languages/lua/t/rx_metachars (native)
# languages/ook/MAINTAINER (native)
# languages/parakeet/MAINTAINER (native)
# languages/parrot_compiler/MAINTAINER (native)
# languages/perl6/MAINTAINER (native)
# languages/pheme/MAINTAINER (native)
# languages/plumhead/lib/Parrot/Test/Plumhead/Perl5re.pm (native)
# languages/plumhead/src/perl5re/gen_past_pir.pl (native)
# languages/punie/MAINTAINER (native)
# languages/pynie/MAINTAINER (native)
# languages/pynie/README (native)
# languages/pynie/config/makefiles/root.in (native)
# languages/pynie/examples/ex-3-1-4-a.py (native)
# languages/pynie/examples/ex-3-2-a.py (native)
# languages/pynie/examples/ex-3-2-b.py (native)
# languages/pynie/examples/ex-3-2-c.py (native)
# languages/pynie/examples/ex-3.py (native)
# languages/pynie/pynie.pir (native)
# languages/pynie/src/PAST/Grammar.tg (native)
# languages/pynie/src/builtins/io.pir (native)
# languages/pynie/src/builtins/lists.pir (native)
# languages/pynie/src/builtins/oper.pir (native)
# languages/pynie/src/parser/Grammar.pg (native)
# languages/pynie/src/parser/indent.pir (native)
# languages/pynie/t/00-parrot/01-literals.t (native)
# languages/pynie/t/00-parrot/02-op-math.t (native)
# languages/pynie/t/00-parrot/03-op-logic.t (native)
# languages/pynie/t/00-parrot/05-vars.t (native)
# languages/pynie/t/00-parrot/06-stmts.t (native)
# languages/pynie/t/00-parrot/07-op-cmp.t (native)
# languages/pynie/t/harness (native)
# languages/scheme/MAINTAINER (native)
# languages/tcl/src/class/tclproc.pir (native)
# languages/unlambda/MAINTAINER (native)
# languages/urm/MAINTAINER (native)
# src/pmc/metaattribute.pmc (native)
# src/pmc/metaclass.pmc (native)
# src/pmc/object.pmc (native)
# src/pmc/smop_attribute.pmc (native)
# src/pmc/smop_class.pmc (native)
# t/compilers/pge/p5regex/re_tests (native)
# t/library/perlhist.txt (native)
# t/library/streams.t (native)
# t/op/sprintf_tests (native)
# t/pmc/parrotrunningthread.t (native)
# t/pmc/smop_attribute.t (native)
# t/pmc/smop_class.t (native)
# '
# expected: ''
# Looks like you failed 2 tests of 3.
t/distro/file_metadata....dubious
Test returned status 2 (wstat 512, 0x200)
DIED. FAILED tests 2-3
Failed 2/3 tests, 33.33% okay
Failed Test Stat Wstat Total Fail Failed List of Failed
-------------------------------------------------------------------------------
t/distro/file_metadata.t 2 512 3 2 66.67% 2-3
Failed 1/1 test scripts, 0.00% okay. 2/3 subtests failed, 33.33% okay.

Thanks,
Ron

[Will Coleda]

I expect the first two to pass, but metadata is often often
overlooked on commits.

The last one is a new test, not everything has been updated yet. (And
I'm not sure it *can* be without breaking windows).

Should be passing the second test again as of r17398.

--
Will "Coke" Coleda
wi...@coleda.com

[Ron Blaschke]

Will Coleda wrote:
> I expect the first two to pass, but metadata is often often overlooked
> on commits.
>
> The last one is a new test, not everything has been updated yet. (And
> I'm not sure it *can* be without breaking windows).
>
> Should be passing the second test again as of r17398.

Thanks for your help. I have attached the patch to make the test work
on Windows. The problem is that Subversion reports file names with
backslashes, but the code expects forward slashes. For example:

>svn propget svn:eol-style t/distro/*
t\distro\file_metadata.t - native
t\distro\manifest.t - native
t\distro\manifest_skip.t - native
t\distro\test_file_coverage.t - native

Attached patch replaces the backslashes with slashes on Windows.

Here's what I get for r17405.

>prove t\distro\file_metadata.t
t\distro\file_metadata....# Collecting svn:mime-type attributes...
# Collecting svn:keywords attributes...
t\distro\file_metadata....ok 1/3# Collecting svn:eol-style attributes...
t\distro\file_metadata....ok
All tests successful.
Files=1, Tests=3, 12 wallclock secs ( 0.00 cusr + 0.00 csys = 0.00 CPU)

Ron

[Paul Cochrane]

On 09/03/07, Ron Blaschke <mailin...@rblasch.org> wrote:
> Will Coleda wrote:
> > I expect the first two to pass, but metadata is often often overlooked
> > on commits.
> >
> > The last one is a new test, not everything has been updated yet. (And
> > I'm not sure it *can* be without breaking windows).
> >
> > Should be passing the second test again as of r17398.
>
> Thanks for your help. I have attached the patch to make the test work
> on Windows. The problem is that Subversion reports file names with
> backslashes, but the code expects forward slashes. For example:
>
> >svn propget svn:eol-style t/distro/*
> t\distro\file_metadata.t - native
> t\distro\manifest.t - native
> t\distro\manifest_skip.t - native
> t\distro\test_file_coverage.t - native
>
> Attached patch replaces the backslashes with slashes on Windows.

Thanks! Applied in r17406.

Paul

[Chromatic]

On Friday 09 March 2007 05:00, Ron Blaschke wrote:

> Attached patch replaces the backslashes with slashes on Windows.

Would using File::Spec be less fragile?

-- c

[Ron Blaschke]

The problem basically boils down to matching a list of MANIFEST (UNIX?)
files with the (native file name, attribute) output of svn. Not sure
how well specified the svn propget output is to begin with.

One way to make things more robust is to take the file names returned by
svn, make sure they are relative, parse them with File::Spec and put
them together with File::Spec::Unix. That is, use the MANIFEST format
as canonical paths.

Another solution would be to pull every file name, from the initial list
and the Subversion output, through File::Spec->canonpath.

Ron

[Paul Cochrane]

I've attached a patch which uses File::Spec instead of replacing one
set of slashes with another. Comments welcome! :-)

Paul

[Jerry Gay]

good idea.

instead of breaking up the path and reconstructing it separately
(since the individual components of the path aren't used anywhere
else,) how about using 'canonpath' to clean up the path in one step.
something like:

if ( $result =~ m{(.*) - (.*)} ) {
my $file = canonpath $1;
my $attribute = $2;

# and add to the results hash
$results{$file} = $attribute;
}

~jerry

[Ron Blaschke]

I may be missing something here, but I think the problem was that the
file name sets in MANIFEST and those reported by svn must match up, but
didn't because of the file separator. MANIFEST uses forward slashes,
File::Spec those of the current platform, which probably brings you back
to square one.

If you decide for File::Spec you should also canonicalize
@manifest_files, I guess.

Ron

[Paul Cochrane]

Done in the attached patch. Does this patch work for you jerry? I
can't test it on Windows unfortunately, but it works on Linux x86.

Paul

[Paul Cochrane]

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?

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).

Paul

[Ron Blaschke]

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. But canonpath does on Win32:

$ perl -MFile::Spec::Functions=canonpath
-e "print canonpath 'some\file.t'"
some\file.t

$ perl -MFile::Spec::Functions=canonpath
-e "print canonpath 'some/file.t'"
some\file.t

Here are the test results with the patch applied on Win32:

$ prove t\distro\file_metadata.t
<many lines snipped>...
# svn ps svn:eol-style 'LF' examples/shootout/takfp.pir.output;
# svn ps svn:eol-style 'LF' t/compilers/pge/p5regex/re_tests;
# svn ps svn:eol-style 'LF' t/library/perlhist.txt;
# svn ps svn:eol-style 'LF' t/op/sprintf_tests;
# '
# expected: ''
# Looks like you failed 4 tests of 4.
t\distro\file_metadata....dubious
Test returned status 4 (wstat 1024, 0x400)
DIED. FAILED tests 1-4
Failed 4/4 tests, 0.00% okay

Failed Test Stat Wstat Total Fail Failed List of Failed
-------------------------------------------------------------------------------

t\distro\file_metadata.t 4 1024 4 4 100.00% 1-4
Failed 1/1 test scripts, 0.00% okay. 4/4 subtests failed, 0.00% okay.

Ron

[Paul Cochrane]

> > 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.

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.

Paul

[Ron Blaschke]

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

[Paul Cochrane]

Ron,

> >> 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.

That's ok! No need to apologise. I was only pointing out what I was
misunderstanding :-)

> > 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.

I couldn't get your patch to apply cleanly and so hacked it in by
hand. I'm attaching a new patch to this email (which is quite
possibly identical to yours) so that you can give it a quick test. If
all is happy, then I'll commit the change and close the ticket.

Thanks heaps for your help!

Paul

[Ron Blaschke]

Paul Cochrane wrote:

> I couldn't get your patch to apply cleanly and so hacked it in by
> hand. I'm attaching a new patch to this email (which is quite
> possibly identical to yours) so that you can give it a quick test. If
> all is happy, then I'll commit the change and close the ticket.

I locally applied it to r19058 and works fine.

$ prove t/distro/file_metadata.t
t/distro/file_metadata....# Collecting svn:mime-type attributes...

# Collecting svn:keywords attributes...

t/distro/file_metadata....ok 2/4# Collecting svn:eol-style attributes...

# Collecting svn:eol-style attributes...

t/distro/file_metadata....ok
All tests successful.
Files=1, Tests=4, 30 wallclock secs ( 0.00 cusr + 0.00 csys = 0.00 CPU)

> Thanks heaps for your help!

Many thanks for cleaning this up!

Ron

[Paul Cochrane via RT]

After much help from Ron Blaschke to get this working nicely on Win32,
I've applied the latest patch as r19059 and am now closing the ticket.

Paul