Hyrum K. Wright wrote on Sat, 27 Dec 2008 at 14:00 -0600:
> Daniel Shahaf wrote:
> > Daniel Shahaf wrote on Sat, 27 Dec 2008 at 20:50 +0200:
> >> IOW, it forgets to add the offset of the rev within the pack file.
> >>
> >
> > And here's the patch I'm testing:
> >
> > [[[
> > Fix 'svnadmin verify' of some packed FSFS repositories by making the
> > calculation of offsets in the rev file consistent.
> >
...
> > @@ -2309,9 +2302,15 @@ get_root_changes_offset(apr_off_t *root_offset,
> > else
> > {
> > seek_relative = APR_END;
> > - offset = 0;
> > + rev_offset = offset = 0;
>
> I don't know what our style is here, but I recommend having these assignments on
> separate lines, as it sure helps readability.
>
Okay, but I will just remove this line entirely, since 'rev_offset' is
set by the 'if' just below:
> > }
> >
> > + /* Offset of the revision from the start of the pack file, if applicable. */
> > + if (rev < ffd->min_unpacked_rev)
> > + SVN_ERR(get_packed_offset(&rev_offset, fs, rev, pool));
> > + else
> > + rev_offset = 0;
> > +
> > /* We will assume that the last line containing the two offsets
> > will never be longer than 64 characters. */
> > SVN_ERR(svn_io_file_seek(rev_file, seek_relative, &offset, pool));
...
> > ]]]
> >
> > I'll appreciate a review by someone more familiar with FSFS than me.
>
> Other than that, it looks good. It took me a minute to figure out what was
> going on here, but your solution looks reasonable.
>
Thanks for the review. Committed in r34958.
> > P.S. I've also noticed that 'svnadmin recover svn-r9-packed' (where
> > svn-r9-packed is the repository created by the script I gave in the
> > original mail) fails (both with and without the patch) with the error:
> >
> > ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:6155: (apr_err=160004)
> > svnadmin: Expected current rev to be <= 0 but found 9
>
> Does this happen with the inconsistent repository you created, or with the sync
> running to some sort of completion? What about a non-packed repository which
> was aborted?
>
It happens on a 3-revisions packed Greek repository, and doesn't happen
on the same repository after I dump|load it to get a non-packed version.
The results with the packed/non-packed syncs of the svn repository are
consistent (error only with the packed sync). In short, it seems to
happen iff the repos is packed.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=993938
Received on 2008-12-27 22:50:13 CET