On Mon, May 9, 2011 at 18:30, Julian Foad <julian.foad_at_wandisco.com> wrote:
> I'm planning to commit the attached wc-db-verification-1.patch, subject
> to any advice on how to best fit it in to the code base or other
> concerns, in order to get a DB "self-check" function started.
>
> I think we need something like this. Earlier today I found that "svn
> status" showed I had a clean, single-rev WC, while "svnversion" said it
> was mixed-rev and switched. Investigation showed there were orphaned
> base node rows in the DB which weren't seen by "svn st" but were seen by
> "svnversion". I'm not interested in how that particular state came to
> be, as I've run hundreds of buggy trunk builds on this WC over many
> months. What I do want is to be able to run a set of checks on a DB
> that detects basic rule violations like that.
>
> Decisions about when and how to run it can come later. Of course if we
> plan to run it frequently and automatically that would mean we'd want to
> make sure it only did fast checks and is efficiently coded whereas if it
> remains a manual intervention for devs then that's no concern. Thoughts
> about this are welcome too.
Regarding the patch itself:
* I dislike creating yet another wc_db_foo file. Just put the damned
thing into wc_db_util.c. These files are not so big that we have to
break them up. wc_db.c, sure. But let's not keep adding files that are
just dozens of lines. It makes it hard to answer the question, "where
is that function?"
* There is a rough sketch of wc-checks.sql. Is there something that
can be done where, for developers, more intense checks are performed?
And even better, can some of these checks be expressed as triggers, in
order to trap problems immediately?
* db_verify() should at least follow precedent with a
VERIFY_USABLE_WCROOT() call. (why'd you omit that?)
* the (new) typical pattern is to pass a WCROOT rather than the
individual SDB and WC_ID, so _verify_nodes should have its args
changed
* wtf is relpath_depth duplicated into the verify file? serious badness.
* there is no such comment as "TODO:" ... we use "###"
-g
Received on 2011-05-10 11:29:45 CEST