The patch attached refactors lib/Parrot/Revision.pm and all the files
that use it directly or indirectly.


On Tue Jan 15 09:31:18 2008, [EMAIL PROTECTED] wrote:
> Having looked a bit more at Parrot::Revision and the instances where it
> is used, I *think* we can make it go away completely, at least as a
> separate module, and incorporate its functionality within
> config/auto/revision.pm.
> 
> [li11-226:parrot] 508 $ fns . | xargs egrep -l 'Parrot(\/|::)Revision'
> ./tools/build/revision_c.pl
> ./lib/Parrot/Revision.pm

Both revised.

> ./t/configure/018-revision.t
> ./t/configure/017-revision_no_DEVELOPING.t

Renamed; new content.

> ./t/postconfigure/04-revision.t
> ./t/postconfigure/03-revision_no_DEVELOPING.t

Go bye-bye.

> ./t/distro/file_metadata.t

No longer uses Parrot::Revision.

> ./t/steps/auto_revision-01.t
> ./config/auto/revision.pm
> ./MANIFEST
> 

Refactored as needed.

Note:  Application of the patch attached entails a change in behavior. 
It will no longer be possible to say:

  svn update
  perl Configure.pl
  svn update (assuming a new revision number is displayed)
  make

... and be able to make 'successfully'.  tools/build/revision_c.pl will
now die should you try to do so.

I put 'successfully' in quotes because the current behavior is that if
you do an svn update and get updated files after configuration but
before build -- meaning your build no longer strictly corresponds to a
single state in the repository -- you can run 'make' to its conclusion
but when you call 'parrot --version' you will get warnings.  But to
respond to those warnings you have to do what you should have done in
the first place, namely, go directly from configuration to build.

Eliminating this toleration for bad programming practices simplified the
code considerably!

Please review.  I'll apply in 2-3 days if there is no objection.  Thank
you very much.

kid51

Index: tools/build/revision_c.pl
===================================================================
--- tools/build/revision_c.pl   (revision 24983)
+++ tools/build/revision_c.pl   (working copy)
@@ -22,46 +22,16 @@
 use warnings;
 use strict;
 use lib qw{lib . ../lib ../../ lib};
-use Parrot::Revision;
+use Parrot::Revision::Utils qw(
+    get_revision_numbers
+    print_src_revision_c
+);
 
-print <<"EOF";
-/* ex: set ro:
- * !!!!!!!   DO NOT EDIT THIS FILE   !!!!!!!
- *
- * This file is generated automatically by $0.
- *
- * Any changes made here will be lost!
- *
- */
+my ($current, $config) = get_revision_numbers();
+exit 1 unless ( $current == $config );
 
-/* HEADERIZER HFILE: none */
-/* HEADERIZER STOP */
+print_src_revision_c($current, $config, $0);
 
-#include "parrot/config.h"
-
-/* also in "parrot/embed.h" */
-PARROT_API int Parrot_revision(void);
-/* also in "parrot/misc.h" */
-PARROT_API int Parrot_config_revision(void);
-
-int Parrot_revision(void)
-{
-    return ${Parrot::Revision::current};
-}
-
-int Parrot_config_revision(void)
-{
-    return ${Parrot::Revision::config};
-}
-
-/*
- * Local variables:
- *   c-file-style: "parrot"
- * End:
- * vim: expandtab shiftwidth=4:
- */
-EOF
-
 # Local Variables:
 #   mode: cperl
 #   cperl-indent-level: 4
Index: MANIFEST
===================================================================
--- MANIFEST    (revision 24983)
+++ MANIFEST    (working copy)
@@ -1,7 +1,7 @@
 # ex: set ro:
 # $Id$
 #
-# generated by C:\usr\local\parrot\trunk\tools\dev\mk_manifest_and_skip.pl Fri 
Jan 18 22:14:41 2008 UT
+# generated by tools/dev/mk_manifest_and_skip.pl Sat Jan 19 03:20:22 2008 UT
 #
 # See tools/dev/install_files.pl for documentation on the
 # format of this file.
@@ -2560,6 +2560,7 @@
 lib/Parrot/Pmc2c/UtilFunctions.pm                           [devel]
 lib/Parrot/Pmc2c/VTable.pm                                  [devel]
 lib/Parrot/Revision.pm                                      [devel]
+lib/Parrot/Revision/Utils.pm                                [devel]
 lib/Parrot/Test.pm                                          [devel]
 lib/Parrot/Test/APL.pm                                      [devel]
 lib/Parrot/Test/Cardinal.pm                                 [devel]
@@ -3079,8 +3080,8 @@
 t/configure/013-die.t                                       []
 t/configure/015-no_return.t                                 []
 t/configure/016-no_return_but_result.t                      []
-t/configure/017-revision_no_DEVELOPING.t                    []
-t/configure/018-revision.t                                  []
+t/configure/017-revision_from_cache.t                       []
+t/configure/018-revision_to_cache.t                         []
 t/configure/019-version.t                                   []
 t/configure/020-version.t                                   []
 t/configure/021-version.t                                   []
@@ -3365,8 +3366,6 @@
 t/pmc/vtablecache.t                                         []
 t/postconfigure/01-options.t                                []
 t/postconfigure/02-data_slurp.t                             []
-t/postconfigure/03-revision_no_DEVELOPING.t                 []
-t/postconfigure/04-revision.t                               []
 t/postconfigure/05-trace.t                                  []
 t/postconfigure/06-data_slurp_temp.t                        []
 t/run/README                                                []
@@ -3625,6 +3624,8 @@
 t/tools/pmc2cutils/07-open_file.t                           []
 t/tools/pmc2cutils/08-pmc-pm.t                              []
 t/tools/pmc2cutils/README                                   []
+t/tools/revision/01-get_revision_numbers.t                  []
+t/tools/revision/02-print_src.t                             []
 t/tools/smartlinks.t                                        []
 tools/build/addopstags.pl                                   []
 tools/build/c2str.pl                                        []
Index: lib/Parrot/Configure/Options/Test.pm
===================================================================
--- lib/Parrot/Configure/Options/Test.pm        (revision 24983)
+++ lib/Parrot/Configure/Options/Test.pm        (working copy)
@@ -51,6 +51,7 @@
     glob("t/tools/pmc2cutils/*.t"),
     glob("t/tools/ops2cutils/*.t"),
     glob("t/tools/ops2pmutils/*.t"),
+    glob("t/tools/revision/*.t"),
 );
 
 sub new {
Index: lib/Parrot/Revision.pm
===================================================================
--- lib/Parrot/Revision.pm      (revision 24983)
+++ lib/Parrot/Revision.pm      (working copy)
@@ -10,7 +10,6 @@
     use Parrot::Revision;
 
     print $Parrot::Revision::current;
-    print $Parrot::Revision::config;
 
 =head1 DESCRIPTION
 
@@ -24,11 +23,35 @@
 use warnings;
 use File::Spec;
 
-sub __get_revision {
-    return 0 unless ( -e 'DEVELOPING' );
+our $cache = q{.parrot_current_rev};
 
+our $current = _get_revision();
+
+sub _get_revision {
     my $revision;
+    if (-f $cache) {
+        eval {
+            open my $FH, "<", $cache;
+            chomp($revision = <$FH>);
+            close $FH;
+        };
+        return $revision unless $@;
+    }
 
+    $revision = _analyze_sandbox();
+
+    unless (-f $cache) {
+        eval {
+            open my $FH, ">", $cache;
+            print $FH "$revision\n";
+            close $FH;
+        };
+    }
+    return $revision;
+}
+
+sub _analyze_sandbox {
+    my $revision = 0;
     # code taken from pugs/util/version_h.pl rev 14410
     my $nul = File::Spec->devnull;
     if ( my @svn_info = qx/svn --xml info 2>$nul/ and $? == 0 ) {
@@ -58,15 +81,9 @@
             }
         }
     }
-    return ( $revision || 0 );
+    return $revision;
 }
 
-our $current = __get_revision();
-our $config  = $current;
-
-# check if Parrot::Config is available
-eval 'use Parrot::Config; $config = $PConfig{revision};';
-
 1;
 
 # Local Variables:
Index: t/configure/018-revision.t
===================================================================
--- t/configure/018-revision.t  (revision 24983)
+++ t/configure/018-revision.t  (working copy)
@@ -1,65 +0,0 @@
-#! perl
-# Copyright (C) 2007, The Perl Foundation.
-# $Id$
-# 018-revision.t
-
-use strict;
-use warnings;
-
-use Test::More tests => 7;
-use Carp;
-use_ok('Cwd');
-use_ok('File::Copy');
-use_ok( 'File::Temp', qw| tempdir | );
-use lib qw( lib );
-use Parrot::Revision;
-
-my ( $current, $config );
-
-# Case 1:  DEVELOPING exists; Parrot::Config not yet available.
-my $reason =
-'Either file DEVELOPING does not exist, or configuration has completed 
(because Parrot::Config::Generated exists).';
-SKIP: {
-    skip $reason, 3 if ( ( not -e 'DEVELOPING' )
-        or ( -e q{lib/Parrot/Config/Generated.pm} ) );
-    $current = $Parrot::Revision::current;
-    like( $current, qr/^\d+$/, "current revision is all numeric" );
-    $config = $Parrot::Revision::config;
-    like( $config, qr/^\d+$/, "current revision is all numeric" );
-    is( $current, $config, "current and config are identical" );
-}    # end SKIP block
-
-pass("Completed all tests in $0");
-
-################### DOCUMENTATION ###################
-
-=head1 NAME
-
-018-revision.t - test Parrot::Revision
-
-=head1 SYNOPSIS
-
-    % prove t/configure/018-revision.t
-
-=head1 DESCRIPTION
-
-The files in this directory test functionality used by F<Configure.pl>.
-
-The tests in this file test Parrot::Revision (F<lib/Parrot/Revision.pm>).
-
-=head1 AUTHOR
-
-James E Keenan
-
-=head1 SEE ALSO
-
-Parrot::Configure, F<Configure.pl>.
-
-=cut
-
-# Local Variables:
-#   mode: cperl
-#   cperl-indent-level: 4
-#   fill-column: 100
-# End:
-# vim: expandtab shiftwidth=4:
Index: t/configure/017-revision_no_DEVELOPING.t
===================================================================
--- t/configure/017-revision_no_DEVELOPING.t    (revision 24983)
+++ t/configure/017-revision_no_DEVELOPING.t    (working copy)
@@ -1,81 +0,0 @@
-#! perl
-# Copyright (C) 2007, The Perl Foundation.
-# $Id$
-# 017-revision_no_DEVELOPING.t
-
-use strict;
-use warnings;
-
-use Test::More tests => 13;
-use Carp;
-use_ok('Cwd');
-use_ok('File::Copy');
-use_ok( 'File::Temp', qw| tempdir | );
-use lib qw( lib );
-
-my ( $current, $config );
-
-# Case 2:  DEVELOPING's non-existence is faked;  Parrot::Config not yet 
available. #'
-my $cwd = cwd();
-my $reason =
-'Either file DEVELOPING does not exist or configuration has completed (as 
evidenced by existence of Parrot::Config::Generated';
-
-SKIP: {
-    skip $reason, 9 if ( ( not -e 'DEVELOPING' )
-        or ( -e q{lib/Parrot/Config/Generated.pm} ) );
-    my $tdir = tempdir( CLEANUP => 1 );
-    ok( chdir $tdir, "Changed to temporary directory for testing" );
-    ok( ( mkdir "lib" ),        "Able to make directory lib" );
-    ok( ( mkdir "lib/Parrot" ), "Able to make directory lib/Parrot" );
-    ok(
-        copy( "$cwd/lib/Parrot/Revision.pm", "lib/Parrot/" ),
-        "Able to copy Parrot::Revision for testing"
-    );
-    unshift( @INC, "lib" );
-    require Parrot::Revision;
-    no warnings qw(once);
-    $current = $Parrot::Revision::current;
-    like( $current, qr/^\d+$/, "current revision is all numeric" );
-    $config = $Parrot::Revision::config;
-    use warnings;
-    like( $config, qr/^\d+$/, "current revision is all numeric" );
-    is( $current, $config, "current and config are identical" );
-    is( $current, 0, 'current is zero as expected' );
-    ok( chdir $cwd, "Able to change back to directory after testing" );
-}
-
-# Case 3:  DEVELOPING exists; Parrot::Config available.
-pass("Completed all tests in $0");
-
-################### DOCUMENTATION ###################
-
-=head1 NAME
-
-017-revision_no_DEVELOPING.t - test Parrot::Revision
-
-=head1 SYNOPSIS
-
-    % prove t/configure/017-revision_no_DEVELOPING.t
-
-=head1 DESCRIPTION
-
-The files in this directory test functionality used by F<Configure.pl>.
-
-The tests in this file test Parrot::Revision (F<lib/Parrot/Revision.pm>).
-
-=head1 AUTHOR
-
-James E Keenan
-
-=head1 SEE ALSO
-
-Parrot::Configure, F<Configure.pl>.
-
-=cut
-
-# Local Variables:
-#   mode: cperl
-#   cperl-indent-level: 4
-#   fill-column: 100
-# End:
-# vim: expandtab shiftwidth=4:
Index: t/postconfigure/04-revision.t
===================================================================
--- t/postconfigure/04-revision.t       (revision 24983)
+++ t/postconfigure/04-revision.t       (working copy)
@@ -1,70 +0,0 @@
-#! perl
-# Copyright (C) 2007, The Perl Foundation.
-# $Id$
-# 04-revision.t
-
-use strict;
-use warnings;
-
-use Test::More tests => 6;
-use Carp;
-use_ok('Cwd');
-use_ok('File::Copy');
-use_ok( 'File::Temp', qw| tempdir | );
-use lib qw( lib );
-use Parrot::Revision;
-
-my ( $current, $config );
-
-# Case 1:  DEVELOPING exists; Parrot::Config available.
-my $reason =
-'Either file DEVELOPING does not exist or configuration has not completed (as 
evidenced by non-existence of Parrot::Config::Generated';
-SKIP: {
-    skip $reason, 2 if ( ( not -e 'DEVELOPING' )
-        or ( not -e q{lib/Parrot/Config/Generated.pm} ) );
-    $current = $Parrot::Revision::current;
-    like( $current, qr/^\d+$/, "current revision is all numeric" );
-    $config = $Parrot::Revision::config;
-    like( $config, qr/^\d+$/, "current revision is all numeric" );
-}    # end SKIP block
-
-pass("Completed all tests in $0");
-
-################### DOCUMENTATION ###################
-
-=head1 NAME
-
-04-revision.t - test Parrot::Revision
-
-=head1 SYNOPSIS
-
-    % prove t/postconfigure/04-revision.t
-
-=head1 DESCRIPTION
-
-The files in this directory test functionality used by F<Configure.pl>.
-Certain of the modules C<use>d by F<Configure.pl> have functionality which is
-only meaningful I<after> F<Configure.pl> has actually been run and
-Parrot::Config::Generated has been created.  So certain tests need to be run
-when your Parrot filesystem is in a "pre-F<make>, post-F<Configure.pl>" state.
-
-The tests in this file test aspects of Parrot::Revision
-(F<lib/Parrot/Revision.pm>) which presume that configuration has been
-completed.
-
-=head1 AUTHOR
-
-James E Keenan
-
-=head1 SEE ALSO
-
-Parrot::Configure, F<Configure.pl>.
-
-=cut
-
-# Local Variables:
-#   mode: cperl
-#   cperl-indent-level: 4
-#   fill-column: 100
-# End:
-# vim: expandtab shiftwidth=4:
Index: t/postconfigure/03-revision_no_DEVELOPING.t
===================================================================
--- t/postconfigure/03-revision_no_DEVELOPING.t (revision 24983)
+++ t/postconfigure/03-revision_no_DEVELOPING.t (working copy)
@@ -1,90 +0,0 @@
-#! perl
-# Copyright (C) 2007, The Perl Foundation.
-# $Id$
-# 03-revision_no_DEVELOPING.t
-
-use strict;
-use warnings;
-
-use Test::More tests => 15;
-use Carp;
-use_ok('Cwd');
-use_ok('File::Copy');
-use_ok( 'File::Temp', qw| tempdir | );
-use lib qw( lib );
-
-my ( $current, $config );
-
-# Case 2:  DEVELOPING's non-existence is faked;  Parrot::Config available. #'
-my $cwd = cwd();
-my $reason =
-'Either file DEVELOPING does not exist or configuration has not completed (as 
evidenced by non-existence of Parrot::Config::Generated';
-
-SKIP: {
-    skip $reason, 11 if ( ( not -e 'DEVELOPING' )
-        or ( not -e q{lib/Parrot/Config/Generated.pm} ) );
-    my $tdir = tempdir();
-    ok( chdir $tdir, "Changed to temporary directory for testing" );
-    ok( ( mkdir "lib" ),               "Able to make directory lib" );
-    ok( ( mkdir "lib/Parrot" ),        "Able to make directory lib/Parrot" );
-    ok( ( mkdir "lib/Parrot/Config" ), "Able to make directory 
lib/Parrot/Config" );
-    ok(
-        copy( "$cwd/lib/Parrot/Revision.pm", "lib/Parrot/" ),
-        "Able to copy Parrot::Revision for testing"
-    );
-    ok(
-        copy( "$cwd/lib/Parrot/Config.pm", "lib/Parrot/" ),
-        "Able to copy Parrot::Config for testing"
-    );
-    ok( copy( "$cwd/lib/Parrot/Config/Generated.pm", "lib/Parrot/Config/" ),
-        "Able to copy Parrot::Config::Generated for testing" );
-    unshift( @INC, "lib" );
-    require Parrot::Revision;
-    no warnings qw(once);
-    $current = $Parrot::Revision::current;
-    like( $current, qr/^\d+$/, "current revision is all numeric" );
-    is( $current, 0, "If DEVELOPING does not exist (release version), 
\$current is set to zero." );
-    $config = $Parrot::Revision::config;
-    use warnings;
-    like( $config, qr/^\d+$/, "current revision is all numeric" );
-    ok( chdir $cwd, "Able to change back to directory after testing" );
-}
-
-pass("Completed all tests in $0");
-
-################### DOCUMENTATION ###################
-
-=head1 NAME
-
-03-revision_no_DEVELOPING.t - test Parrot::Revision
-
-=head1 SYNOPSIS
-
-    % prove t/postconfigure/03-revision_no_DEVELOPING.t
-
-=head1 DESCRIPTION
-
-The files in this directory test functionality used by F<Configure.pl>.
-Certain of the modules C<use>d by F<Configure.pl> have functionality which is
-only meaningful I<after> F<Configure.pl> has actually been run and
-Parrot::Config::Generated has been created.  So certain tests need to be run
-when your Parrot filesystem is in a "pre-F<make>, post-F<Configure.pl>" state.
-
-The tests in this file test Parrot::Revision (F<lib/Parrot/Revision.pm>).
-
-=head1 AUTHOR
-
-James E Keenan
-
-=head1 SEE ALSO
-
-Parrot::Configure, F<Configure.pl>.
-
-=cut
-
-# Local Variables:
-#   mode: cperl
-#   cperl-indent-level: 4
-#   fill-column: 100
-# End:
-# vim: expandtab shiftwidth=4:
Index: t/steps/auto_fink-09.t
===================================================================
--- t/steps/auto_fink-09.t      (revision 24983)
+++ t/steps/auto_fink-09.t      (working copy)
@@ -8,7 +8,6 @@
 use Test::More;
 plan( skip_all => 'Fink is Darwin only' ) unless $^O =~ /darwin/;
 plan( tests => 13 );
-# plan( 'no_plan' );
 use Carp;
 use File::Temp;
 use lib qw( lib t/configure/testlib );
Index: t/distro/file_metadata.t
===================================================================
--- t/distro/file_metadata.t    (revision 24983)
+++ t/distro/file_metadata.t    (working copy)
@@ -241,7 +241,7 @@
             plan skip_all => q{git svn file metadata not retained};
         }
     }
-    elsif ( !( $Parrot::Revision::current or `svk ls .` ) ) {
+    elsif ( !( `svn ls .` or `svk ls .` ) ) {
         plan skip_all => 'not a working copy';
     }
     else { plan tests => 4 }
Index: config/gen/makefiles/root.in
===================================================================
--- config/gen/makefiles/root.in        (revision 24983)
+++ config/gen/makefiles/root.in        (working copy)
@@ -328,7 +328,8 @@
 FLUID_FILES_2 = \
     $(GEN_LIBRARY) \
     runtime/parrot/include/parrotlib.pbc \
-    .configure_trace.sto
+    .configure_trace.sto \
+       .parrot_current_rev
 
 
 ###############################################################################
@@ -1395,36 +1396,12 @@
 PMC2CUTILS_DIR = t/tools/pmc2cutils
 OPS2PMUTILS_DIR = t/tools/ops2pmutils
 OPS2CUTILS_DIR = t/tools/ops2cutils
+REVISIONUTILS_DIR = t/tools/revision
 BUILDTOOLS_TEST_FILES = \
-        $(PMC2CUTILS_DIR)/00-qualify.t \
-        $(PMC2CUTILS_DIR)/01-pmc2cutils.t \
-        $(PMC2CUTILS_DIR)/02-find_file.t \
-        $(PMC2CUTILS_DIR)/03-dump_vtable.t \
-        $(PMC2CUTILS_DIR)/04-dump_pmc.t \
-        $(PMC2CUTILS_DIR)/05-gen_c.t \
-        $(PMC2CUTILS_DIR)/06-print_tree.t \
-        $(PMC2CUTILS_DIR)/07-open_file.t \
-        $(OPS2PMUTILS_DIR)/00-qualify.t \
-        $(OPS2PMUTILS_DIR)/01-ops2pmutils.t \
-        $(OPS2PMUTILS_DIR)/02-usage.t \
-        $(OPS2PMUTILS_DIR)/03-new.t \
-        $(OPS2PMUTILS_DIR)/04-prepare_ops.t \
-        $(OPS2PMUTILS_DIR)/05-renum_op_map_file.t \
-        $(OPS2PMUTILS_DIR)/06-load_op_map_files.t \
-        $(OPS2PMUTILS_DIR)/07-no_ops_skip.t \
-        $(OPS2PMUTILS_DIR)/08-sort_ops.t \
-        $(OPS2PMUTILS_DIR)/09-prepare_real_ops.t \
-        $(OPS2PMUTILS_DIR)/10-print_module.t \
-        $(OPS2PMUTILS_DIR)/11-print_h.t \
-        $(OPS2CUTILS_DIR)/01-new.t \
-        $(OPS2CUTILS_DIR)/02-usage.t \
-        $(OPS2CUTILS_DIR)/03-print_c_header_file.t \
-        $(OPS2CUTILS_DIR)/04-print_c_source_top.t \
-        $(OPS2CUTILS_DIR)/05-print_c_source_bottom.t \
-        $(OPS2CUTILS_DIR)/06-dynamic.t \
-        $(OPS2CUTILS_DIR)/07-make_incdir.t \
-        $(OPS2CUTILS_DIR)/08-nolines.t \
-        $(OPS2CUTILS_DIR)/09-dynamic_nolines.t
+        $(PMC2CUTILS_DIR)/*.t \
+        $(OPS2PMUTILS_DIR)/*.t \
+        $(OPS2CUTILS_DIR)/*.t \
+        $(REVISIONUTILS_DIR)/*.t
 MANIFEST_DIR = t/manifest
 MANIFEST_TEST_FILES = \
         $(MANIFEST_DIR)/01-basic.t \

Reply via email to