First batch of whitespace patches

Hello Porters,

the first batch if patches is ready - my memory failed me in
the 
previous message - miniperl doesn't get built without a
small patch. 
These patches patch the following things:

1) perl-5.9.5-whitespace-Makefile.SH.diff

There is one instance in Makefile.SH where
     -I `pwd`/lib
gets used - this breaks if `pwd` contains whitespace.

2) perl-5.9.5-whitespace-ExtUtils.diff

ExtUtils::MM_Unix emits an unquoted $(ABSPERL) macro. This
patch puts 
$(ABSPERL) in quotes if that's needed. This will break any
program that 
uses EU:MM and does a file test on $(ABSPERL). But it makes
Perl build.

3) perl-5.9.5-whitespace-base.diff

This patch patches base/term.t which uses an unquoted $^X in
`` 
interpolation. The checks are done using regular expressions
- I'm not 
sure if that's safe/sane to do in a base test.

4) perl-5.9.5-whitespace-test.pl.diff

This patches test.pl to introduce quoted_perl(), a function
that returns 
a string fit for interpolation and also changes
_create_runperl() to use 
a quoted string where necessary.

This patch is a change that will be used by the subsequent
patches.

With these three/four patches, I get the following:

     Failed 58 tests out of 1369, 95.76% okay.

Which is a good start 

-max

Hi Max.

Could you regenerate your patches so that I can apply with
"patch -p1"
or similar, please? Right now, the "---" lines
have just the relative
path, while the "+++" lines have "../fresh
perl/perl-5.9.5/$path" and
patch can't cope with that.

At 2007-08-20 20:50:19 +0200, corioncorion.net wrote:
>
> 1) perl-5.9.5-whitespace-Makefile.SH.diff

Looks good.

> 2) perl-5.9.5-whitespace-ExtUtils.diff
> 
> ExtUtils::MM_Unix emits an unquoted $(ABSPERL) macro.
This patch puts 
> $(ABSPERL) in quotes if that's needed. This will break
any program that 
> uses EU:MM and does a file test on $(ABSPERL). But it
makes Perl build.

At what point does the build fail without this patch?

> 3) perl-5.9.5-whitespace-base.diff

Looks OK.

> 4) perl-5.9.5-whitespace-test.pl.diff

Looks OK.

> --- lib/ExtUtils/t/oneliner.t	2007-07-07
15:40:24.000000000 +0200
> +++ ../fresh
blead/perl-5.9.5/lib/ExtUtils/t/oneliner.t	2007-08-18
22:42:46.000000000 +0200
>  -28,6 +28,8 
>  sub try_oneliner {
>      my($code, $switches, $expect, $name) = _;
>      my $cmd = $mm->oneliner($code, $switches);
> +    my $Perl = $^X;
> +    $Perl = qq{$Perl} if $Perl =~ /s/ and $Perl !~
/^["']/;

I think you're missing some "" inside that qq?

> -}
>  Kein Zeilenumbruch am Dateiende.
> +}

Oh no! Not another missing Zeilenumbruch! 

(Thanks for splitting up the patches for review and
providing such a
useful summary, BTW. It's much appreciated.)

-- ams

Hello again,

attached please find a set of fixes to make the tests more
robust 
against whitespaces in the build directory path. All patches
should 
apply with

     patch -p 2 < ...

1) perl-5.9.5-whitespace-FileCheckTree-2.diff

I messed up the last patch - one test still was failing.
This one needs 
to go on top of the current blead.

2) perl-5.9.5-whitespace-io.diff

This is a plain patch.

3) perl-5.9.5-whitespace-run.diff

This fixes some of the failures in t/run, but not all. The
tests for -C 
and some other tests will likely need to be skipped if the
build path is 
found to contain whitespace. That'll go into a separate
discussion though.

-max

Hello again,

attached is the patch to make Test::Harness understand
whitespace in the 
build directory. I didn't write a separate test to test this
condition 
though.

The patch changes the quoting logic from hardwiring against
Windows to 
checking whether quoting seems to be needed and then quoting
regardless 
of OS.

-max

Hello again,

attached the patch to make the Test::Simple test suite safe
against 
whitespace in the build directory.

-max

Hello once more,

attached the patch to the test library of
ExtUtils::MakeMaker, 
MakeMaker::Test::Utils, and the tests:

1) perl-5.9.5-whitespace-MakeMakerTest.diff

This adds a/the function C<quoted_perl>, which also
lives in C<test.pl> 
already. But here it is also documented, a bit.

2) perl-5.9.5-whitespace-test.pl.diff

Modifies the tests to use quoted_perl() instead of
which_perl() where 
appropriate.

-max

Hi,

attached is the first big batch of core test patches. After
these are 
applied, I get:

Failed 10 tests out of 1370, 99.27% okay.
         ../ext/Compress/Zlib/t/05examples.t
         ../ext/Devel/DProf/t/DProf.t
         ../ext/IO_Compress_Zlib/t/010examples.t
         ../lib/Archive/Extract/t/01_Archive-Extract.t
        
../lib/CPANPLUS/Dist/Build/t/02_CPANPLUS-Dist-Build.t
         ../lib/CPANPLUS/t/20_CPANPLUS-Dist-MM.t
         ../lib/IPC/Cmd/t/01_IPC-Cmd.t
         ../lib/Module/Build/t/compat.t
         comp/cpp.t
         run/switchPx.t

The following tests remain problematic/unfixable as I think
they rely on 
the intelligence of external tools:

t/comp/cpp
t/run/switchPx

I have the feeling that the failures in Module::Build and 
Archive::Extract do not stem from a faulty test suite, but I
haven't 
investigated yet.

To the patches:

1) perl-5.9.5-whitespace-io.diff

Plain vanilla upgrade from C<which_perl> to
C<quoted_perl>, using $QPerl 
and $Perl where both are needed. No test failures remain in
t/io/

2) perl-5.9.5-whitespace-run.diff

Upgrade C<which_perl> to C<quoted_perl>.
t/run/switchPx.t remains as
a failure to be investigated.

3) perl-5.9.5-whitespace-lib.diff

The patch to C<filter-util.pl> is a bit unclean as
that one might fail 
on MacOS if the build path contains whitespace and the MacOS
shell (?) 
does not understand double quotes.

4) perl-5.9.5-whitespace-comp.diff

This one attempts to fix comp/cpp.t, but fails, most likely
because 
somewhere the shell gets involved, which does not understand
whitespace 
in a hashbang line.

5a) perl-5.9.5-whitespace-h2ph.diff
5b) perl-5.9.5-whitespace-h2xs.diff
5c) perl-5.9.5-whitespace-AutoSplit.diff
5d) perl-5.9.5-whitespace-B.diff
5e) perl-5.9.5-whitespace-IO_.diff
5f) perl-5.9.5-whitespace-SelfStubber.diff
5g) perl-5.9.5-whitespace-IPCOpen3.diff

Low hanging fruit of simple quoting upgrades

6) perl-5.9.5-whitespace-op.diff

This one does basic quoting. One test of note is
t/op/magic.t , which
involves creating a shell script and then running that. This
test is 
skipped on non-Win32 OSes. I'm not sure if that approach is
too broad.

7) perl-5.9.5-whitespace-strict.diff

The case of a MacOS build directory containing whitespace
remains 
unhandled here.

That's the low hanging fruit so far. Next up will be the
remaining 
failures, but I wanted to get these patches out to get early
feedback 
before I produce more 

-max

On Aug 24, 2007, at 3:04 PM, Max Maischein wrote:

> The patch changes the quoting logic from hardwiring
against Windows  
> to checking whether quoting seems to be needed and then
quoting  
> regardless of OS.

Thanks for this.  I've applied it to Test::Harness.

In the future, though, people who apply patches to
Test::Harness  
should submit RT tickets, too, so I don't lose track of
them.

xoxo,
Andy

--
Andy Lester => andypetdance.com => www.petdance.com =>
AIM:petdance

Max Maischein wrote:
> Hello once more,
> 
> attached the patch to the test library of
ExtUtils::MakeMaker,
> MakeMaker::Test::Utils, and the tests:
> 
> 1) perl-5.9.5-whitespace-MakeMakerTest.diff
> 
> This adds a/the function C<quoted_perl>, which
also lives in C<test.pl>
> already. But here it is also documented, a bit.
> 
> 2) perl-5.9.5-whitespace-test.pl.diff
> 
> Modifies the tests to use quoted_perl() instead of
which_perl() where
> appropriate.

Thanks for all that work.  But I look at it and... guh, its
so much WORK!  I
keep thinking there's got to be a lazier way to handle the
problem.


-- 
Hating the web since 1994.

Max Maischein wrote:
>> ...
>> Thanks for all that work.  But I look at it and...
guh, its so much
>> WORK!  I
>> keep thinking there's got to be a lazier way to
handle the problem.
> 
> How about using
> 
>     system(LIST)

Alas, need to capture output.


> or the list form of backticks instead of interpolating
strings to feed
> to the shell.

What list form of backticks?


> That would be safer and saner. Except I seem to
remember that
> 
>     open "-|", ...
> 
> has problems on Win32, but the problems I encountered
might have been
> buffering problems.
> 
> I see no way to do clever modification of $^X, because
the tests partly
> use system(LIST), which works if $^X contains
whitespace but fails if
> $^X is quoted shell-safe, and use $^X in string
interpolation, which
> fails if $^X contains whitespace unless $^X is quoted
shell-safe.

My lament was sort of a general lament not specific to
MakeMaker.  A "the
programmer shouldn't have to be this careful" thing.

A version of system(LIST) which captured would go a long way
towards helping.


-- 
Whip me, beat me, make my code compatible with VMS!