that harvest_committables() patch

Hi Philip,

it took me quite a while and a lot of poking Greg to figure out the error.
I'm at the point where merge_tests.py 34 passes :D and I'd run a complete
make check. Only I will go to bed now and see the results tomorrow.

If you're at your keyboard earlier than me and if you like, take attached
version of the patch and make check it yourself. Maybe it should be split
into two commits...

(and make sure to send me a mail if/when you start taking over, I'll do the
same.)

Heh, grokking the really tricky ones is quite rewarding, thanks for letting
me have it :)
~Neels

I hope the attachment comes through, else find it at
http://kleinekatze.de/entry_t.20100501-0200

* subversion/libsvn_client/commit_util.c
  (harvest_committables, svn_client__harvest_committables):
    Use wc-ng to determine added state instead of entry_t.

* subversion/libsvn_wc/node.c
  (svn_wc__node_get_copyfrom_info):
    Model IS_COPY_TARGET the way wc-1 returned entry->copyfrom_url. Current
    code still expects mixed-revision copies to appear as one copy with a
    single root (see comment).

Thanks to: gstein

--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/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c (revision 939737)
+++ subversion/libsvn_client/commit_util.c (working copy)
@@ -378,10 +378,10 @@ harvest_committables(apr_hash_t *committ
   svn_boolean_t text_mod = FALSE, prop_mod = FALSE;
   apr_byte_t state_flags = 0;
   svn_node_kind_t working_kind, db_kind;
- const char *entry_url, *entry_lock_token, *cf_url = NULL;
- svn_revnum_t cf_rev = entry->copyfrom_rev;
+ const char *entry_url, *entry_lock_token, *entry_copyfrom_url, *cf_url = NULL;
+ svn_revnum_t entry_copyfrom_rev, cf_rev = SVN_INVALID_REVNUM;
   const svn_string_t *propval;
- svn_boolean_t is_special, is_file_external;
+ svn_boolean_t is_special, is_file_external, is_added, is_copy_target;
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
 
@@ -521,18 +521,26 @@ harvest_committables(apr_hash_t *committ
   /* Check for the trivial addition case. Adds can be explicit
      (schedule == add) or implicit (schedule == replace ::= delete+add).
      We also note whether or not this is an add with history here. */
- if ((entry->schedule == svn_wc_schedule_add)
- || (entry->schedule == svn_wc_schedule_replace))
- {
- state_flags |= SVN_CLIENT_COMMIT_ITEM_ADD;
- if (entry->copyfrom_url)
+ SVN_ERR(svn_wc__node_is_added(&is_added, ctx->wc_ctx, local_abspath,
+ scratch_pool));
+ if (is_added)
+ {
+ SVN_ERR(svn_wc__node_get_copyfrom_info(&entry_copyfrom_url,
+ &entry_copyfrom_rev,
+ &is_copy_target,
+ ctx->wc_ctx, local_abspath,
+ scratch_pool, scratch_pool));
+ if (is_copy_target)
         {
+ state_flags |= SVN_CLIENT_COMMIT_ITEM_ADD;
           state_flags |= SVN_CLIENT_COMMIT_ITEM_IS_COPY;
- cf_url = entry->copyfrom_url;
+ cf_url = entry_copyfrom_url;
+ cf_rev = entry_copyfrom_rev;
           adds_only = FALSE;
         }
- else
+ else if (!entry_copyfrom_url)
         {
+ state_flags |= SVN_CLIENT_COMMIT_ITEM_ADD;
           adds_only = TRUE;
         }
     }
@@ -989,6 +997,7 @@ svn_client__harvest_committables(apr_has
     {
       const svn_wc_entry_t *entry;
       const char *target_abspath;
+ svn_boolean_t is_added;
       svn_error_t *err;
 
       svn_pool_clear(iterpool);
@@ -1030,13 +1039,12 @@ svn_client__harvest_committables(apr_has
 
       /* We have to be especially careful around entries scheduled for
          addition or replacement. */
- if ((entry->schedule == svn_wc_schedule_add)
- || (entry->schedule == svn_wc_schedule_replace))
+ SVN_ERR(svn_wc__node_is_added(&is_added, ctx->wc_ctx, target_abspath,
+ iterpool));
+ if (is_added)
         {
           const char *parent_abspath = svn_dirent_dirname(target_abspath,
                                                           iterpool);
- svn_boolean_t is_added;
-
           err = svn_wc__node_is_added(&is_added, ctx->wc_ctx, parent_abspath,
                                       iterpool);
           if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
Index: subversion/libsvn_wc/node.c
===================================================================
--- subversion/libsvn_wc/node.c (revision 939737)
+++ subversion/libsvn_wc/node.c (working copy)
@@ -414,8 +414,51 @@ svn_wc__node_get_copyfrom_info(const cha
     {
       /* If this was the root of the copy then the URL is immediately
          available... */
+
       if (is_copy_target)
- *is_copy_target = TRUE;
+ {
+ /* ### At this point we'd just set is_copy_target to TRUE, *but* we
+ * currently want to model wc-1 behaviour. Particularly, this
+ * affects mixed-revision copies (e.g. wc-wc copy):
+ * - Wc-1 saw only the root of a mixed-revision copy as the copy's
+ * root.
+ * - Wc-ng returns an explicit original_root_url,
+ * original_repos_relpath pair for each subtree with mismatching
+ * revision.
+ * We need to compensate for that: Find out if the parent of
+ * this node is also copied and has a matching copy_from URL. If so,
+ * nevermind the revision, just like wc-1 did, and say this was not
+ * a separate copy target. */
+ const char *parent_abspath;
+ const char *base_name;
+ const char *parent_original_repos_relpath;
+ const char *parent_original_root_url;
+
+ svn_dirent_split(local_abspath, &parent_abspath, &base_name,
+ scratch_pool);
+
+ /* This is a copied node, so we should never fall off the top of a
+ * working copy here. */
+ SVN_ERR(svn_wc__db_read_info(NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL,
+ &parent_original_repos_relpath,
+ &parent_original_root_url, NULL, NULL,
+ NULL, NULL, NULL, NULL, NULL, db,
+ parent_abspath, scratch_pool,
+ scratch_pool));
+
+ /* So, count this as a separate copy target only if the URLs
+ * don't match up, or if the parent isn't copied at all. */
+ if (parent_original_root_url == NULL
+ || parent_original_repos_relpath == NULL
+ || strcmp(original_root_url, parent_original_root_url) != 0
+ || strcmp(svn_relpath_join(parent_original_repos_relpath,
+ base_name, scratch_pool),
+ original_repos_relpath) != 0)
+ *is_copy_target = TRUE;
+ }
+
       if (copyfrom_url)
         *copyfrom_url = svn_path_url_add_component2(original_root_url,
                                                     original_repos_relpath,