Re: Why does "update" treat an obstruction differently if adding without history?

Quoting Greg Stein <gstein_at_gmail.com>:

> On Wed, Oct 29, 2008 at 8:03 AM, Stephen Butler <sbutler_at_elego.de> wrote:
>> ...
>> I'm hacking away at the skipping problem, too. For the allow-existing-add
>> feature, I simply gave check_tree_conflicts() a ptr-to-bool arg. Here's
>> the signature for that function in my incomplete patch:
>
> How big and ugly is that function? update_editor.c has a ton of these
> huge nasty functions. The code complexity is really bad.

Maybe I've been working in the update-editor mine too long and lost
perspective...

>
> I'm assuming this is similar, and so in that vein:
>
>> /* First, check whether a significant conflict has already been raised
>> * on FULL_PATH. For any incoming change ACTION, check for a tree
>> * conflict. When adding an item, text and prop conflicts may also be
>> * checked. If an existing conflict is found, return NULL in
>> * *PCONFLICT and set *SKIP_P to TRUE.
>
> Can this first check be broken out into a function, say,
> covered_by_existing_conflict(...) ?

Yes. Actually, it already is. Doh!

>
>> *
>> * Second, check whether the incoming change ACTION on FULL_PATH would
>> * conflict with FULL_PATH's scheduled change. If so, then FULL_PATH
>> * is a new tree conflict victim. Raise a persistent tree conflict by
>> * appending log actions to LOG_ACCUM. Return a conflict description
>> * in *PCONFLICT and set *SKIP_P to TRUE.
>
> Rather than taking LOG_ACCUM, can this function instead rely on the
> caller to perform log actions. IOW, only *check* for a conflict. Don't
> take any actions on it.

We could pull out the code that writes the tree conflict data to the
parent dir's entry. But if we find a new tree conflict, we always
write it.

On a related note, we expect the caller to flush and run the log
(for update and switch).

>
>> *
>> * If no conflicts are found, return NULL in *PCONFLICT and set
>> * *SKIP_P to FALSE.
>> *
>> * When adding a file or directory, the KIND is the kind of the item
>> * to be added, and *ADD_EXISTED_P will be set to TRUE if the item is
>> * already scheduled for add without history (which is not a
>> * conflict). For other actions, KIND and ADD_EXISTED_P are ignored.
>
> Can this be left to the caller? This doesn't seem to be related to
> tree conflicts.

Actually it is. It's a special case where Subversion automatically
resolves a potential tree conflict without raising it.

>
>> *
>> * The edit baton EB gives information including whether the operation is
>> * an update or a switch.
>
> How many pieces of EB are accessed? Having "the whole EB" can give
> this function ugly powers. If you only need one or two parameters,
> then I'd suggest passing those directly.

Let's see, from the edit baton we need these members for the tree
conflict check:

   allow_unver_obstructions
   cancel_func
   cancel_baton

And one more for the tree conflict persistence:

   switch_url (just a boolean if-null check)

Not so many after all.

>
>> *
>> * PARENT_ADM_ACCESS is the admin access baton of FULL_PATH's parent
>> * directory.
>
> Parent? Why not pass the corresponding access baton, and if/when this
> function needs it, then it can retrieve it itself. Maybe this is
> nomenclature. Consider:
>
> If kind==file, then is this baton, the directory that the file resides
> in? Or the parent of *that* directory?
>
> If kind==dir, then is this baton related to self, or is this the
> parent of the directory?
>
> Please clarify.

It's always the parent dir of the potential tree conflict victim.

Should each function retrieve this dir's access baton on its own?
Or is it better to pass around the same baton, which the caller
can use later (if there's no conflict and the update proceeds)?

Thanks for the feedback.
Steve

-- 
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org