Please review the patch attached.  Note the following:

1.  As mentioned in my last post in this RT, the flow in this step
class's runstep() method is quite convoluted.  I tried to improve it,
but this step still has five different points at which it can return.  I
have, however, created a distinction between those variables which hold
the user's command-line options and those which hold the current state
of the user's attempt to configure with those options.

2.  I suspect that some of the command-line options haven't been used in
years.  Evidence?  The fact that one command-line, --icudatadir, is
mentioned in the Configure.pl --help message (held in
lib/Parrot/Configure/Options/Conf.pm) and in README_win32.pod -- but is
not mentioned anywhere in config/auto/icu.pm, or anyplace else for that
matter!  The fact that we have lived happily without this option since
r14398 provides the rationale to remove it from the files where it is
mentioned.

Moreover, I have heard no response to the request in my earlier post for
feedback from people who regularly use the --icushared and --icuheaders
options.  I suspect that in practice we could do without those options,
but have taken no action to remove them.

So, if you have any strong feelings about this patch, please speak up. 
Otherwise I will apply it to trunk in 3 days (or 2, if I get impatient).

Thank you very much.
kid51

Index: lib/Parrot/Configure/Options/Conf.pm
===================================================================
--- lib/Parrot/Configure/Options/Conf.pm        (.../trunk)     (revision 28735)
+++ lib/Parrot/Configure/Options/Conf.pm        (.../branches/autoicu)  
(revision 28926)
@@ -37,7 +37,6 @@
     gc
     help
     icu-config
-    icudatadir
     icuheaders
     icushared
     includedir
@@ -213,7 +212,6 @@
    --without-icu        Build parrot without ICU support
    --icuheaders=(path)  Location of ICU headers without /unicode
    --icushared=(flags)  Full linker command to create shared libraries
-   --icudatadir=(path)  Directory to locate ICU's data file(s)
 
 Other Options (may not be implemented):
 
Index: MANIFEST
===================================================================
--- MANIFEST    (.../trunk)     (revision 28735)
+++ MANIFEST    (.../branches/autoicu)  (revision 28926)
@@ -1,7 +1,7 @@
 # ex: set ro:
 # $Id$
 #
-# generated by tools/dev/mk_manifest_and_skip.pl Fri Jun 27 01:45:45 2008 UT
+# generated by tools/dev/mk_manifest_and_skip.pl Tue Jul  1 21:05:08 2008 UT
 #
 # See tools/dev/install_files.pl for documentation on the
 # format of this file.
@@ -3673,6 +3673,10 @@
 t/steps/auto_headers-03.t                                   []
 t/steps/auto_headers-04.t                                   []
 t/steps/auto_icu-01.t                                       []
+t/steps/auto_icu-02.t                                       []
+t/steps/auto_icu-03.t                                       []
+t/steps/auto_icu-04.t                                       []
+t/steps/auto_icu-05.t                                       []
 t/steps/auto_inline-01.t                                    []
 t/steps/auto_inline-02.t                                    []
 t/steps/auto_inline-03.t                                    []
Index: README_win32.pod
===================================================================
--- README_win32.pod    (.../trunk)     (revision 28735)
+++ README_win32.pod    (.../branches/autoicu)  (revision 28926)
@@ -75,7 +75,6 @@
     perl Configure.pl
         --icushared="C:\usr\lib\icu\lib\icudt.lib C:\usr\lib\icu\lib\icuuc.lib"
         --icuheaders="C:\usr\lib\icu\include"
-        --icudatadir="C:\usr\local\icu\data"
 
 Note the step of creating the F<C:\usr\lib\data> directory, as Parrot
 really wants it and the binary packages don't contain it.  It doesn't
@@ -178,7 +177,6 @@
          --cc=gcc
          --icushared="C:\usr\lib\icu\lib\icudt.lib 
C:\usr\lib\icu\lib\icuuc.lib"
          --icuheaders="C:\usr\lib\icu\include"
-         --icudatadir="C:\usr\local\icu\data"
 or
     perl Configure.pl --cc=gcc --without-icu
 
Index: t/steps/auto_icu-03.t
===================================================================
--- t/steps/auto_icu-03.t       (.../trunk)     (revision 0)
+++ t/steps/auto_icu-03.t       (.../branches/autoicu)  (revision 28926)
@@ -0,0 +1,109 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# auto_icu-03.t
+
+use strict;
+use warnings;
+use Test::More;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+use Parrot::Configure::Utils qw( capture_output );
+{
+    my ($stdout, $stderr, $ret);
+    eval { ($stdout, $stderr, $ret) =
+        capture_output( qw| icu-config --exists | ); };
+    if ($stderr) {
+        plan skip_all => "icu-config not available";
+    }
+    else {
+        plan tests => 14;
+    }
+}
+use_ok('config::init::defaults');
+use_ok('config::auto::icu');
+use IO::CaptureOutput qw( capture );
+
+my $args = process_options(
+    {
+        argv => [
+            q{--icu-config=1},
+            q{--icuheaders=alpha},
+            q{--icushared=beta}
+        ],
+        mode => q{configure},
+    }
+);
+
+my $conf = Parrot::Configure->new;
+
+test_step_thru_runstep( $conf, q{init::defaults}, $args );
+
+my $pkg = q{auto::icu};
+
+$conf->add_steps($pkg);
+$conf->options->set( %{$args} );
+
+my ( $task, $step_name, $step);
+$task        = $conf->steps->[1];
+$step_name   = $task->step;
+
+$step = $step_name->new();
+ok( defined $step, "$step_name constructor returned defined value" );
+isa_ok( $step, $step_name );
+ok( $step->description(), "$step_name has description" );
+
+{
+    my ($stdout, $stderr, $ret);
+    capture(
+        sub { $ret = $step->runstep($conf); },
+        \$stdout,
+        \$stderr,
+    );
+    ok(! defined $ret, "runstep() returned undefined value as expected");
+    like($stderr, qr/error: icushared not defined/s,
+        "Got expected warnings");
+    like($stderr, qr/error: icuheaders not defined or invalid/s,
+        "Got expected warnings");
+    like($stderr, qr/Something is wrong with your ICU installation/s,
+        "Got expected warnings");
+}
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+auto_icu-03.t - test config::auto::icu
+
+=head1 SYNOPSIS
+
+    % prove t/steps/auto_icu-03.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test config::auto::icu with command-line options
+C<--icushared> and C<--icuheaders>.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::icu, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+#   mode: cperl
+#   cperl-indent-level: 4
+#   fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:

Property changes on: t/steps/auto_icu-03.t
___________________________________________________________________
Name: svn:eol-style
   + native
Name: svn:mime-type
   + text/plain
Name: svn:keywords
   + Author Date Id Revision

Index: t/steps/auto_icu-04.t
===================================================================
--- t/steps/auto_icu-04.t       (.../trunk)     (revision 0)
+++ t/steps/auto_icu-04.t       (.../branches/autoicu)  (revision 28926)
@@ -0,0 +1,85 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# auto_icu-04.t
+
+use strict;
+use warnings;
+use Test::More tests => 12;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+use_ok('config::init::defaults');
+use_ok('config::auto::icu');
+
+my $args = process_options(
+    {
+        argv => [ ],
+        mode => q{configure},
+    }
+);
+
+my $conf = Parrot::Configure->new;
+
+test_step_thru_runstep( $conf, q{init::defaults}, $args );
+
+my $pkg = q{auto::icu};
+
+$conf->add_steps($pkg);
+$conf->options->set( %{$args} );
+
+my ( $task, $step_name, $step);
+$task        = $conf->steps->[1];
+$step_name   = $task->step;
+
+$step = $step_name->new();
+ok( defined $step, "$step_name constructor returned defined value" );
+isa_ok( $step, $step_name );
+ok( $step->description(), "$step_name has description" );
+
+my $phony = q{phony};
+$step->{icuconfig_default} = $phony;
+
+my $ret = $step->runstep($conf);
+ok( $ret, "$step_name runstep() returned true value" );
+my $expected = q{failed};
+is($step->result(), $expected,
+    "Got expected return value: $expected");
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+auto_icu-04.t - test config::auto::icu
+
+=head1 SYNOPSIS
+
+    % prove t/steps/auto_icu-04.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test config::auto::icu in the case where an alternate
+ICU configuration program is utilized.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::icu, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+#   mode: cperl
+#   cperl-indent-level: 4
+#   fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:

Property changes on: t/steps/auto_icu-04.t
___________________________________________________________________
Name: svn:eol-style
   + native
Name: svn:mime-type
   + text/plain
Name: svn:keywords
   + Author Date Id Revision

Index: t/steps/auto_icu-01.t
===================================================================
--- t/steps/auto_icu-01.t       (.../trunk)     (revision 28735)
+++ t/steps/auto_icu-01.t       (.../branches/autoicu)  (revision 28926)
@@ -5,18 +5,436 @@
 
 use strict;
 use warnings;
-use Test::More tests =>  2;
+use Test::More tests => 81;
 use Carp;
-use lib qw( lib );
+use Cwd;
+use File::Path qw( mkpath );
+use File::Temp qw( tempdir );
+use lib qw( lib t/configure/testlib );
+use_ok('config::init::defaults');
 use_ok('config::auto::icu');
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+use IO::CaptureOutput qw( capture );
 
-=for hints_for_testing Provide an explanation in the POD for what 'ICU'
-is.  Try to test all branches and conditions; consult a recent code
-coverage report for guidance.  See if you can write tests which generate
-the 'die's, then capture the error messages and analyze them.
+my $args = process_options(
+    {
+        argv => [ q{--without-icu}  ],
+        mode => q{configure},
+    }
+);
 
-=cut
+my $conf = Parrot::Configure->new;
 
+test_step_thru_runstep( $conf, q{init::defaults}, $args );
+
+my $pkg = q{auto::icu};
+
+$conf->add_steps($pkg);
+$conf->options->set( %{$args} );
+
+my ( $task, $step_name, $step);
+$task        = $conf->steps->[1];
+$step_name   = $task->step;
+
+$step = $step_name->new();
+ok( defined $step, "$step_name constructor returned defined value" );
+isa_ok( $step, $step_name );
+ok( $step->description(), "$step_name has description" );
+
+my $ret = $step->runstep($conf);
+ok( $ret, "$step_name runstep() returned true value" );
+
+is( $conf->data->get('has_icu'), 0,
+    "Got expected value for 'has_icu'" );
+is( $conf->data->get('icu_shared'), q{},
+    "Got expected value for 'icu_shared'" );
+is( $conf->data->get('icu_dir'), q{},
+    "Got expected value for 'icu_dir'" );
+is( $step->result(), 'no', "Got expected result" );
+$step->set_result(q{});  # prepare for subsequent tests
+
+# Test some internal routines
+my $icuconfig;
+my $phony = q{/path/to/icu-config};
+
+is($step->_handle_icuconfig_opt(undef), q{},
+    "Got expected value for icu-config");
+is($step->_handle_icuconfig_opt('none'), q{},
+    "Got expected value for icu-config");
+is($step->_handle_icuconfig_opt($phony), $phony,
+    "Got expected value for icu-config");
+
+my ($autodetect, $without);
+
+($icuconfig, $autodetect, $without) =
+    $step->_handle_search_for_icu_config( {
+        icuconfig   => q{},
+        autodetect  => 1,
+        without     => 0,
+        verbose     => 0,
+        ret         => -1,
+} );
+ok(! defined $icuconfig, "icu-config not found, as expected");
+is($autodetect, 0, "Autodetection cancelled, as expected");
+is($without, 1, "Continuing to configure without ICU");
+
+($icuconfig, $autodetect, $without) =
+    $step->_handle_search_for_icu_config( {
+        icuconfig   => q{},
+        autodetect  => 1,
+        without     => 0,
+        verbose     => 0,
+        ret         => 256,
+} );
+ok(! defined $icuconfig, "icu-config not found, as expected");
+is($autodetect, 0, "Autodetection cancelled, as expected");
+is($without, 1, "Continuing to configure without ICU");
+
+($icuconfig, $autodetect, $without) =
+    $step->_handle_search_for_icu_config( {
+        icuconfig   => q{},
+        autodetect  => 1,
+        without     => 0,
+        verbose     => 0,
+        ret         => 0,
+} );
+is($icuconfig, q{icu-config}, "icu-config found, as expected");
+is($autodetect, 1, "Autodetection continues, as expected");
+is($without, 0, "Continuing to try to configure with ICU");
+
+{
+    my ($stdout, $stderr);
+    capture( sub {
+            ($icuconfig, $autodetect, $without) =
+                $step->_handle_search_for_icu_config( {
+                    icuconfig   => q{},
+                    autodetect  => 1,
+                    without     => 0,
+                    verbose     => 1,
+                    ret         => 0,
+            } );
+        },
+        \$stdout,
+        \$stderr,
+    );
+    is($icuconfig, q{icu-config}, "icu-config found, as expected");
+    is($autodetect, 1, "Autodetection continues, as expected");
+    is($without, 0, "Continuing to try to configure with ICU");
+    like($stdout, qr/icu-config found/,
+        "Got expected verbose output");
+}
+
+($icuconfig, $autodetect, $without) =
+    $step->_handle_autodetect( {
+        icuconfig   => $phony,
+        autodetect  => 1,
+        without     => 0,
+        verbose     => 0,
+} );
+is($icuconfig, $phony, "icu-config unchanged, as expected");
+is($autodetect, 1, "Autodetection still active, as expected");
+is($without, 0, "Continuing to try to configure with ICU");
+
+{
+    my ($stdout, $stderr);
+    capture( sub {
+        ($icuconfig, $autodetect, $without) =
+            $step->_handle_autodetect( {
+                icuconfig   => $phony,
+                autodetect  => 0,
+                without     => 0,
+                verbose     => 1,
+            } );
+        },
+        \$stdout,
+        \$stderr,
+    );
+    is($icuconfig, $phony, "icu-config unchanged, as expected");
+    is($autodetect, 0, "Autodetection still inactive, as expected");
+    is($without, 0, "Continuing to try to configure with ICU");
+    like($stdout, qr/ICU autodetection disabled/s,
+        "Got expected verbose output");
+}
+
+my $icushared;
+
+$icushared = qq{-licui18n -lalpha\n};
+($icushared, $without) = $step->_handle_icushared($icushared, 0);
+like($icushared, qr/-lalpha/, "Got expected ld flags");
+is($without, 0, "Continuing to try to configure with ICU");
+
+$icushared = qq{-licui18n\n};
+($icushared, $without) = $step->_handle_icushared($icushared, 0);
+ok(! $icushared, "No icushared, as expected");
+is($without, 1, "No longer trying to configure with ICU");
+
+$icushared = undef;
+($icushared, $without) = $step->_handle_icushared($icushared, 0);
+ok(! defined $icushared, "icushared remains undefined, as expected");
+is($without, 0, "Continuing to try to configure with ICU");
+
+my $icuheaders;
+($icuheaders, $without) =
+    $step->_handle_icuheaders($conf, undef, 0);
+ok(! defined $icuheaders, "icuheaders path undefined, as expected");
+is($without, 0, "Continuing to try to configure with ICU");
+
+my $cwd = cwd();
+{
+    my $tdir = tempdir();
+    chdir $tdir or croak "Unable to change to temporary directory";
+    my $expected_dir = q{alpha};
+    my $expected_include_dir =
+        $expected_dir . $conf->data->get('slash') .  q{include};
+    ($icuheaders, $without) =
+        $step->_handle_icuheaders($conf, qq{$expected_dir\n}, 0);
+    is($icuheaders, $expected_include_dir,
+        "Got expected icuheaders path value");
+    is($without, 1, "No longer trying to configure with ICU");
+    chdir $cwd or croak "Unable to change back to starting directory";
+}
+
+{
+    my $tdir = tempdir();
+    chdir $tdir or croak "Unable to change to temporary directory";
+    my $expected_dir = q{alpha};
+    my $expected_include_dir =
+        $expected_dir . $conf->data->get('slash') .  q{include};
+    mkdir $expected_dir or croak "Unable to make testing directory";
+    ($icuheaders, $without) =
+        $step->_handle_icuheaders($conf, qq{$expected_dir\n}, 0);
+    is($icuheaders, $expected_include_dir,
+        "Got expected icuheaders path value");
+    is($without, 1, "No longer trying to configure with ICU");
+    chdir $cwd or croak "Unable to change back to starting directory";
+}
+
+{
+    my $tdir = tempdir();
+    chdir $tdir or croak "Unable to change to temporary directory";
+    my $expected_dir = q{alpha};
+    my $expected_include_dir =
+        $expected_dir . $conf->data->get('slash') .  q{include};
+    mkdir $expected_dir or croak "Unable to make testing directory";
+    mkpath($expected_include_dir, 0, 755)
+        or croak "Unable to make second-level testing directory";
+    ($icuheaders, $without) =
+        $step->_handle_icuheaders($conf, qq{$expected_dir\n}, 0);
+    is($icuheaders, $expected_include_dir,
+        "Got expected icuheaders path value");
+    is($without, 0, "Continuing to try to configure with ICU");
+    chdir $cwd or croak "Unable to change back to starting directory";
+}
+
+($without, $icushared, $icuheaders) =
+    $step->_try_icuconfig(
+        $conf,
+        {
+            without         => 1,
+            autodetect      => 1,
+            icuconfig       => 1,
+            verbose         => 0,
+        }
+    );
+is($without, 1, "Not trying to configure with ICU");
+ok(! defined $icushared, "icushared undefined, as expected");
+ok(! defined $icuheaders, "icuheaders undefined, as expected");
+is($step->result(), q{}, "result is still empty string, as expected");
+
+($without, $icushared, $icuheaders) =
+    $step->_try_icuconfig(
+        $conf,
+        {
+            without         => 0,
+            autodetect      => 0,
+            icuconfig       => 1,
+            verbose         => 0,
+        }
+    );
+is($without, 0, "Still trying to configure with ICU");
+ok(! defined $icushared, "icushared undefined, as expected");
+ok(! defined $icuheaders, "icuheaders undefined, as expected");
+is($step->result(), q{}, "result is still empty string, as expected");
+
+($without, $icushared, $icuheaders) =
+    $step->_try_icuconfig(
+        $conf,
+        {
+            without         => 0,
+            autodetect      => 1,
+            icuconfig       => q{},
+            verbose         => 0,
+        }
+    );
+is($without, 0, "Still trying to configure with ICU");
+ok(! defined $icushared, "icushared undefined, as expected");
+ok(! defined $icuheaders, "icuheaders undefined, as expected");
+is($step->result(), q{}, "result is still empty string, as expected");
+
+my $die = auto::icu::_die_message();
+like($die, qr/Something is wrong with your ICU installation/s,
+    "Got expected die message");
+
+{   
+    my $phony = q{/path/to/icu-config};
+    my ($stdout, $stderr);
+    capture(
+        sub { auto::icu::_verbose_report(1, $phony, undef, undef); },
+        \$stdout,
+        \$stderr,
+    );
+    like( $stdout, qr/icuconfig:\s+$phony/s,
+        "Got expected verbose output"
+    );
+}
+
+{   
+    my $phony = q{-lalpha};
+    my ($stdout, $stderr);
+    capture(
+        sub { auto::icu::_verbose_report(1, undef, $phony, undef); },
+        \$stdout,
+        \$stderr,
+    );
+    like( $stdout, qr/icushared='$phony'/s,
+        "Got expected verbose output"
+    );
+}
+
+{   
+    my $phony = q{alpha/include};
+    my ($stdout, $stderr);
+    capture(
+        sub { auto::icu::_verbose_report(1, undef, undef, $phony); },
+        \$stdout,
+        \$stderr,
+    );
+    like( $stdout, qr/headers='$phony'/s,
+        "Got expected verbose output"
+    );
+}
+
+{   
+    my ($stdout, $stderr);
+    capture(
+        sub { auto::icu::_verbose_report(0, 'alpha', 'beta', 'gamma'); },
+        \$stdout,
+        \$stderr,
+    );
+    ok(! $stdout, "No verbose output, as expected");
+}
+
+{
+    my ($stdout, $stderr);
+    capture(
+        sub {
+            $icuheaders = $step->_handle_icuconfig_errors( {
+                icushared   => undef,
+                icuheaders  => undef,
+            } );
+        },
+        \$stdout,
+        \$stderr,
+    );
+    like($stderr, qr/error: icushared not defined/s,
+        "Got expected warnings");
+    like($stderr, qr/error: icuheaders not defined or invalid/s,
+        "Got expected warnings");
+    like($stderr, qr/Something is wrong with your ICU installation/s,
+        "Got expected warnings");
+}
+
+$icuheaders = q{alpha};
+my $status = $conf->data->get( 'ccflags' );
+
+{
+    my ($stdout, $stderr);
+    capture(
+        sub {
+           auto::icu::_handle_ccflags_status($conf,
+               {
+                   ccflags_status  => 1,
+                   verbose         => 1,
+                   icuheaders      => $icuheaders,
+               },
+           );
+       },
+       \$stdout,
+       \$stderr,
+   );
+   like($stdout, qr/Your compiler found the icu headers/,
+       "Got expected verbose output");
+}
+$conf->data->set( ccflags => $status ); # re-set for next test
+
+{
+    my ($stdout, $stderr);
+    capture(
+        sub {
+           auto::icu::_handle_ccflags_status($conf,
+               {
+                   ccflags_status  => 0,
+                   verbose         => 1,
+                   icuheaders      => $icuheaders,
+               },
+           );
+       },
+       \$stdout,
+       \$stderr,
+   );
+
+   like($stdout, qr/Adding -I $icuheaders to ccflags for icu headers/,
+       "Got expected verbose output");
+}
+like($conf->data->get( 'ccflags'),
+    qr/-I $icuheaders/,
+    "ccflags augmented as expected"
+);
+$conf->data->set( ccflags => $status ); # re-set for next test
+
+{
+    my ($stdout, $stderr);
+    capture(
+        sub {
+           auto::icu::_handle_ccflags_status($conf,
+               {
+                   ccflags_status  => 0,
+                   verbose         => 0,
+                   icuheaders      => $icuheaders,
+               },
+           );
+       },
+       \$stdout,
+       \$stderr,
+   );
+
+   ok(! $stdout, "No verbose output, as expected");
+}
+like($conf->data->get( 'ccflags'),
+    qr/-I $icuheaders/,
+    "ccflags augmented as expected"
+);
+$conf->data->set( ccflags => $status ); # re-set for next test
+
+$conf->data->set( 'has_icu', undef );
+$conf->data->set( 'icu_shared', undef );
+$conf->data->set( 'icu_dir', undef );
+my $result = q{hooray!};
+$step->_set_no_configure_with_icu($conf, $result);
+is($conf->data->get( 'has_icu' ), 0,
+    "Got expected value for 'has_icu'");
+is($conf->data->get( 'icu_shared' ), q{},
+    "Got expected value for 'icu_shared'");
+is($conf->data->get( 'icu_dir' ), q{},
+    "Got expected value for 'icu_dir'");
+is($step->{result}, $result, "Got expected result");
+# reset for next test
+$conf->data->set( 'has_icu', undef );
+$conf->data->set( 'icu_shared', undef );
+$conf->data->set( 'icu_dir', undef );
+
 pass("Completed all tests in $0");
 
 ################### DOCUMENTATION ###################
@@ -33,7 +451,9 @@
 
 The files in this directory test functionality used by F<Configure.pl>.
 
-The tests in this file test subroutines exported by config::auto::icu.
+The tests in this file test config::auto::icu in the case where configuration
+without ICU is requested.  Also tested are most of the internal subroutines
+and methods.
 
 =head1 AUTHOR
 
Index: t/steps/auto_icu-05.t
===================================================================
--- t/steps/auto_icu-05.t       (.../trunk)     (revision 0)
+++ t/steps/auto_icu-05.t       (.../branches/autoicu)  (revision 28926)
@@ -0,0 +1,106 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# auto_icu-05.t
+
+use strict;
+use warnings;
+use Test::More;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+use Parrot::Configure::Utils qw( capture_output );
+{
+    my ($stdout, $stderr, $ret);
+    eval { ($stdout, $stderr, $ret) =
+        capture_output( qw| icu-config --exists | ); };
+    if ($stderr) {
+        plan skip_all => "icu-config not available";
+    }
+    else {
+        plan tests => 14;
+    }
+}
+use_ok('config::init::defaults');
+use_ok('config::auto::icu');
+use IO::CaptureOutput qw( capture );
+
+my $args = process_options(
+    {
+        argv => [ q{--icuheaders=alpha}, ],
+        mode => q{configure},
+    }
+);
+
+my $conf = Parrot::Configure->new;
+
+test_step_thru_runstep( $conf, q{init::defaults}, $args );
+
+my $pkg = q{auto::icu};
+
+$conf->add_steps($pkg);
+$conf->options->set( %{$args} );
+
+my ( $task, $step_name, $step);
+$task        = $conf->steps->[1];
+$step_name   = $task->step;
+
+$step = $step_name->new();
+ok( defined $step, "$step_name constructor returned defined value" );
+isa_ok( $step, $step_name );
+ok( $step->description(), "$step_name has description" );
+
+{
+    my ($stdout, $stderr, $ret);
+    capture(
+        sub { $ret = $step->runstep($conf); },
+        \$stdout,
+        \$stderr,
+    );
+    ok(! defined $ret, "runstep() returned undefined value as expected");
+    like($stderr, qr/error: icushared not defined/s,
+        "Got expected warnings");
+    like($stderr, qr/error: icuheaders not defined or invalid/s,
+        "Got expected warnings");
+    like($stderr, qr/Something is wrong with your ICU installation/s,
+        "Got expected warnings");
+}
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+auto_icu-05.t - test config::auto::icu
+
+=head1 SYNOPSIS
+
+    % prove t/steps/auto_icu-05.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test config::auto::icu in the case where the
+command-line options are incomplete.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::icu, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+#   mode: cperl
+#   cperl-indent-level: 4
+#   fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:
+

Property changes on: t/steps/auto_icu-05.t
___________________________________________________________________
Name: svn:eol-style
   + native
Name: svn:mime-type
   + text/plain
Name: svn:keywords
   + Author Date Id Revision

Index: t/steps/auto_icu-02.t
===================================================================
--- t/steps/auto_icu-02.t       (.../trunk)     (revision 0)
+++ t/steps/auto_icu-02.t       (.../branches/autoicu)  (revision 28926)
@@ -0,0 +1,87 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# auto_icu-02.t
+
+use strict;
+use warnings;
+use Test::More tests => 15;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use_ok('config::init::defaults');
+use_ok('config::auto::icu');
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+
+my $args = process_options(
+    {
+        argv => [ q{--without-icu}, q{--icu-config=none}  ],
+        mode => q{configure},
+    }
+);
+
+my $conf = Parrot::Configure->new;
+
+test_step_thru_runstep( $conf, q{init::defaults}, $args );
+
+my $pkg = q{auto::icu};
+
+$conf->add_steps($pkg);
+$conf->options->set( %{$args} );
+
+my ( $task, $step_name, $step);
+$task        = $conf->steps->[1];
+$step_name   = $task->step;
+
+$step = $step_name->new();
+ok( defined $step, "$step_name constructor returned defined value" );
+isa_ok( $step, $step_name );
+ok( $step->description(), "$step_name has description" );
+
+my $ret = $step->runstep($conf);
+ok( $ret, "$step_name runstep() returned true value" );
+
+is( $conf->data->get('has_icu'), 0,
+    "Got expected value for 'has_icu'" );
+is( $conf->data->get('icu_shared'), q{},
+    "Got expected value for 'icu_shared'" );
+is( $conf->data->get('icu_dir'), q{},
+    "Got expected value for 'icu_dir'" );
+is( $step->result(), 'no', "Got expected result" );
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+auto_icu-02.t - test config::auto::icu
+
+=head1 SYNOPSIS
+
+    % prove t/steps/auto_icu-02.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test config::auto::icu with command-line option
+C<--icu-config=none>.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::icu, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+#   mode: cperl
+#   cperl-indent-level: 4
+#   fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:

Property changes on: t/steps/auto_icu-02.t
___________________________________________________________________
Name: svn:eol-style
   + native
Name: svn:mime-type
   + text/plain
Name: svn:keywords
   + Author Date Id Revision

Index: config/auto/icu.pm
===================================================================
--- config/auto/icu.pm  (.../trunk)     (revision 28735)
+++ config/auto/icu.pm  (.../branches/autoicu)  (revision 28926)
@@ -7,7 +7,7 @@
 
 =head1 DESCRIPTION
 
-Determines whether ICU is available.  If so, configures ICU and add
+Determines whether ICU is available.  If so, configures ICU and adds
 appropriate targets to the Makefile.
 
 From the ICU home page (L<http://www.icu-project.org/>):  "ICU is a mature,
@@ -31,16 +31,22 @@
 sub _init {
     my $self = shift;
     my %data;
-    $data{description} = q{Determining whether ICU is installed};
-    $data{result}      = q{};
+    $data{description}          = q{Determining whether ICU is installed};
+    $data{result}               = q{};
+    # The following key-value pairs are defined here rather than being buried
+    # deep inside subroutines below.  Also, so that they can be overridden
+    # during testing.
+    $data{icuconfig_default}    = q{icu-config};
+    $data{icu_headers}          = [ qw(ucnv.h utypes.h uchar.h) ];
+    $data{icu_shared_pattern}   = qr/-licui18n\w*/;
     return \%data;
 }
 
 sub runstep {
     my ( $self, $conf ) = @_;
 
-    my ( $verbose, $icushared, $icuheaders, $icuconfig, $without ) =
-        $conf->options->get( qw|
+    my ( $verbose, $icushared_opt, $icuheaders_opt,
+        $icuconfig_opt, $without_opt ) = $conf->options->get( qw|
             verbose
             icushared
             icuheaders
@@ -48,106 +54,156 @@
             without-icu
     | );
 
-    my @icu_headers = qw(ucnv.h utypes.h uchar.h);
-    my $autodetect  = !defined($icushared) && !defined($icuheaders);
+    # If we haven't provided the path to a specific ICU configuration program
+    # on the command line, then we probe to see if the program 'icu-config' is
+    # available.
 
-    $self->set_result(undef);
-    unless ($without) {
-        if ( !$autodetect ) {
-            print "specified a icu config parameter,\nICU autodetection 
disabled.\n" if $verbose;
-        }
-        elsif ( !defined $icuconfig || !$icuconfig ) {
+    # From the icu-config(1) man page
+    # (L<http://linux.die.net/man/1/icu-config>):
 
-            # From the icu-config(1) man page
-            # (L<http://linux.die.net/man/1/icu-config>):
+    # "icu-config simplifies the task of building and linking
+    # against ICU as compared to manually configuring user
+    # makefiles or equivalent. Because icu-config is an executable
+    # script, it also solves the problem of locating the ICU
+    # libraries and headers, by allowing the system PATH to locate
+    # it."
 
-            # "icu-config simplifies the task of building and linking against
-            # ICU as compared to manually configuring user makefiles or
-            # equivalent. Because icu-config is an executable script, it also
-            # solves the problem of locating the ICU libraries and headers, by
-            # allowing the system PATH to locate it."
+    # $icuconfig is a string holding the name of an executable program.
+    # So if it's not provided on the command line -- or if it's explicitly
+    # ruled out by being provided with the value 'none' -- an empty string
+    # is its most appropriate value.
 
-            my ( undef, undef, $ret ) = capture_output( "icu-config", 
"--exists" );
+    my $icuconfig = $self->_handle_icuconfig_opt($icuconfig_opt);
 
-            if ( ( $ret == -1 ) || ( ( $ret >> 8 ) != 0 ) ) {
-                undef $icuconfig;
-                $autodetect = 0;
-                $without    = 1;
-            }
-            else {
-                $icuconfig = "icu-config";
-                print "icu-config found... good!\n" if $verbose;
-            }
-        }
+    # Oooh, how I wish we had Perl 5.10's defined-or operator available!
+    my $icushared = (defined $icushared_opt)
+        ? $icushared_opt
+        : undef;
+    my $icuheaders = (defined $icuheaders_opt)
+        ? $icuheaders_opt
+        : undef;
 
-        if ( !$without && $autodetect && $icuconfig && $icuconfig ne "none" ) {
-            my $slash = $conf->data->get('slash');
+    # $without_opt holds user's command-line value for --without-icu=?
+    # If it's a true value, there's no point in going further.  We set the
+    # values needed in the Parrot::Configure object, set the step result and
+    # return.  If, however, it's a false value, then we're going to try to
+    # configure with ICU and we proceed to probe for ICU.
 
-            # icu-config script to use
-            $icuconfig = "icu-config" if $icuconfig eq "1";
+    # 1st possible return point
+    if ( $without_opt ) {
+        $self->_set_no_configure_with_icu($conf, q{no});
+        return 1;
+    }
 
-            # ldflags
-            $icushared = capture_output("$icuconfig --ldflags");
-            if ( defined $icushared ) {
-                chomp $icushared;
-                $icushared =~ s/-licui18n\w*//;    # "-licui18n32" too
-                $without = 1 if length $icushared == 0;
-            }
+    my $autodetect  =   ( ! defined($icushared)  )
+                            &&
+                        ( ! defined($icuheaders) );
 
-            # location of header files
-            $icuheaders = capture_output("$icuconfig --prefix");
-            if ( defined $icuheaders ) {
-                chomp $icuheaders;
-                $without = 1 unless -d $icuheaders;
-                $icuheaders .= "${slash}include";
-                $without = 1 unless -d $icuheaders;
-            }
+    my $without = 0;
+    ($icuconfig, $autodetect, $without) =
+        $self->_handle_autodetect( {
+            icuconfig   => $icuconfig,
+            autodetect  => $autodetect,
+            without     => $without,
+            verbose     => $verbose,
+    } );
+    # Inside _handle_autodetect(), $without can be set to 1 by
+    # _handle_search_for_icu_config().  In that case, we're abandoning our
+    # attempt to configure with ICU and so may return here.
+    # 2nd possible return point
+    if ( $without ) {
+        $self->_set_no_configure_with_icu($conf, q{failed});
+        return 1;
+    }
 
-            if ($without) {
-                $self->set_result("failed");
+    ($without, $icushared, $icuheaders) =
+        $self->_try_icuconfig(
+            $conf,
+            {
+                conf            => $conf,
+                without         => $without,
+                autodetect      => $autodetect,
+                icuconfig       => $icuconfig,
             }
-        }
-    }
+        );
 
-    if ($verbose) {
-        print "icuconfig: $icuconfig\n"  if defined $icuconfig;
-        print "icushared='$icushared'\n" if defined $icushared;
-        print "headers='$icuheaders'\n"  if defined $icuheaders;
-    }
-
-    if ($without) {
-        $conf->data->set(
-            has_icu    => 0,
-            icu_shared => '',    # used for generating src/dynpmc/Makefile
-            icu_dir    => '',
-        );
-        $self->set_result("no") unless defined $self->result;
+    # 3rd possible return point
+    if ( $without ) {
+        $self->_set_no_configure_with_icu($conf, q{no});
         return 1;
     }
 
-    my $ok = 1;
+    _verbose_report($verbose, $icuconfig, $icushared, $icuheaders);
 
-    unless ( defined $icushared ) {
-        warn "error: icushared not defined\n";
-        $ok = 0;
-    }
+    $icuheaders = $self->_handle_icuconfig_errors( {
+        icushared   => $icushared,
+        icuheaders  => $icuheaders,
+    } );
+    # 4th possible return point.  This one is a step configuration failure
+    # because we would have really expected it to succeed.
+    return unless defined $icuheaders;
 
-    unless ( defined $icuheaders and -d $icuheaders ) {
-        warn "error: icuheaders not defined or invalid\n";
-        $ok = 0;
+    my $icudir = dirname($icuheaders);
+    $conf->data->set(
+        has_icu    => 1,
+        icu_shared => $icushared,
+        icu_dir    => $icudir,
+    );
+
+    # Add -I $Icuheaders if necessary.
+    my $header = "unicode/ucnv.h";
+    $conf->data->set( testheaders => "#include <$header>\n" );
+    $conf->data->set( testheader  => "$header" );
+    $conf->cc_gen('config/auto/headers/test_c.in');
+
+    # Clean up.
+    $conf->data->set( testheaders => undef );
+    $conf->data->set( testheader  => undef );
+    eval { $conf->cc_build(); };
+    my $ccflags_status = ( ! $@ && $conf->cc_run() =~ /^$header OK/ );
+    _handle_ccflags_status(
+        $conf,
+        {
+            ccflags_status  => $ccflags_status,
+            verbose         => $verbose,
+            icuheaders      => $icuheaders,
+        },
+    );
+    $conf->cc_clean();
+    $self->set_result("yes");
+    # 5th possible return point; this is the only really successful return.
+    return 1;
+}
+
+########## INTERNAL SUBROUTINES ##########
+
+sub _set_no_configure_with_icu {
+    my ($self, $conf, $result) = @_;
+    $conf->data->set(
+        has_icu    => 0,
+        icu_shared => '',    # used for generating src/dynpmc/Makefile
+        icu_dir    => '',
+    );
+    $self->set_result($result);
+}
+
+sub _handle_icuconfig_opt {
+    my ($self, $icuconfig_opt) = @_;
+    my $icuconfig;
+    if ( ( ! $icuconfig_opt ) or ( $icuconfig_opt eq q{none} ) ) {
+        $icuconfig = q{};
     }
+    elsif ( $icuconfig_opt eq '1' ) {
+        $icuconfig = $self->{icuconfig_default};
+    }
     else {
-        $icuheaders =~ s![\\/]$!!;
-        foreach my $header (@icu_headers) {
-            $header = "$icuheaders/unicode/$header";
-            unless ( -e $header ) {
-                $ok = 0;
-                warn "error: ICU header '$header' not found\n";
-            }
-        }
+        $icuconfig = $icuconfig_opt;
     }
+    return $icuconfig;
+}
 
-    die <<"HELP" unless $ok;
+sub _die_message {
+    my $die = <<"HELP";
 Something is wrong with your ICU installation!
 
    If you do not have a full ICU installation:
@@ -157,38 +213,177 @@
    --icuheaders=(path)  Location of ICU headers without /unicode
    --icushared=(flags)  Full linker command to create shared libraries
 HELP
+    return $die;
+}
 
-    my $icudir = dirname($icuheaders);
+sub _handle_search_for_icu_config {
+    my $self = shift;
+    my $arg = shift;
+    if (
+        ( $arg->{ret} == -1 )
+            ||
+        ( ( $arg->{ret} >> 8 ) != 0 )
+    ) {
+        undef $arg->{icuconfig};
+        $arg->{autodetect} = 0;
+        $arg->{without}    = 1;
+    }
+    else {
+        $arg->{icuconfig} = $self->{icuconfig_default};
+        if ($arg->{verbose}) {
+            print "icu-config found... good!\n";
+        }
+    }
+    return ( $arg->{icuconfig}, $arg->{autodetect}, $arg->{without} );
+}
 
-    $conf->data->set(
-        has_icu    => 1,
-        icu_shared => $icushared,
-        icu_dir    => $icudir,
-    );
+sub _handle_autodetect {
+    my $self = shift;
+    my $arg = shift;
+    if ( $arg->{autodetect} ) {
+        if ( ! $arg->{icuconfig} ) {
 
-    # Add -I $Icuheaders if necessary
-    my $header = "unicode/ucnv.h";
-    $conf->data->set( testheaders => "#include <$header>\n" );
-    $conf->data->set( testheader  => "$header" );
-    $conf->cc_gen('config/auto/headers/test_c.in');
+            my ( undef, undef, $ret ) =
+                capture_output( $self->{icuconfig_default}, "--exists" );
 
-    $conf->data->set( testheaders => undef );    # Clean up.
-    $conf->data->set( testheader  => undef );
-    eval { $conf->cc_build(); };
-    if ( !$@ && $conf->cc_run() =~ /^$header OK/ ) {
+            ($arg->{icuconfig}, $arg->{autodetect}, $arg->{without}) =
+                $self->_handle_search_for_icu_config( {
+                    icuconfig   => $arg->{icuconfig},
+                    autodetect  => $arg->{autodetect},
+                    without     => $arg->{without},
+                    verbose     => $arg->{verbose},
+                    ret         => $ret,
+            } );
+        }
+    } # end $autodetect true
+    else {
+        if ($arg->{verbose}) {
+            print "Specified an ICU config parameter,\n";
+            print "ICU autodetection disabled.\n";
+        }
+    } # end $autodetect false
+    return ( $arg->{icuconfig}, $arg->{autodetect}, $arg->{without} );
+}
 
-        # Ok, we don't need anything more.
-        print "Your compiler found the icu headers... good!\n" if $verbose;
+sub _try_icuconfig {
+    my $self = shift;
+    my $conf = shift;
+    my $arg = shift;
+    my ($icushared, $icuheaders);
+    if (
+        ( ! $arg->{without} )  &&
+        $arg->{autodetect}    &&
+        $arg->{icuconfig}
+    ) {
+        # ldflags
+        $icushared = capture_output("$arg->{icuconfig} --ldflags");
+        ($icushared, $arg->{without}) =
+            $self->_handle_icushared($icushared, $arg->{without});
+        # location of header files
+        $icuheaders = capture_output("$arg->{icuconfig} --prefix");
+        ($icuheaders, $arg->{without}) =
+            $self->_handle_icuheaders($conf, $icuheaders, $arg->{without});
+
+        # This branch is going to be very difficult to cover during testing
+        # because we would have to be able to manipulate the return values of
+        # either capture_output() call above.  We can't do this because we're
+        # autoconfiguring with the standard icu-config program.
+        if ($arg->{without}) {
+            $self->set_result("failed");
+        }
     }
+
+    return ($arg->{without}, $icushared, $icuheaders);
+}
+
+sub _handle_icushared {
+    my $self = shift;
+    my ($icushared, $without) = @_;
+    if ( defined $icushared ) {
+        chomp $icushared;
+        $icushared =~ s/$self->{icu_shared_pattern}//;    # "-licui18n32" too
+        if (length $icushared == 0) {
+            $without = 1;
+        }
+    }
+    return ($icushared, $without);
+}
+
+sub _handle_icuheaders {
+    my $self = shift;
+    my ($conf, $icuheaders, $without) = @_;
+    if ( defined $icuheaders ) {
+        chomp $icuheaders;
+        if (! -d $icuheaders) {
+            $without = 1;
+        }
+        my $slash = $conf->data->get('slash');
+        $icuheaders .= "${slash}include";
+        if (! -d $icuheaders) {
+            $without = 1;
+        }
+    }
+    return ($icuheaders, $without);
+}
+
+sub _verbose_report {
+    my ($verbose, $icuconfig, $icushared, $icuheaders) = @_;
+    if ($verbose) {
+        print "icuconfig: $icuconfig\n"  if defined $icuconfig;
+        print "icushared='$icushared'\n" if defined $icushared;
+        print "headers='$icuheaders'\n"  if defined $icuheaders;
+    }
+}
+
+sub _handle_icuconfig_errors {
+    my $self = shift;
+    my $arg = shift;
+    my $icuconfig_errors = 0;
+
+    if ( ! defined $arg->{icushared} ) {
+        warn "error: icushared not defined\n";
+        $icuconfig_errors++;
+    }
+
+    if ( ! ( defined $arg->{icuheaders} and -d $arg->{icuheaders} ) ) {
+        warn "error: icuheaders not defined or invalid\n";
+        $icuconfig_errors++;
+    }
     else {
-        print "Adding -I $icuheaders to ccflags for icu headers.\n" if 
$verbose;
-        $conf->data->add( ' ', ccflags => "-I $icuheaders" );
+        $arg->{icuheaders} =~ s![\\/]$!!;
+        foreach my $header ( @{ $self->{icu_headers} } ) {
+            $header = "$arg->{icuheaders}/unicode/$header";
+            if  ( ! -e $header ) {
+                $icuconfig_errors++;
+                warn "error: ICU header '$header' not found\n";
+            }
+        }
     }
-    $conf->cc_clean();
 
-    $self->set_result("yes");
+    if ($icuconfig_errors) {
+        warn _die_message();
+        return;
+    }
+    else {
+        return $arg->{icuheaders};
+    }
+}
 
-    return 1;
+sub _handle_ccflags_status {
+    my $conf = shift;
+    my $arg = shift;
+    if ($arg->{ccflags_status}) {
+        # Ok, we don't need anything more.
+        if ($arg->{verbose}) {
+            print "Your compiler found the icu headers... good!\n";
+        }
+    }
+    else {
+        if ($arg->{verbose}) {
+            print "Adding -I $arg->{icuheaders} to ccflags for icu headers.\n";
+        }
+        $conf->data->add( ' ', ccflags => "-I $arg->{icuheaders}" );
+    }
 }
 
 1;

Reply via email to