[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r34506 - trunk/subversion/libsvn_fs_base

From: David Glasser <glasser_at_davidglasser.net>
Date: Mon, 1 Dec 2008 11:45:03 -0800

I'm not sure; I think that base_node_origin_rev refuses to return
anything at all if the repository is too old? (Which I found
surprising, since FSFS is happy to run the algorithm no matter what.)

--dave

On Mon, Dec 1, 2008 at 11:13 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> David, is there an easy way to use the C test suite's new support for
> optionally making older-style repositories to test for these bugs?
>
> glasser_at_tigris.org wrote:
>> Author: glasser
>> Date: Mon Dec 1 09:45:39 2008
>> New Revision: 34506
>>
>> Log:
>> Fix a few bugs (one which can lead to a crash and one to an incorrect
>> error) in the BDB implementation of node-origin-rev calculation.
>>
>> These bugs should have been caught by the C tests for
>> svn_fs_node_origin_rev, but the code is practically dead code: it's
>> essentially only used for legacy repositories, since any node created
>> in a 1.5-era repository will immediately have its origin ID stored in
>> the node-origins cache. To reveal these bugs, go into
>> svn_fs_bdb__get_node_origin, and comment out the "if (db_err ==
>> DB_NOTFOUND)" line (ie, make it always be a cache miss).
>>
>> Then run fs-test 34: you'll get a segfault in the last svn_path_join
>> of prev_location. This is because "path" isn't canonicalized ("A/D")
>> but "copy_path" is ("/A/D"), so the svn_path_is_child call returns
>> NULL for remainder. The first change to base_node_origin_rev fixes
>> this. (Though perhaps the correct answer is to change the test to
>> always pass absolute paths? It's unclear to me if
>> base_node_origin_rev's use of relative paths is actually legal.)
>>
>> However, you then end up with a "File not found: revision 7, path
>> '/A/D2'" error; this is because the loop calling prev_location
>> accidentally fails to update the revision that it's looking at with
>> the revision returned from prev_location, despite a comment to the
>> contrary. The second change to base_node_origin_rev fixes this. Now
>> the test happily passes without the cache, and you can remove the hack
>> to svn_fs_bdb__get_node_origin.
>>
>> This change was tested by running "make check FS_TYPE=bdb" with the
>> svn_fs_bdb__get_node_origin hack still in. Modifying the C test suite
>> so that it manages to test this code without a manual hack is left as
>> an exercise to the reader. (OK, that's a lie: after about 8 hours of
>> running the test suite on my Mac (with no failures, and fs-test having
>> passed) I got bored of waiting and killed it. Then I asked on dev@,
>> and people said that bdb tests were just known to be slow on OS X, so
>> I decided to try it on my Linux box instead. But after 45 minutes of
>> trying to figure out how to build trunk Subversion on a Dapper box
>> with SQLite 3.2 installed in /usr/lib and failing, I figured I might
>> as well just commit this fix. It's not like anybody noticed the
>> segfault bug in the past year. Maybe nobody uses fs_base anymore
>> anyway. I don't know why they would.)
>>
>> * subversion/libsvn_fs_base/tree.c
>> (base_node_origin_rev): Canonicalize the input path as an absolute
>> path. Actually update a revision variable in a loop instead of
>> staying stuck on the revision it started with.
>>
>> Modified:
>> trunk/subversion/libsvn_fs_base/tree.c
>>
>> Modified: trunk/subversion/libsvn_fs_base/tree.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/tree.c?pathrev=34506&r1=34505&r2=34506
>> ==============================================================================
>> --- trunk/subversion/libsvn_fs_base/tree.c Mon Dec 1 09:15:13 2008 (r34505)
>> +++ trunk/subversion/libsvn_fs_base/tree.c Mon Dec 1 09:45:39 2008 (r34506)
>> @@ -4795,6 +4795,8 @@ base_node_origin_rev(svn_revnum_t *revis
>> SVN_ERR(svn_fs_base__test_required_feature_format
>> (fs, "node-origins", SVN_FS_BASE__MIN_NODE_ORIGINS_FORMAT));
>>
>> + path = svn_fs__canonicalize_abspath(path, pool);
>> +
>> SVN_ERR(base_node_id(&id, root, path, pool));
>> args.node_id = svn_fs_base__id_node_id(id);
>> err = svn_fs_base__retry_txn(root->fs, txn_body_get_node_origin,
>> @@ -4847,6 +4849,7 @@ base_node_origin_rev(svn_revnum_t *revis
>> /* Update our LASTPATH and LASTREV variables (which survive
>> SUBPOOL). */
>> svn_stringbuf_set(lastpath, curpath);
>> + lastrev = currev;
>> }
>>
>> /* Walk the predecessor links back to origin. */
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
>> For additional commands, e-mail: svn-help_at_subversion.tigris.org
>>
>
>
> --
> C. Michael Pilato <cmpilato_at_collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
>

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-12-01 20:45:14 CET

This is an archived mail posted to the Subversion Dev mailing list.