Skip to content

Commit

Permalink
Item14368: reinstated RcsWrap to handler tests, fixed the problems th…
Browse files Browse the repository at this point in the history 
…at showed up (none serious), added tests for edge cases of getRevision. Still not sure what causes the corrupt histories in RcsLite, but closer to an understanding.
  • Loading branch information
@gac410
cdot authored and gac410 committed Apr 4, 2017
1 parent b081dd9 commit ee3a5bd
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 45 deletions.
55 changes: 34 additions & 21 deletions RCSStoreContrib/lib/Foswiki/Store/Rcs/RcsLiteHandler.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,8 @@ sub _ensureRead {

$downToVersion ||= 0; # just in case

# If we only need tha latest and we already have the head, that's our rev
$downToVersion = $this->{head} if !$downToVersion && $this->{head};
# If we only need the latest and we already have the head, that's our rev
$downToVersion = $this->{head} if ( !$downToVersion && $this->{head} );

$downToVersion = 1 if $downToVersion < 0; # read everything

Expand Down Expand Up @@ -286,22 +286,26 @@ sub _ensureRead {

#print STDERR "Reading ".($historyOnly?'history':'everything')." to $downToVersion\n";

# We *will* end up re-reading the history if we previously only read the history; there is
# no way to restart the parse mid-stream (though an ftell and fseek would do it if we saved
# the rest of the state)
# We *will* end up re-reading the history if we previously only
# read the history; there is no way to restart the parse mid-stream
# (though an ftell and fseek would do it if we saved the rest of the
# state)
while (1) {
( $_, $string ) = _readTo( $fh, $term );
last if ( !$_ );

#print STDERR "expecting $state: seeing $_\n";

if ( $state eq 'admin.head' ) {
if (/^head\s+([0-9]+)\.([0-9]+);$/o) {
if (/^head\s+([0-9]+)\.([0-9]+);$/) {
ASSERT( $1 eq 1 ) if DEBUG;
$headNum = $2;

# If $downToVersion is 0, we now know what version to ready up to
$downToVersion = $headNum unless $downToVersion;
# If $downToVersion is 0, we now know what version to read up to
$downToVersion = $headNum
if $downToVersion <= 0
|| $downToVersion > $headNum;

$state = 'admin.access'; # Don't support branches
}
else {
Expand Down Expand Up @@ -533,8 +537,11 @@ sub ci {
}
my $head = $this->{head} || 0;
if ($head) {
my $lNew = _split($data);
my $lOld = _split( $this->{revs}[$head]->{text} );
my $lNew = _split($data);

# The top of the revisions array contains the plain text of
# the topic in it's latest incarnation, with no differences.
my $lOld = _split( $this->{revs}[$head]->{text} );
my $delta = _diff( $lNew, $lOld );
$this->{revs}[$head]->{text} = $delta;
}
Expand Down Expand Up @@ -678,22 +685,28 @@ sub _patch {
my $length = $3;
if ( $act eq 'd' ) {
my $start = $offset + $adj - 1;

# If the splice fails, it's almost certainly a problem in the
# later revision
my @removed = splice( @$text, $start, $length );
$adj -= $length;
$pos++;
}
elsif ( $act eq 'a' ) {
my @toAdd = @$delta[ $pos + 1 .. $pos + $length ];

# Fix for Item2957
# Check if the last element of what is to be added contains
# a valid marker. If it does, the chances are very high that
# this topic was saved using a broken version of RcsLite, and
# a line ending has been lost.
# Fixes for twikibug Item2957
# Check if the last element of what is to be
# added contains a valid marker. If it does, the chances
# are very high that this topic was saved using a broken
# version of RcsLite, and a line ending has been lost.
# As soon as a topic containing this problem is re-saved
# using this code, the need for this hack should go away,
# as the line endings will now be correct.
if ( scalar(@toAdd)
# If the first element contains a valid marker this is
# also indicative of a problem, but it's unclear how to
# resolve that as the cause is unknown.
if (scalar(@toAdd)
&& $toAdd[$#toAdd] =~ /^([ad])(\d+)\s(\d+)$/
&& $2 > $pos )
{
Expand Down Expand Up @@ -726,6 +739,7 @@ sub getRevision {
return $this->SUPER::getRevision($version);
}

$version = 0 if $version < 0;
$this->_ensureRead( $version, 0 );

return $this->SUPER::getRevision($version)
Expand All @@ -734,12 +748,11 @@ sub getRevision {
my $head = $this->{head};
return $this->SUPER::getRevision($version) unless $head;

if ( $version == $head ) {
return ( $this->{revs}[$version]->{text}, 1 );
}
$version = $head if $version > $head;
my $headText = $this->{revs}[$head]->{text};
my $text = _split($headText);
if ( $version <= 0 || $version >= $head ) {
return ( $headText, 1 );
}
my $text = _split($headText);
return ( _patchN( $this, $text, $head - 1, $version ), 0 );
}

Expand Down
50 changes: 29 additions & 21 deletions RCSStoreContrib/lib/Foswiki/Store/Rcs/RcsWrapHandler.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ sub ci {

$comment = 'none' unless $comment;

undef $this->{numRevisions};

my ( $cmd, $rcsOutput, $exit, $stderr );
if ( defined($date) ) {
require Foswiki::Time;
Expand Down Expand Up @@ -187,6 +189,7 @@ sub repRev {
$date = Foswiki::Time::formatTime( $date, '$rcs', 'gmtime' );

_lock($this);
undef $this->{numRevisions};
my ( $rcsOut, $exit ) = Foswiki::Sandbox->sysCommand(
$Foswiki::cfg{RCS}{ciDateCmd},
DATE => $date,
Expand All @@ -213,17 +216,17 @@ sub _deleteRevision {
my ( $this, $rev ) = @_;

# delete latest revision (unlock (may not be needed), delete revision)
my ( $rcsOut, $exit ) =
my ( $rcsOut, $exit, $stderr ) =
Foswiki::Sandbox->sysCommand( $Foswiki::cfg{RCS}{unlockCmd},
FILENAME => _encode( $this->{file}, 1 ) );
if ($exit) {
throw Error::Simple(
$Foswiki::cfg{RCS}{unlockCmd} . ' failed: ' . $rcsOut );
"$Foswiki::cfg{RCS}{unlockCmd} failed: $rcsOut $stderr");
}

chmod( $Foswiki::cfg{Store}{filePermission}, _encode( $this->{file}, 1 ) );

( $rcsOut, $exit ) = Foswiki::Sandbox->sysCommand(
( $rcsOut, $exit, $stderr ) = Foswiki::Sandbox->sysCommand(
$Foswiki::cfg{RCS}{delRevCmd},
REVISION => '1.' . $rev,
FILENAME => _encode( $this->{file}, 1 )
Expand All @@ -232,13 +235,13 @@ sub _deleteRevision {
if ($exit) {
throw Error::Simple( $Foswiki::cfg{RCS}{delRevCmd} . ' of '
. $this->hidePath( $this->{file} )
. ' failed: '
. $rcsOut );
. " failed: $rcsOut $stderr" );
}

# Update the checkout
undef $this->{numRevisions};
$rev--;
( $rcsOut, $exit ) = Foswiki::Sandbox->sysCommand(
( $rcsOut, $exit, $stderr ) = Foswiki::Sandbox->sysCommand(
$Foswiki::cfg{RCS}{coCmd},
REVISION => '1.' . $rev,
FILENAME => _encode( $this->{file}, 1 )
Expand All @@ -247,8 +250,7 @@ sub _deleteRevision {
if ($exit) {
throw Error::Simple( $Foswiki::cfg{RCS}{coCmd} . ' of '
. $this->hidePath( $this->{file} )
. ' failed: '
. $rcsOut );
. " failed: $rcsOut $stderr" );
}
$this->saveFile( $this->{file}, $rcsOut );
}
Expand All @@ -265,17 +267,13 @@ sub getRevision {
{

# Get the latest rev from the cache
return ( $this->SUPER::getRevision($version) );
return ( $this->SUPER::getRevision($version), 1 );
}

# We've been asked for an explicit rev. The rev might be outside the
# range of revs in RCS. RCS will return the latest, though it reports
# the rev retrieved to STDERR (no use to us, as we have no access
# to STDERR)

# SMELL: we need to determine if the rev we are returning is the latest.
# co prints the retrieved revision, but unfortunately it prints it
# to STDERR, which the Sandbox can't retrieve.
# the rev retrieved to STDERR
$version = 2 ^ 31 if $version <= 0;

my $tmpfile;
my $tmpRevFile;
Expand Down Expand Up @@ -313,7 +311,16 @@ sub getRevision {
if ( defined $stderr
&& $stderr =~ /revision 1\.(\d+)/s )
{
$isLatest = ( $version >= $1 );
if ( $version > $1 ) {
$this->{numRevisions} = $1;
$isLatest = 1;
}
elsif ( defined $this->{numRevisions} ) {
$isLatest = ( $1 == $this->{numRevisions} );
}
else {
$isLatest = ( $1 == $this->_numRevisions() );
}
}

# otherwise we will have to resort to numRevisions to tell if
Expand Down Expand Up @@ -376,11 +383,12 @@ sub getInfo {
sub _numRevisions {
my $this = shift;

return $this->{numRevisions} if defined $this->{numRevisions};

unless ( $this->revisionHistoryExists() ) {

# If there is no history, there can only be one.
return 1 if $this->storedDataExists();
return 0;
return $this->{numRevisions} = $this->storedDataExists() ? 1 : 0;
}

my ( $rcsOutput, $exit ) =
Expand All @@ -394,12 +402,12 @@ sub _numRevisions {
. $rcsOutput );
}
if ( $rcsOutput =~ /head:\s+\d+\.(\d+)\n/ ) {
return $1;
return $this->{numRevisions} = $1;
}
if ( $rcsOutput =~ /total revisions: (\d+)\n/ ) {
return $1;
return $this->{numRevisions} = $1;
}
return 1;
return $this->{numRevisions} = 1;
}

# implements Rcs::Handler
Expand Down
25 changes: 22 additions & 3 deletions RCSStoreContrib/test/unit/RCSStoreContrib/RCSHandlerTests.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ sub fixture_groups {
my $groups = [];

push( @$groups, 'RcsLite' );
return ($groups);

# return ($groups);

if ( FoswikiStoreTestCase::rcs_is_installed() ) {
push( @$groups, 'RcsWrap' );
Expand Down Expand Up @@ -910,19 +911,37 @@ HERE
$rcs->addRevisionFromText( $rev2, "more", "idiot", $time );
$rcs = $class->new( new StoreStub, $testWeb, 'Item2957', '' );
$rcs->addRevisionFromText( $rev3, "more", "idiot", $time );

$rcs = $class->new( new StoreStub, $testWeb, 'Item2957', '' );
my ($text) = $rcs->getRevision(1);
if ( $Foswiki::cfg{OS} eq 'WINDOWS' ) {
$text =~ s/\r\n/\n/sg;
}
$this->assert_equals( $rev1, $text );

my $isl;
$rcs = $class->new( new StoreStub, $testWeb, 'Item2957', '' );
($text) = $rcs->getRevision(2);
( $text, $isl ) = $rcs->getRevision(1);
$this->assert_equals( $rev1, $text );
$this->assert( !$isl );
$rcs = $class->new( new StoreStub, $testWeb, 'Item2957', '' );
( $text, $isl ) = $rcs->getRevision(2);
$this->assert( !$isl );
$this->assert_equals( $rev2, $text );
$rcs = $class->new( new StoreStub, $testWeb, 'Item2957', '' );
($text) = $rcs->getRevision(3);
( $text, $isl ) = $rcs->getRevision(3);
$this->assert($isl);
$this->assert_equals( $rev3, $text );
$rcs = $class->new( new StoreStub, $testWeb, 'Item2957', '' );
( $text, $isl ) = $rcs->getRevision(4);
$this->assert($isl);
$this->assert_equals( $rev3, $text );
( $text, $isl ) = $rcs->getRevision(0);
$this->assert($isl);
$this->assert_equals( $rev3, $text );
( $text, $isl ) = $rcs->getRevision(-1);
$this->assert_equals( $rev3, $text );
$this->assert($isl);
}

sub verify_Item3122 {
Expand Down

0 comments on commit ee3a5bd

Please sign in to comment.