Skip Menu |

This queue is for tickets about the App-SnerpVortex CPAN distribution.

Report information
The Basics
Id: 65595
Status: open
Priority: 0/
Queue: App-SnerpVortex
People
Owner: Nobody in particular
Requestors: nick [...] ccl4.org
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: 1.000
Fixed in: (no value)
Attachments
65595.patch
t.dump1

History Show all quoted text
  Wed Feb 09 12:13:49 2011 nick [...] ccl4.org - Ticket created
Subject: snanalyze chokes on a pathological copy/delete/add combo
snanalyze will choke on the dump file attached: $ /home/netbanx/.cpan/build/App-SnerpVortex-1.000-axQdhu/bin/snanalyze --dump=t.dump1 --db=db1 trunk/dir not a container of trunk/dir at 5 at /export/home/netbanx/base/usr/lib/perl5/site_perl/5.10.1/SVN/Analysis.pm line 847 SVN::Analysis::_get_container_paths('SVN::Analysis=HASH(0x8ba5770)', 'trunk/dir', 5) called at /export/home/netbanx/base/usr/lib/perl5/site_perl/5.10.1/SVN/Analysis.pm line 799 SVN::Analysis::_touch_directory('SVN::Analysis=HASH(0x8ba5770)', 'trunk/dir', 5) called at /export/home/netbanx/base/usr/lib/perl5/site_perl/5.10.1/SVN/Analysis.pm line 792 SVN::Analysis::_touch_parent_directory('SVN::Analysis=HASH(0x8ba5770)', 'trunk/dir/file', 5) called at /export/home/netbanx/base/usr/lib/perl5/site_perl/5.10.1/SVN/Analysis.pm line 241 SVN::Analysis::consider_delete('SVN::Analysis=HASH(0x8ba5770)', 'trunk/dir/file', 5) called at /export/home/netbanx/base/usr/lib/perl5/site_perl/5.10.1/SVN/Dump/Analyzer.pm line 76 SVN::Dump::Analyzer::on_node_delete('SVN::Dump::Analyzer=HASH(0x8a69030)', 5, 'trunk/dir/file') called at /export/home/netbanx/base/usr/lib/perl5/site_perl/5.10.1/SVN/Dump/Walker.pm line 142 SVN::Dump::Walker::walk('SVN::Dump::Analyzer=HASH(0x8a69030)') called at /home/netbanx/.cpan/build/App-SnerpVortex-1.000-axQdhu/bin/snanalyze line 60 The specific commit that it takes objection to was this sequence of actions: $ svn cp file:///home/netbanx/tmp/testit/trunk/dir@2 . A dir/file Checked out revision 2. A dir $ svn rm dir/file D dir/file $ svn cp file:///home/netbanx/tmp/testit/trunk/dir/file@2 dir/ A dir/file $ svn diff $ svn st A + dir R + dir/file $ EDITOR=vi svn commit which looks like this in the dump: Revision-number: 5 Prop-content-length: 263 Content-length: 263 K 7 svn:log V 160 Copy the directory from a historical revision. Delete its contents. Copy some contents from a historical revision (which happens to be the same, but nevermind) K 10 svn:author V 7 netbanx K 8 svn:date V 27 2011-02-09T17:02:02.426314Z PROPS-END Node-path: trunk/dir Node-kind: dir Node-action: add Node-copyfrom-rev: 2 Node-copyfrom-path: trunk/dir Node-path: trunk/dir/file Node-kind: file Node-action: delete Node-path: trunk/dir/file Node-kind: file Node-action: add Node-copyfrom-rev: 2 Node-copyfrom-path: trunk/dir/file Text-copy-source-md5: d41d8cd98f00b204e9800998ecf8427e This case may not be the simplest - I've not tried doing a single commit that doesn't have the delete in it - ie a copy of an empty directory from a historical revision, and a copy of a file into it from a historical revision. But this case is pretty small :-) I wouldn't know where to start to try to fix this. Right now I've fallen back to trying git svn, but it's slow and I don't know if it's going to do a good enough job.
Subject: t.dump1
Download t.dump1
application/octet-stream 2.4k

Message body not shown because it is not plain text.

  Fri Feb 11 11:55:53 2011 nick [...] ccl4.org - Correspondence added
From: nick [...] ccl4.org
On Wed Feb 09 12:13:49 2011, nick@ccl4.org wrote: Show quoted text
> snanalyze will choke on the dump file attached:
At the point when it chokes, dir looks like this: Show quoted text
sqlite> select is_active, path, rev_first, rev_last from dir;
is_active path rev_first rev_last ---------- ---------- ---------- ---------- 1 0 1 1 branches 1 1 1 tags 1 1 1 trunk 1 5 0 trunk/dir 2 4 Note that trunk/dir *isn't* active (correctly, if I understand correctly what active means). However, the query to determine what gets copied is: SELECT path, rev_first FROM dir WHERE (path = ? OR path LIKE ?) AND rev_first <= ? AND is_active = 1 ORDER BY length(path) DESC, path ASC ie when at r5, copying trunk/dir@2, it won't return anything, because trunk/dir is *not* active. I think that the correct fix is to change the `is_active = 1` to `(is_active = 1 OR ? < rev_last)`. Suggested patch attached. With this, the test dump file is now processed without error. (Although I didn't verify that what it produces is sane, because I'm not sure what sanity should look like). Obviously, don't assume that this patch is correct, because I don't know how good either my reasoning about the cause is, or how good my implementation of my intended change is. I'm now trying this revised version on my real (full) repositories, which takes a bit longer than the test case repository :-)
Subject: 65595.patch
--- lib/SVN/Analysis.pm~ 2010-10-18 16:38:46.000000000 +0100 +++ lib/SVN/Analysis.pm 2011-02-11 16:22:56.000000000 +0000 @@ -864,11 +864,11 @@ my $sth = $self->dbh()->prepare_cached(" SELECT count(rev_first) as ct FROM dir - WHERE path = ? AND rev_first <= ? AND is_active = 1 + WHERE path = ? AND rev_first <= ? AND (is_active = 1 OR rev_last > ?) ORDER BY path DESC, rev_first DESC ") or die $self->dbh()->errstr(); - $sth->execute($path, $revision) or die $sth->errstr(); + $sth->execute($path, $revision, $revision) or die $sth->errstr(); my $exists = 0; $sth->bind_columns(\$exists) or die $sth->errstr(); @@ -886,13 +886,13 @@ my $sth = $self->dbh()->prepare_cached(" SELECT path, rev_first FROM dir - WHERE (path = ? OR path LIKE ?) AND rev_first <= ? AND is_active = 1 + WHERE (path = ? OR path LIKE ?) AND rev_first <= ? AND (is_active = 1 OR ? < rev_last) ORDER BY length(path) DESC, path ASC ") or die $self->dbh()->errstr(); (my $partial_path = $path) =~ s!/*$!/%!; - $sth->execute($path, $partial_path, $revision) or die $sth->errstr(); + $sth->execute($path, $partial_path, $revision, $revision) or die $sth->errstr(); $sth->bind_columns(\my ($found_path, $found_rev)) or die $sth->errstr();
  Fri Feb 11 12:17:48 2011 nick [...] ccl4.org - Correspondence added
From: nick [...] ccl4.org
On Fri Feb 11 11:55:53 2011, nick@ccl4.org wrote: Show quoted text
> I'm now trying this revised version on my real (full) repositories, > which takes a bit longer than the test case repository :-)
Forgot to add that I don't know whether the suggested change to the queries renders the current indexes less than useful.
  Sun Mar 04 12:35:23 2012 RCAPUTO [...] cpan.org - Correspondence added
On Fri Feb 11 12:17:48 2011, nick@ccl4.org wrote: Show quoted text
> On Fri Feb 11 11:55:53 2011, nick@ccl4.org wrote: >
> > I'm now trying this revised version on my real (full) repositories, > > which takes a bit longer than the test case repository :-)
> > Forgot to add that I don't know whether the suggested change to the > queries renders the current indexes less than useful.
Did your patch work? I can optimize the indexes if the patch is good.
  Sun Mar 04 12:35:26 2012 The RT System itself - Status changed from 'new' to 'open'