Philip Martin wrote on Wed, Jul 25, 2012 at 12:06:48 +0100:
> Daniel Shahaf <danie...@elego.de> writes:
>
> > Philip Martin wrote on Wed, Jul 25, 2012 at 11:09:54 +0100:
> >> Daniel Shahaf <danie...@elego.de> writes:
> >>
> >> > Philip Martin wrote on Wed, Jul 25, 2012 at 10:49:36 +0100:
> >> >>
> >> >> I attached to the issue a repository that demonstrates the corruption;
> >> >> verify doesn't report a problem on that repository. What does verify
> >> >> check?
> >> >
> >> > validate_root_noderev() catches an instance of the #4129 corruption and
> >> > in its docstring xrefs svn_fs_fs__verify(), so I think the checks in the
> >> > former are done in the latter too.
> >>
> >> validate_root_noderev is only called by write_final_rev and not during
> >> verify. svn_fs_fs__verify calls svn_fs_fs__verify_root and that does:
> >>
> >> /* Issue #4129: bogus predecessors. */
> >> /* Check 1: predecessor must be an earlier revision.
> >> */
> >> if (pred_rev >= root->rev)
> >> return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> >> "r%ld's root node's predecessor is r%ld"
> >> " but must be earlier revision",
> >> root->rev, pred_rev);
> >
> > The check was included in svn_fs_fs__verify_root() in r1349313:
> >
> > /* Verify explicitly the predecessor of the root. */
> > {
> > ...
> > /* Check the predecessor's revision. */
> > if (pred_id)
> > {
> > SVN_ERR(svn_fs_fs__dag_get_node(&pred, root->fs, pred_id, pool));
> > SVN_ERR(svn_fs_fs__dag_get_revision(&pred_rev, pred, pool));
> > if (pred_rev+1 != root->rev)
> > /* Issue #4129. */
> > return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> > "r%ld's root node's predecessor is r%ld",
> > root->rev, pred_rev);
> > }
> > {
>
> That check was relaxed by:
>
> r1358322 | stefan2 | 2012-07-06 19:04:09 +0100 (Fri, 06 Jul 2012) | 5 lines
>
> Relax overly strict consistency check that is not covered by the FSFS
> format specification.
>
> * subversion/libsvn_fs_fs/tree.c
> (svn_fs_fs__verify_root): fix test and add commentary
>
Thanks for digging it. I think that change is wrong --- the predecessor
of the root noderev of rN MUST be the root noderev of r(N-1) --- that's
how Subversion filesystems behave (not just FSFS).
Stefan2, WDYT?
>
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download
