(take 2) Re: svn commit: rev 5928 - in trunk/subversion: include libsvn_wc

[Greg Stein <gs...@lyra.org>]

[ looking at some more context... ]

On Tue, May 13, 2003 at 04:59:08PM -0500, sussman@tigris.org wrote:
>...
> +++ trunk/subversion/libsvn_wc/update_editor.c	Tue May 13 16:59:06 2003
>...
> @@ -618,7 +586,31 @@
>  
>    *dir_baton = d = make_dir_baton (NULL, eb, NULL, eb->is_checkout, pool);
>    if (eb->is_checkout)
> -    SVN_ERR (prep_directory (d, eb->ancestor_url, eb->target_revision, pool));
> +    {
> +      SVN_ERR (prep_directory (d, eb->ancestor_url,
> +                               eb->target_revision, pool));
> +    }
> +  else if (! eb->target)
> +    {
> +      /* For an update with a NULL target, this is equivalent to open_dir(): */

Then these two blocks of code should be factored out.

> +      svn_wc_adm_access_t *adm_access;
> +      svn_wc_entry_t tmp_entry;
> +      apr_uint32_t modify_flags = 0;

And after noting the above refactor, I'll also note that my previous comment
about 'modify_flags' applies here, too.

>...
> @@ -763,6 +756,16 @@
>                                                    db->pool);
>        copyfrom_revision = pb->edit_baton->target_revision;      
>  
> +      /* Immediately create an entry for the new directory in the parent.
> +         Note that the parent must already be either added or opened, and
> +         thus it's in an 'incomplete' state just like the new dir.  */      
> +      tmp_entry.kind = svn_node_dir;
> +      tmp_entry.deleted = FALSE;
> +      SVN_ERR (svn_wc__entry_modify (adm_access, db->name, &tmp_entry,
> +                                     (SVN_WC__ENTRY_MODIFY_KIND
> +                                      | SVN_WC__ENTRY_MODIFY_DELETED),
> +                                     TRUE /* immediate write */, pool));
> +      
>        /* Extra check:  a directory by this name may not exist, but there
>           may still be one scheduled for addition.  That's a genuine
>           tree-conflict.  */

The check that follows this comment fetches the entry for db->name.
Specifically, it has a test for whether the entry exists. Given the above,
it definitely will exist. But more importantly, these two entry
manipulations should be folded together.

Specifically, the __entry_modify() is going to read the entries, but then it
will toss them out. But the following code will read the entries *again*.

Hmm. Further: the following check is to look for an obstructed update. I
would think that should happen *before* you go and create a new entry.
Especially because you might have a file sitting there and the above
modification would force that entry to a directory(!).

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org