To keep everybody updated... I've tracked this down. Please, put on
your rebreathers... you don't want to smell this.
The original code maintained two lists of entries inside the access baton:
1) "all" entries that are known to svn
2) "visible" entries
The second list is a subset of the first. It doesn't contain things
like excluded subdirs, absent nodes, items deleted in the parent's
rev, etc. The logical set of children, rather than "all children we
need to account for".
In the particular test that I examined, it constructed a loggy log like this:
1) set some values for entry FOO
2) set some wcprops for entry FOO
3) set more values or FOO, including DELETED=FALSE
Now... in certain circumstances, FOO already existed and was marked as
DELETED before the above log was run. Step (2) looks at the "trimmed"
list, so it *should* have failed because FOO was still marked as
DELETED.
But... you guessed it. Step (2) did not fail.
In step (1), the "set-values" step was applied to *both* lists. The
full list, and the trimmed list. FOO did not exist in the trimmed
list, so this step *inserted* the entry.
And here is the problem. That insertion occurred in-place. What was
supposed to be a "trimmed" list now has entries counter to its
definition. It now contains a DELETED node. As a result, when step (2)
fetches the trimmed list, it thinks the node is present, so it
continues happily.
Enter the new code, as defined by r36663. It is impossible to modify
the trimmed list since it is generated upon request. Thus, step (1)
modified both lists, but it didn't "stick". Step (2) finds FOO
missing, and bails out.
We could revert r36663, but I further "broke" things with r36680. That
eliminated the set-values to *both* lists. Since one is just a
derivation of the other, the code was implified to only change the
master. Of course, the overall code flow is accidentally relying on
the changes to the trimmed list to stick.
Okay. So. We know what the problem is. Reverting some key revisions
could "fix" things, but really... that is just papering over a core,
underlying problem: entry_modify() made changes in-place that were
*counter* to the definitions of the structures it was modifying. Right
now, entry_modify() no longer does that, which is good, but we need to
find a solution for the code which *happened* to depend on that
behavior. The code does not assume that behavior. -- it just hapened
to work.
The dav tests will continue to have issues for the next day or two
while we figure out a reasonable solution here. I'm hoping that we can
detect step (1) in the log and immediately clear the DELTEED flag to
make way for step (2). We just need to be careful about be too
aggressive on clearing that flag. It needs to happen in *only* the
very specific case where a node is truly being added back.
Cheers,
-g
On Fri, Mar 20, 2009 at 03:40, Greg Stein <gstein_at_gmail.com> wrote:
> The problem is with r36663. I backdated to 36662 and the test passed.
> Failed with 36663. Hellifiknow *why* though.
>
> Gonna investigate around this revision. Weirdness...
>
> On Thu, Mar 19, 2009 at 10:16, Greg Stein <gstein_at_gmail.com> wrote:
>> On Thu, Mar 19, 2009 at 05:30, Hyrum K. Wright
>> <hyrum_wright_at_mail.utexas.edu> wrote:
>>> Greg,
>>> It appears that the currently failing tests on trunk over ra_neon are due to
>>> various issues running the wcprops log. It's the same set of tests which
>>> fails when do_sync is removed from svn_wc__entry_modify(), and I think it
>>> has to do with the fact that read_entries() now returns a unique hash from
>>> prune_deleted().
>>
>> Not entirely surprised :-(
>>
>>> I'm not sure what the Grand Plan for wcprops in wc-ng is. Unfortunately, we
>>
>> My initial thought was to lump them in with the other properties at
>> the wc_db level. libsvn_wc can sort them out on its own (we have
>> functions to classify any given property name).
>>
>>> still need to store them for compat reasons with the older http protocol.
>>> Are they just part of the properties hash in BASE, or should there be
>>> another column for wcprops? (Thankfully, unlike "real" properties, wcprops
>>> only have a BASE version, not WORKING or ACTUAL.)
>>
>> I believe there are WORKING wcprops, too. Or if we don't have the
>> concept in wc-1.0, then we *should* in wc-ng. Consider copying a
>> subtree down from the repository. We may want to cache the version
>> resource URL (I say "may" because I'm not positive without more
>> thinking about when/where wcprops are truly used).
>>
>> If the wcprops are just lumped into the other properties in wc_db,
>> then this poses no additional difficulty.
>>
>>> I'd like to get these wcprops issues ironed our fairly quickly. I think
>>> that we can implement simple wc-ng prop handling for wcprops, switch over to
>>> that, and fix the test failures on trunk. This should be relatively
>>> straight-forward *and* give us some context for use in props.c. Thoughts?
>>
>> I'd suggest going with the "lump in with others, and sort them at a
>> higher level" approach. See how that goes. Does that seem reasonable
>> to you? Alternatives/suggestions?
>>
>> Cheers,
>> -g
>>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1361454
Received on 2009-03-20 06:06:14 CET