more harvest_committables() -- was: that harvest_committables() patch

I have this patch in progress which tries to get rid of one {entry->deleted
|| (entry->schedule one of _delete, _replace)} use in harvest_committables().

It seems to pass now (complete make check still running, will verify
tomorrow), but it definitely needs polish. Even if this patch passes make
check it's a sad twisty bastard.

Thoughts welcome!

~Neels

* subversion/include/private/svn_wc_private.h
  (svn_wc__node_is_status_deleted):

* subversion/libsvn_client/commit_util.c
  (harvest_committables):

* subversion/libsvn_wc/node.c
  (svn_wc__node_is_replaced):

* subversion/tests/cmdline/copy_tests.py
  (mixed_wc_to_url):

--This line, and those below, will be ignored--
conf: # dub:/home/neels/pat/.pat-base/config-default
conf: http://archive.apache.org/dist/apr/apr-1.3.9.tar.bz2
conf: http://archive.apache.org/dist/apr/apr-util-1.3.9.tar.bz2
conf: http://www.sqlite.org/sqlite-3.6.22.tar.gz
conf: http://www.webdav.org/neon/neon-0.29.3.tar.gz
conf: fsfs
conf: local
Index: subversion/include/private/svn_wc_private.h
===================================================================
--- subversion/include/private/svn_wc_private.h (revision 940121)
+++ subversion/include/private/svn_wc_private.h (working copy)
@@ -414,6 +414,19 @@ svn_wc__node_is_status_deleted(svn_boole
                                apr_pool_t *scratch_pool);
 
 /**
+ * Set @a *is_obstructed_delete to TRUE if @a local_abspath is
+ * "obstructed_deleted", using @a wc_ctx. If @a local_abspath is not in the
+ * working copy, return @c SVN_ERR_WC_PATH_NOT_FOUND. Use @a scratch_pool for
+ * all temporary allocations.
+ */
+svn_error_t *
+svn_wc__node_is_status_deleted_or_obstructed_del(
+ svn_boolean_t *is_obstructed_delete,
+ svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool);
+
+/**
  * Set @a *is_obstructed to whether @a local_abspath is obstructed, using
  * @a wc_ctx. If @a local_abspath is not in the working copy, return
  * @c SVN_ERR_WC_PATH_NOT_FOUND. Use @a scratch_pool for all temporary
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c (revision 940121)
+++ subversion/libsvn_client/commit_util.c (working copy)
@@ -493,6 +493,11 @@ harvest_committables(apr_hash_t *committ
 
   /* If the entry is deleted with "--keep-local", we can skip checking
      for conflicts inside. */
+ /* ### Since wc-ng won't store whether a node was deleted with --keep-local
+ ### or not, a 'svn delete --keep-local' should instead clear the
+ ### tree-conflicts found in the targets. This code here should simply
+ ### always check for tree-conflicts and assume ignorable conflicts to be
+ ### gone already. */
   if (entry->keep_local)
     SVN_ERR_ASSERT(entry->schedule == svn_wc_schedule_delete);
   else
@@ -519,12 +524,23 @@ harvest_committables(apr_hash_t *committ
      - The entry is scheduled for deletion or replacement, which case
        we need to send a delete either way.
   */
- if ((! adds_only)
- && ((entry->deleted && entry->schedule == svn_wc_schedule_normal)
- || (entry->schedule == svn_wc_schedule_delete)
- || (entry->schedule == svn_wc_schedule_replace)))
+ if (! adds_only)
     {
- state_flags |= SVN_CLIENT_COMMIT_ITEM_DELETE;
+ svn_boolean_t is_replaced;
+ svn_boolean_t is_status_deleted;
+ svn_boolean_t is_present;
+
+ /* ### There is room for optimization here, but let's keep it plain
+ * while this function is in flux. */
+ SVN_ERR(svn_wc__node_is_status_deleted_or_obstructed_del(
+ &is_status_deleted, ctx->wc_ctx, local_abspath,
+ scratch_pool));
+ SVN_ERR(svn_wc__node_is_replaced(&is_replaced, ctx->wc_ctx,
+ local_abspath, scratch_pool));
+ SVN_ERR(svn_wc__node_is_status_present(&is_present, ctx->wc_ctx,
+ local_abspath, scratch_pool));
+ if (is_status_deleted || is_replaced || ! is_present)
+ state_flags |= SVN_CLIENT_COMMIT_ITEM_DELETE;
     }
 
   /* Check for the trivial addition case. Adds can be explicit
Index: subversion/libsvn_wc/node.c
===================================================================
--- subversion/libsvn_wc/node.c (revision 940121)
+++ subversion/libsvn_wc/node.c (working copy)
@@ -675,6 +675,28 @@ svn_wc__node_is_status_deleted(svn_boole
 }
 
 svn_error_t *
+svn_wc__node_is_status_deleted_or_obstructed_del(svn_boolean_t *is_deleted,
+ svn_wc_context_t *wc_ctx,
+ const char *local_abspath,
+ apr_pool_t *scratch_pool)
+{
+ svn_wc__db_status_t status;
+
+ SVN_ERR(svn_wc__db_read_info(&status,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL,
+ wc_ctx->db, local_abspath,
+ scratch_pool, scratch_pool));
+
+ *is_deleted = (status == svn_wc__db_status_deleted) ||
+ (status == svn_wc__db_status_obstructed_delete);
+
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
 svn_wc__node_is_status_obstructed(svn_boolean_t *is_obstructed,
                                   svn_wc_context_t *wc_ctx,
                                   const char *local_abspath,
@@ -803,9 +825,35 @@ svn_wc__node_is_replaced(svn_boolean_t *
                          const char *local_abspath,
                          apr_pool_t *scratch_pool)
 {
- return svn_error_return(svn_wc__internal_is_replaced(replaced, wc_ctx->db,
- local_abspath,
- scratch_pool));
+ SVN_ERR(svn_wc__internal_is_replaced(replaced, wc_ctx->db, local_abspath,
+ scratch_pool));
+
+ if (*replaced)
+ {
+ /* ### Catch a mixed-rev copy that replaces. The mixed-rev children are
+ * each regarded as op-roots of the replace and result in currently
+ * unexpected behaviour. Model wc-1 behaviour via
+ * svn_wc__node_get_copyfrom_info(). */
+ svn_wc__db_status_t status;
+
+ SVN_ERR(svn_wc__db_scan_addition(&status, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL,
+ wc_ctx->db, local_abspath,
+ scratch_pool, scratch_pool));
+
+ if (status == svn_wc__db_status_copied
+ || status == svn_wc__db_status_moved_here)
+ {
+ svn_boolean_t is_copy_target;
+
+ SVN_ERR(svn_wc__node_get_copyfrom_info(NULL, NULL, &is_copy_target,
+ wc_ctx, local_abspath,
+ scratch_pool, scratch_pool));
+ if (! is_copy_target)
+ *replaced = FALSE;
+ }
+ }
+ return SVN_NO_ERROR;
 }
 
 
Index: subversion/tests/cmdline/copy_tests.py
===================================================================
--- subversion/tests/cmdline/copy_tests.py (revision 940121)
+++ subversion/tests/cmdline/copy_tests.py (working copy)
@@ -1764,7 +1764,7 @@ def mixed_wc_to_url(sbox):
                                      'co', Z_url, wc_dir)
 
   if os.path.exists(os.path.join(wc_dir, 'pi')):
- raise svntest.Failure
+ raise svntest.Failure("Path 'pi' exists but should be gone.")
 
   fp = open(os.path.join(wc_dir, 'rho'), 'r')
   found_it = 0
@@ -1772,7 +1772,7 @@ def mixed_wc_to_url(sbox):
     if re.match("^Second modification to rho.", line):
       found_it = 1
   if not found_it:
- raise svntest.Failure
+ raise svntest.Failure("The second modification to rho didn't make it.")
 
 
 #----------------------------------------------------------------------