Will Coleda wrote:
# New Ticket Created by Will Coleda
# Please include the string: [perl #43923]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=43923 >
The recently added functionality to not regen MANIFEST.SKIP should be
used when determining whether or not to regenerate MANIFEST as well.
--
Will "Coke" Coleda
[EMAIL PROTECTED]
[I posted this to RT > 24 hours ago, but it hasn't made it to the
newsgroup interface yet. I wonder if something is amiss there.]
Please review the attached. Modifications to
tools/dev/mk_manifest_and_skip.pl and lib/Parrot/Manifest.pm. Tso
additional test files in t/manifest/.
In reviewing the patch, I recommend that you first run
tools/dev/mk_manifest_and skip.pl before running t/manifest/*.t. In
that way, you will be starting from a presumptively correct MANIFEST.
The interface to Parrot::Manifest has changed since last weekend.
However, that will not matter assuming that all you ever do is call:
perl tools/dev/mk_manifest_and_skip.pl. *That* is unchanged.
Thank you very much.
kid51
Index: tools/dev/mk_manifest_and_skip.pl
===================================================================
--- tools/dev/mk_manifest_and_skip.pl (revision 20000)
+++ tools/dev/mk_manifest_and_skip.pl (working copy)
@@ -7,14 +7,20 @@
my $script = $0;
-my $mani = Parrot::Manifest->new($script);
+my $mani = Parrot::Manifest->new( {
+ script => $script,
+} );
my $manifest_lines_ref = $mani->prepare_manifest();
-$mani->print_manifest($manifest_lines_ref);
+my $need_for_files = $mani->determine_need_for_manifest($manifest_lines_ref);
+$mani->print_manifest($manifest_lines_ref) if $need_for_files;
-my $ignore_ref = $mani->prepare_manifest_skip();
-$mani->print_manifest_skip($ignore_ref);
+my $print_str = $mani->prepare_manifest_skip();
+my $need_for_skip = $mani->determine_need_for_manifest_skip($print_str);
+$mani->print_manifest_skip($print_str) if $need_for_skip;
+#################### DOCUMENTATION ####################
+
=head1 NAME
tools/dev/mk_manifest_and_skip.pl - Recreate MANIFEST and MANIFEST.SKIP
Index: MANIFEST
===================================================================
--- MANIFEST (revision 20000)
+++ MANIFEST (working copy)
@@ -1,7 +1,7 @@
# ex: set ro:
# $Id$
#
-# generated by tools/dev/mk_manifest_and_skip.pl Wed Jul 18 18:17:51 2007 UT
+# generated by tools/dev/mk_manifest_and_skip.pl Thu Jul 19 02:43:53 2007 UT
#
# See tools/dev/install_files.pl for documentation on the
# format of this file.
@@ -2931,6 +2931,8 @@
t/library/test_more.t []
t/library/yaml_parser_syck.t []
t/manifest/01-basic.t []
+t/manifest/02-regenerate_file.t []
+t/manifest/03-regenerate_skip.t []
t/manifest/README []
t/native_pbc/header.t []
t/native_pbc/integer.t []
Index: lib/Parrot/Manifest.pm
===================================================================
--- lib/Parrot/Manifest.pm (revision 20000)
+++ lib/Parrot/Manifest.pm (working copy)
@@ -3,26 +3,29 @@
use strict;
use warnings;
use Carp;
+use Data::Dumper;
sub new {
my $class = shift;
- my $script = shift;
+ my $argsref = shift;
my $self = bless( {}, $class );
my %data = (
- id => '$' . 'Id$',
- time => scalar gmtime,
- cmd => -d '.svn' ? 'svn' : 'svk',
- script => $script,
+ id => '$' . 'Id$',
+ time => scalar gmtime,
+ cmd => -d '.svn' ? 'svn' : 'svk',
+ script => $argsref->{script},
+ file => $argsref->{file} ? $argsref->{file} : q{MANIFEST},
+ skip => $argsref->{skip} ? $argsref->{skip} : q{MANIFEST.SKIP},
);
- my @status_output = qx($data{cmd} status -v);
+ my $status_output_ref = [ qx($data{cmd} status -v) ];
# grab the versioned resources:
my @versioned_files = ();
my @dirs = ();
- my @versioned_output = grep !/^[?D]/, @status_output;
+ my @versioned_output = grep !/^[?D]/, @{ $status_output_ref };
for my $line (@versioned_output) {
my @line_info = split( /\s+/, $line );
@@ -47,20 +50,36 @@
sub prepare_manifest {
my $self = shift;
- my @manifest_lines;
+ my %manifest_lines;
for my $file (@{ $self->{versioned_files} }) {
- push @manifest_lines, _get_manifest_entry($file);
+ $manifest_lines{$file} = _get_manifest_entry($file);
}
- return [EMAIL PROTECTED];
+ return \%manifest_lines;
}
+sub determine_need_for_manifest {
+ my $self = shift;
+ my $proposed_files_ref = shift;
+ if ( ! -f $self->{file} ) {
+ return 1;
+ } else {
+ my $current_files_ref = $self->_get_current_files();
+ my $different_patterns_count = 0;
+ foreach my $cur (keys %{ $current_files_ref }) {
+ $different_patterns_count++ unless $proposed_files_ref->{$cur};
+ }
+ foreach my $pro (keys %{ $proposed_files_ref }) {
+ $different_patterns_count++ unless $current_files_ref->{$pro};
+ }
+ $different_patterns_count ? return 1 : return;
+ }
+}
+
sub print_manifest {
my $self = shift;
my $manifest_lines_ref = shift;
- open my $MANIFEST, '>', 'MANIFEST'
- or croak "Unable to open MANIFEST for writing";
- print $MANIFEST <<"END_HEADER";
+ my $print_str = <<"END_HEADER";
# ex: set ro:
# $self->{id}
#
@@ -72,8 +91,14 @@
# has been told about new or deleted files.
END_HEADER
- print $MANIFEST $_ for ( sort @{ $manifest_lines_ref } );
- close $MANIFEST or croak "Unable to close MANIFEST after writing";
+ for my $k ( sort keys %{ $manifest_lines_ref } ) {
+ $print_str .= sprintf "%- 59s %s\n", ($k, $manifest_lines_ref->{$k});
+ }
+ open my $MANIFEST, '>', $self->{file}
+ or croak "Unable to open $self->{file} for writing";
+ print $MANIFEST $print_str;
+ close $MANIFEST or croak "Unable to close $self->{file} after writing";
+ return 1;
}
sub _get_manifest_entry {
@@ -99,7 +124,7 @@
: m[^(apps/\w+)/] ? "[$1]"
: '[]';
}
- return sprintf( "%- 59s %s\n", $file, $loc );
+ return $loc;
}
sub _get_special {
@@ -147,6 +172,22 @@
return \%special;
}
+sub _get_current_files {
+ my $self = shift;
+ my %current_files = ();
+ open my $FILE, "<", $self->{file}
+ or die "Unable to open $self->{file} for reading";
+ while (my $line = <$FILE>) {
+ chomp $line;
+ next if $line =~ /^\s*$/o;
+ next if $line =~ /^#/o;
+ my @els = split /\s+/, $line;
+ $current_files{$els[0]}++;
+ }
+ close $FILE or die "Unable to close $self->{file} after reading";
+ return \%current_files;
+}
+
sub prepare_manifest_skip {
my $self = shift;
my $svnignore = `$self->{cmd} propget svn:ignore @{ $self->{dirs} }`;
@@ -168,11 +209,41 @@
$ignore{$1} = $2 if $2;
}
}
- return \%ignore;
+ return $self->_compose_print_str( \%ignore );
}
+sub determine_need_for_manifest_skip {
+ my $self = shift;
+ my $print_str = shift;
+ if ( ! -f $self->{skip} ) {
+ return 1;
+ } else {
+ my $current_skips_ref = $self->_get_current_skips();
+ my $proposed_skips_ref = _get_proposed_skips($print_str);
+ my $different_patterns_count = 0;
+ foreach my $cur (keys %{ $current_skips_ref }) {
+ $different_patterns_count++ unless $proposed_skips_ref->{$cur};
+ }
+ foreach my $pro (keys %{ $proposed_skips_ref }) {
+ $different_patterns_count++ unless $current_skips_ref->{$pro};
+ }
+ $different_patterns_count ? return 1 : return;
+ }
+}
+
sub print_manifest_skip {
my $self = shift;
+ my $print_str = shift;
+ open my $MANIFEST_SKIP, '>', $self->{skip}
+ or die "Unable to open $self->{skip} for writing";
+ print $MANIFEST_SKIP $print_str;
+ close $MANIFEST_SKIP
+ or die "Unable to close $self->{skip} after writing";
+ return 1;
+}
+
+sub _compose_print_str {
+ my $self = shift;
my $ignore_ref = shift;
my %ignore = %{ $ignore_ref };
my $print_str = <<"END_HEADER";
@@ -206,37 +277,21 @@
: "^$_\$\n^$_/\n";
}
}
- my $current_skips_ref = _get_current_skips();
- my $proposed_skips_ref = _get_proposed_skips($print_str);
- my $different_patterns_count = 0;
- foreach my $cur (keys %{ $current_skips_ref }) {
- $different_patterns_count++ unless $proposed_skips_ref->{$cur};
- }
- foreach my $pro (keys %{ $proposed_skips_ref }) {
- $different_patterns_count++ unless $current_skips_ref->{$pro};
- }
- if ( $different_patterns_count or (! -f 'MANIFEST.SKIP') ) {
- open my $MANIFEST_SKIP, '>', 'MANIFEST.SKIP'
- or die "Unable to open MANIFEST.SKIP for writing";
- print $MANIFEST_SKIP $print_str;
- close $MANIFEST_SKIP
- or die "Unable to close MANIFEST.SKIP after writing";
- }
- return 1;
+ return $print_str;
}
sub _get_current_skips {
- my $sk = q{MANIFEST.SKIP};
- return {} unless -f $sk;
+ my $self = shift;
my %current_skips = ();
- open my $SKIP, "<", $sk or die "Unable to open $sk for reading";
+ open my $SKIP, "<", $self->{skip}
+ or die "Unable to open $self->{skip} for reading";
while (my $line = <$SKIP>) {
chomp $line;
next if $line =~ /^\s*$/o;
next if $line =~ /^#/o;
$current_skips{$line}++;
}
- close $SKIP or die "Unable to close $sk after reading";
+ close $SKIP or die "Unable to close $self->{skip} after reading";
return \%current_skips;
}
@@ -267,10 +322,12 @@
$mani = Parrot::Manifest->new($0);
$manifest_lines_ref = $mani->prepare_manifest();
- $mani->print_manifest($manifest_lines_ref);
+ $need_for_files = $mani->determine_need_for_manifest($manifest_lines_ref);
+ $mani->print_manifest($manifest_lines_ref) if $need_for_files;
- $ignore_ref = $mani->prepare_manifest_skip();
- $mani->print_manifest_skip($ignore_ref);
+ $print_str = $mani->prepare_manifest_skip();
+ $need_for_skip = $mani->determine_need_for_manifest_skip($print_str);
+ $mani->print_manifest_skip($print_str) if $need_for_skip;
=head1 SEE ALSO
Index: t/manifest/01-basic.t
===================================================================
--- t/manifest/01-basic.t (revision 20000)
+++ t/manifest/01-basic.t (working copy)
@@ -6,7 +6,7 @@
use strict;
use warnings;
-use Test::More tests => 11;
+use Test::More tests => 13;
use Carp;
use Cwd;
use File::Temp qw( tempdir );
@@ -14,43 +14,51 @@
use_ok('Parrot::Manifest');
my $script = $0;
-my $mani = Parrot::Manifest->new($script);
+my $file = q{MANIFEST};
+my $skip = q{MANIFEST.SKIP};
+
+my $mani = Parrot::Manifest->new( {
+ script => $script,
+} );
isa_ok($mani, 'Parrot::Manifest');
-ok(scalar(@{ $mani->{dirs} } ),
+ok(scalar( @{ $mani->{dirs} } ),
"Parrot::Manifest constructor used 'status' command to find at least 1
directory.");
-ok(scalar(@{ $mani->{versioned_files} } ),
+ok(scalar( @{ $mani->{versioned_files} } ),
"Parrot::Manifest constructor used 'status' command to find at least 1
versioned file.");
my $manifest_lines_ref = $mani->prepare_manifest();
-is(ref($manifest_lines_ref), q{ARRAY},
- "prepare_manifest() returned array ref");
+is(ref($manifest_lines_ref), q{HASH},
+ "prepare_manifest() returned hash ref");
+
my $cwd = cwd();
{
my $tdir = tempdir( CLEANUP => 1 );
chdir $tdir or
croak "Unable to change to temporary directory for testing";
- ok(! -f 'MANIFEST',
- "No MANIFEST yet in this directory");
+ ok(! -f $file, "No $file yet in this directory");
$mani->print_manifest($manifest_lines_ref);
- ok( -f 'MANIFEST',
- "MANIFEST has been created in this directory");
+ ok( -f $file, "$file has been created in this directory");
chdir $cwd or
croak "Unable to change back from temporary directory after testing";
}
-my $ignore_ref = $mani->prepare_manifest_skip();
-is(ref($ignore_ref), q{HASH},
- "prepare_manifest() returned hash ref");
+my $print_str = $mani->prepare_manifest_skip();
+ok($print_str, "prepare_manifest_skip() returned");
+
{
my $tdir = tempdir( CLEANUP => 1 );
chdir $tdir or
croak "Unable to change to temporary directory for testing";
- ok(! -f 'MANIFEST.SKIP',
- "No MANIFEST.SKIP yet in this directory");
- $mani->print_manifest_skip($ignore_ref);
- ok( -f 'MANIFEST.SKIP',
- "MANIFEST.SKIP has been created in this directory");
+ ok(! -f $skip,
+ "No $skip yet in tempdir");
+ my $need_for_skip = $mani->determine_need_for_manifest_skip($print_str);
+ ok($need_for_skip,
+ "No $skip in tempdir; it must be regenerated");
+ ok( $mani->print_manifest_skip($print_str),
+ "print_manifest_skip() returned true");
+ ok( -f $skip,
+ "$skip has been created in tempdir");
chdir $cwd or
croak "Unable to change back from temporary directory after testing";
}
Index: t/manifest/02-regenerate_file.t
===================================================================
--- t/manifest/02-regenerate_file.t (revision 0)
+++ t/manifest/02-regenerate_file.t (revision 0)
@@ -0,0 +1,116 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# 02-regenerate_file.t
+
+use strict;
+use warnings;
+
+use Test::More tests => 10;
+use Carp;
+use Cwd;
+use File::Copy;
+use File::Temp qw( tempdir );
+use Tie::File;
+use lib ( qw| lib | );
+use_ok('Parrot::Manifest');
+
+my $script = $0;
+my $mani = Parrot::Manifest->new( {
+ script => $script,
+} );
+isa_ok($mani, 'Parrot::Manifest');
+
+my $cwd = cwd();
+my $f = q{MANIFEST};
+
+my $manifest_lines_ref = $mani->prepare_manifest();
+ok($manifest_lines_ref, "prepare_manifest_skip() returned");
+
+# 1: Copy the real MANIFEST unaltered to the tempdir.
+# Assuming the real MANIFEST was correct going in to this test, the
+# absence of any change in it will mean that there will be no need to
+# regenerate it.
+{
+ my $tdir = tempdir( CLEANUP => 1 );
+ chdir $tdir or
+ croak "Unable to change to temporary directory for testing";
+ copy(qq{$cwd/$f}, qq{$tdir/$f})
+ or croak "Unable to copy $f to tempdir";
+ ok(-f $f, "$f found in tempdir");
+ my $need_for_file =
+ $mani->determine_need_for_manifest($manifest_lines_ref);
+ ok(! $need_for_file, "No need to regenerate $f");
+ chdir $cwd or
+ croak "Unable to change back from temporary directory after testing";
+}
+
+# 2: Copy the real MANIFEST to the tempdir but mangle it there.
+# The alteration in the copied MANIFEST will be sufficient to require
+# regeneration of MANIFEST.
+{
+ my $tdir = tempdir( CLEANUP => 1 );
+ chdir $tdir or
+ croak "Unable to change to temporary directory for testing";
+ copy(qq{$cwd/$f}, qq{$tdir/$f})
+ or croak "Unable to copy $f to tempdir";
+ ok(-f $f, "$f found in tempdir");
+ my @lines;
+ tie @lines, 'Tie::File', qq{$tdir/$f}
+ or croak "Unable to tie to $f in tempdir";
+ for (1..10) {
+ if ( defined($lines[-1]) ) {
+ pop @lines;
+ }
+ }
+ untie @lines or croak "Unable to untie from $f";
+ my $need_for_file =
+ $mani->determine_need_for_manifest($manifest_lines_ref);
+ ok($need_for_file, "Need to regenerate $f");
+ ok( $mani->print_manifest($manifest_lines_ref),
+ "print_manifest() returned true");
+ ok( -f $f,
+ "$f has been created in tempdir");
+ chdir $cwd or
+ croak "Unable to change back from temporary directory after testing";
+}
+
+pass("Completed all tests in $0");
+
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+02-regenerate_file.t - test C<Parrot::Manifest> MANIFEST-related methods
+
+=head1 SYNOPSIS
+
+ % prove t/manifest/02-regenerate_file.t
+
+=head1 DESCRIPTION
+
+The files in this directory test the publicly callable methods of
+F<lib/Parrot/Manifest.pm> and packages which inherit from that package.
+
+F<02-regenerate_file.t> tests whether Parrot::Manifest correctly determines
+whether MANIFESTneeds to be regenerated or not.
+
+=head1 AUTHOR
+
+James E Keenan ([EMAIL PROTECTED])
+
+=head1 SEE ALSO
+
+Parrot::Manifest, Parrot::Manifest::Files, Parrot::Manifest::Skip,
+F<tools/dev/mk_manifest_and_skip.pl>.
+
+=cut
+
+# Local Variables:
+# mode: cperl
+# cperl-indent-level: 4
+# fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:
+
Property changes on: t/manifest/02-regenerate_file.t
___________________________________________________________________
Name: svn:mime-type
+ text/plain
Name: svn:keywords
+ Author Date Id Revision
Name: svn:eol-style
+ native
Index: t/manifest/03-regenerate_skip.t
===================================================================
--- t/manifest/03-regenerate_skip.t (revision 0)
+++ t/manifest/03-regenerate_skip.t (revision 0)
@@ -0,0 +1,113 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# 03-regenerate_skip.t
+
+use strict;
+use warnings;
+
+use Test::More tests => 10;
+use Carp;
+use Cwd;
+use File::Copy;
+use File::Temp qw( tempdir );
+use Tie::File;
+use lib ( qw| lib | );
+use_ok('Parrot::Manifest');
+
+my $script = $0;
+my $mani = Parrot::Manifest->new( {
+ script => $script,
+} );
+isa_ok($mani, 'Parrot::Manifest');
+
+my $cwd = cwd();
+my $sk = q{MANIFEST.SKIP};
+my $print_str = $mani->prepare_manifest_skip();
+ok($print_str, "prepare_manifest_skip() returned");
+
+# 1: Copy the real MANIFEST.SKIP unaltered to the tempdir.
+# Assuming the real MANIFEST.SKIP was correct going in to this test, the
+# absence of any change in it will mean that there will be no need to
+# regenerate it.
+{
+ my $tdir = tempdir( CLEANUP => 1 );
+ chdir $tdir or
+ croak "Unable to change to temporary directory for testing";
+ copy(qq{$cwd/$sk}, qq{$tdir/$sk})
+ or croak "Unable to copy $sk to tempdir";
+ ok(-f $sk, "$sk found in tempdir");
+ my $need_for_skip = $mani->determine_need_for_manifest_skip($print_str);
+ ok(! $need_for_skip, "No need to regenerate $sk");
+ chdir $cwd or
+ croak "Unable to change back from temporary directory after testing";
+}
+
+# 2: Copy the real MANIFEST.SKIP to the tempdir but mangle it there.
+# The alteration in the copied MANIFEST.SKIP will be sufficient to require
+# regeneration of MANIFEST.SKIP.
+{
+ my $tdir = tempdir( CLEANUP => 1 );
+ chdir $tdir or
+ croak "Unable to change to temporary directory for testing";
+ copy(qq{$cwd/$sk}, qq{$tdir/$sk})
+ or croak "Unable to copy $sk to tempdir";
+ ok(-f $sk, "$sk found in tempdir");
+ my @lines;
+ tie @lines, 'Tie::File', qq{$tdir/$sk}
+ or croak "Unable to tie to $sk in tempdir";
+ for (1..10) {
+ if ( defined($lines[-1]) ) {
+ pop @lines;
+ }
+ }
+ untie @lines or croak "Unable to untie from $sk";
+ my $need_for_skip = $mani->determine_need_for_manifest_skip($print_str);
+ ok($need_for_skip, "Need to regenerate $sk");
+ ok( $mani->print_manifest_skip($print_str),
+ "print_manifest_skip() returned true");
+ ok( -f $sk,
+ "$sk has been created in tempdir");
+ chdir $cwd or
+ croak "Unable to change back from temporary directory after testing";
+}
+
+pass("Completed all tests in $0");
+
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+03-regenerate_skip.t - test C<Parrot::Manifest> MANIFEST.SKIP-related methods
+
+=head1 SYNOPSIS
+
+ % prove t/manifest/03-regenerate_skip.t
+
+=head1 DESCRIPTION
+
+The files in this directory test the publicly callable methods of
+F<lib/Parrot/Manifest.pm> and packages which inherit from that package.
+
+F<03-regenerate_skip.t> tests whether Parrot::Manifest correctly determines
+whether MANIFEST.SKIP needs to be regenerated or not.
+
+=head1 AUTHOR
+
+James E Keenan ([EMAIL PROTECTED])
+
+=head1 SEE ALSO
+
+Parrot::Manifest, Parrot::Manifest::Files, Parrot::Manifest::Skip,
+F<tools/dev/mk_manifest_and_skip.pl>.
+
+=cut
+
+# Local Variables:
+# mode: cperl
+# cperl-indent-level: 4
+# fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:
+
Property changes on: t/manifest/03-regenerate_skip.t
___________________________________________________________________
Name: svn:mime-type
+ text/plain
Name: svn:keywords
+ Author Date Id Revision
Name: svn:eol-style
+ native
