On Sun Jul 15 09:25:30 2007, coke wrote: > The recently added functionality to not regen MANIFEST.SKIP should be > used when determining whether or not to regenerate MANIFEST as well. >
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