You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Brian Denny <br...@briandenny.net> on 2003/04/28 00:58:34 UTC
[PATCH] Issue #1075
Okay, here's a patch for #1075, sans regression tests so far, but
enough that some kind soul (Ben? Mike?) could look it over. :)
The meat of the change (as discussed in a previous thread) is in
'recursively_tweak_entries'.
The changes in 'svn_wc_remove_from_revision_control' and
'log_do_delete_entry' address another aspect of this issue which
wasn't discussed in the previous thread -- a different error that
would result if you specified the deleted path explicitly as an
argument to 'svn up'. (See the second of Philip's recipes for this
issue in IZ.)
-brian
Log:
* libsvn_wc/log.c
(log_do_delete_entry): Don't try to recurse on a missing directory;
just delete the entry in the parent directory.
* libsvn_wc/adm_ops.c
(recursively_tweak_entries): Accept a notification function and baton.
Assume that missing directories are due for deletion; remove their
entry in the parent directory, and notify the client of the deletion.
(svn_wc__do_update_cleanup): Accept a notification function and baton.
Pass these to recursively_tweak_entries.
(svn_wc_add): Pass NULL to svn_wc__do_update_cleanup for the
notification function and baton.
(svn_wc_remove_from_revision_control): Don't try to recurse on a
missing directory; just delete the entry in the parent directory.
* libsvn_wc/adm_ops.h
(svn_wc__do_update_cleanup): Accept a notification function and baton.
Comment accordingly.
* libsvn_wc/update_editor.c
(close_edit): Pass our notification function and baton to
svn_wc__do_update_cleanup.
Index: subversion/libsvn_wc/log.c
===================================================================
--- subversion/libsvn_wc/log.c (revision 5680)
+++ subversion/libsvn_wc/log.c (working copy)
@@ -622,11 +622,25 @@
### If they were available, it would be nice to use them. */
if (entry->kind == svn_node_dir)
{
- err = svn_wc_remove_from_revision_control (adm_access,
- SVN_WC_ENTRY_THIS_DIR,
- TRUE,
- NULL, NULL,
- loggy->pool);
+ if (svn_wc__adm_missing (adm_access, full_path))
+ {
+ /* The directory is already missing, so don't try to recurse --
+ just delete the entry in the parent directory. */
+ apr_hash_t *entries;
+ SVN_ERR (svn_wc_entries_read (&entries, loggy->adm_access, FALSE,
+ loggy->pool));
+ svn_wc__entry_remove (entries, name);
+ SVN_ERR (svn_wc__entries_write (entries, loggy->adm_access,
+ loggy->pool));
+ }
+ else
+ {
+ err = svn_wc_remove_from_revision_control (adm_access,
+ SVN_WC_ENTRY_THIS_DIR,
+ TRUE,
+ NULL, NULL,
+ loggy->pool);
+ }
}
else if (entry->kind == svn_node_file)
{
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 5680)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -59,6 +59,8 @@
recursively_tweak_entries (svn_wc_adm_access_t *dirpath,
const char *base_url,
const svn_revnum_t new_rev,
+ svn_wc_notify_func_t notify_func,
+ void *notify_baton,
apr_pool_t *pool)
{
apr_hash_t *entries;
@@ -101,16 +103,38 @@
child_url, new_rev,
svn_wc_adm_access_pool (dirpath)));
- /* If a dir, recurse. */
+ /* If a directory... */
else if (current_entry->kind == svn_node_dir)
{
- svn_wc_adm_access_t *child_access;
const char *child_path
= svn_path_join (svn_wc_adm_access_path (dirpath), name, subpool);
- SVN_ERR (svn_wc_adm_retrieve (&child_access, dirpath, child_path,
- subpool));
- SVN_ERR (recursively_tweak_entries
- (child_access, child_url, new_rev, subpool));
+
+ /* If the directory is 'missing', remove it. This is safe as
+ long as this function is only called as a helper to
+ svn_wc__do_update_cleanup, since the update will already have
+ restored any missing items that it didn't want to delete. */
+ if (svn_wc__adm_missing (dirpath, child_path))
+ {
+ svn_wc__entry_remove (entries, name);
+ if (notify_func)
+ (* notify_func) (notify_baton, child_path,
+ svn_wc_notify_delete,
+ current_entry->kind, NULL,
+ svn_wc_notify_state_unknown,
+ svn_wc_notify_state_unknown,
+ SVN_INVALID_REVNUM);
+ }
+
+ /* Not missing or deleted, so recurse. */
+ else
+ {
+ svn_wc_adm_access_t *child_access;
+ SVN_ERR (svn_wc_adm_retrieve (&child_access, dirpath, child_path,
+ subpool));
+ SVN_ERR (recursively_tweak_entries
+ (child_access, child_url, new_rev,
+ notify_func, notify_baton, subpool));
+ }
}
}
@@ -131,6 +155,8 @@
const svn_boolean_t recursive,
const char *base_url,
const svn_revnum_t new_revision,
+ svn_wc_notify_func_t notify_func,
+ void *notify_baton,
apr_pool_t *pool)
{
apr_hash_t *entries;
@@ -168,7 +194,8 @@
}
else
SVN_ERR (recursively_tweak_entries (dir_access, base_url,
- new_revision, pool));
+ new_revision, notify_func,
+ notify_baton, pool));
}
else
@@ -1055,7 +1082,8 @@
/* Change the entry urls recursively (but not the working rev). */
SVN_ERR (svn_wc__do_update_cleanup (path, adm_access, TRUE, new_url,
- SVN_INVALID_REVNUM, pool));
+ SVN_INVALID_REVNUM, NULL,
+ NULL, pool));
/* Recursively add the 'copied' existence flag as well! */
SVN_ERR (mark_tree (adm_access, SVN_WC__ENTRY_MODIFY_COPIED,
@@ -1750,23 +1778,33 @@
= svn_path_join (svn_wc_adm_access_path(adm_access),
current_entry_name,
subpool);
- SVN_ERR (svn_wc_adm_retrieve (&entry_access, adm_access,
- entrypath, pool));
+ if (svn_wc__adm_missing (adm_access, entrypath))
+ {
+ /* The directory is already missing, so don't try to
+ recurse, just delete the entry in the parent directory. */
+ svn_wc__entry_remove (entries, current_entry_name);
+ }
+ else
+ {
+ SVN_ERR (svn_wc_adm_retrieve (&entry_access, adm_access,
+ entrypath, pool));
- err = svn_wc_remove_from_revision_control (entry_access,
+ err =
+ svn_wc_remove_from_revision_control (entry_access,
SVN_WC_ENTRY_THIS_DIR,
destroy_wf,
cancel_func,
cancel_baton,
subpool);
- if (err && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
- {
- svn_error_clear (err);
- left_something = TRUE;
+ if (err && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
+ {
+ svn_error_clear (err);
+ left_something = TRUE;
+ }
+ else if (err)
+ return err;
}
- else if (err)
- return err;
}
}
Index: subversion/libsvn_wc/adm_ops.h
===================================================================
--- subversion/libsvn_wc/adm_ops.h (revision 5680)
+++ subversion/libsvn_wc/adm_ops.h (working copy)
@@ -49,12 +49,17 @@
working revision (excluding files that are scheduled for addition
or replacement.) Likewise, if BASE_URL is non-null, then rewrite
all urls to be "telescoping" children of the base_url.
+
+ If NOTIFY_FUNC is non-null, invoke it with NOTIFY_BATON for any
+ missing entry deleted during the cleanup.
*/
svn_error_t *svn_wc__do_update_cleanup (const char *path,
svn_wc_adm_access_t *adm_access,
const svn_boolean_t recursive,
const char *base_url,
const svn_revnum_t new_revision,
+ svn_wc_notify_func_t notify_func,
+ void *notify_baton,
apr_pool_t *pool);
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 5680)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -1924,6 +1924,8 @@
eb->recurse,
eb->switch_url,
eb->target_revision,
+ eb->notify_func,
+ eb->notify_baton,
eb->pool));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #1075
Posted by cm...@collab.net.
cmpilato@collab.net writes:
> So I'd suggested two things:
>
> - a flag to this function stating whether or not to do this
> missing-item check, true in all cases except the svn_wc_add call, and
> - a new name for the function altogether that matches its
> functionality in all its use-cases. :-)
Of course, this latter change should probably be its own commit.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #1075
Posted by cm...@collab.net.
Brian Denny <br...@briandenny.net> writes:
> On Mon, Apr 28, 2003 at 10:51:33AM -0500, cmpilato@collab.net wrote:
> >
> > My only concern is how svn_wc__do_update_cleanup() plugs into
> > svn_wc_add(). If I understand the code correctly, your new goodness
> > will get called any time svn_wc_add is called on a directory with
> > copyfrom args. I don't think we want that behavior at all.
>
> am i correct in thinking that the use case that would trigger this
> codepath is if you do an 'svn cp' on a directory that contains a
> missing directory?
I believe that's right.
> what is correct behavior in that case? the missing directory is also
> missing in the copy?
In theory, there's nothing inherently wrong with copying a missing
directory. In practice, it's likely to be buggy (just as, for
example, trying to commit the addition of a new, but missing,
directory). I wouldn't try to address those bugs as part of Issue
#1015, though.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #1075
Posted by Brian Denny <br...@briandenny.net>.
On Mon, Apr 28, 2003 at 10:51:33AM -0500, cmpilato@collab.net wrote:
>
> My only concern is how svn_wc__do_update_cleanup() plugs into
> svn_wc_add(). If I understand the code correctly, your new goodness
> will get called any time svn_wc_add is called on a directory with
> copyfrom args. I don't think we want that behavior at all.
am i correct in thinking that the use case that would trigger this
codepath is if you do an 'svn cp' on a directory that contains a
missing directory?
what is correct behavior in that case? the missing directory is also
missing in the copy?
(the reason i ask is, as it turns out, the above use case errors out
before even getting to the point at which svn_wc__do_update_cleanup
would be called).
-brian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #1075
Posted by Brian Denny <br...@briandenny.net>.
On Mon, Apr 28, 2003 at 10:51:33AM -0500, cmpilato@collab.net wrote:
>
> So I'd suggested two things:
>
> - a flag to this function stating whether or not to do this
> missing-item check, true in all cases except the svn_wc_add call, and
[snip]
Incorporated this suggestion, added a regression test, and committed.
See r5814.
-brian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #1075
Posted by cm...@collab.net.
Brian Denny <br...@briandenny.net> writes:
> Okay, here's a patch for #1075, sans regression tests so far, but
> enough that some kind soul (Ben? Mike?) could look it over. :)
Patch looks pretty good.
My only concern is how svn_wc__do_update_cleanup() plugs into
svn_wc_add(). If I understand the code correctly, your new goodness
will get called any time svn_wc_add is called on a directory with
copyfrom args. I don't think we want that behavior at all.
Of course, that revealed another annoyance, namely that here we have a
function called svn_wc__do_update_cleanup() that's being used for
other purposes than updates.
So I'd suggested two things:
- a flag to this function stating whether or not to do this
missing-item check, true in all cases except the svn_wc_add call, and
- a new name for the function altogether that matches its
functionality in all its use-cases. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #1075
Posted by cm...@collab.net.
Brian Denny <br...@briandenny.net> writes:
> Okay, here's a patch for #1075, sans regression tests so far, but
> enough that some kind soul (Ben? Mike?) could look it over. :)
Reviewing now.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org