[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] making depth upgrades work (sparse-directories), v2

From: Karl Fogel <kfogel_at_red-bean.com>
Date: 2007-05-20 11:26:57 CEST

In a sense, this is a step down from the previous patch I posted,
because this breaks a lot more tests (clearly there's some major flaw
in it). But in a larger sense it's progress, because it fixes more
places where requested-depth is confused with reported-depth, and
finds more of the important depth-sensitive spots in reporter.c.

Obviously, this is not to be applied. I'm posting it because I don't
want to be off in a corner coding for days without review (not just
for coding errors, but for general approach).

[[[
Start work on the problem that 'svn up --depth=files' in a depth-empty
working copy does not bring in the expected files. (These changes
will probably help all the "request a deeper depth" cases, but right
now I'm only testing with the one mentioned above.)

               ### DO NOT COMMIT, BREAKS TESTS ###

* subversion/libsvn_wc/adm_crawler.c
  (svn_wc_crawl_revisions3): Report svn_depth_infinity for paths where
    it doesn't matter; report entry->depth for paths where it does
    matter. Remove obsolete "TODO(sd)" comments; add some regular
    comments.
  (report_revisions_and_depths): Document depth behavior. When parent
    is depth-empty, report files explicitly. Recurse correctly based
    on depth. Remove obsolete "TODO(sd)" comments.

* subversion/libsvn_repos/reporter.c
  (delta_dirs): Skip paths when a comparison of parent and child
    depths indicates to.
  (update_entry): Adjust depth before updating an entry.

* subversion/libsvn_client/update.c
  (svn_client__update_internal): Stop doing bone divination, as it
    confused requested depth with ambient depth.
]]]

Index: subversion/libsvn_wc/adm_crawler.c
===================================================================
--- subversion/libsvn_wc/adm_crawler.c (revision 25073)
+++ subversion/libsvn_wc/adm_crawler.c (working copy)
@@ -137,6 +137,24 @@
    Alternatively, if REPORT_EVERYTHING is set, then report all
    children unconditionally.
 
+ DEPTH is actually the *requested* depth for the update-like
+ operation for which we are reporting working copy state. However,
+ certain requested depths affect the depth of the report crawl. For
+ example, if the requested depth is svn_depth_empty, there's no
+ point descending into subdirs, no matter what their depths. So:
+
+ If DEPTH is svn_depth_empty, don't report any files and don't
+ descend into any subdirs. If svn_depth_files, report files but
+ still don't descend into subdirs. If svn_depth_immediates, report
+ files, and report subdirs themselves but not their entries. If
+ svn_depth_infinity or svn_depth_unknown, report everything all the
+ way down. (That last sentence might sound counterintuitive, but
+ since you can't go deeper than the local ambient depth anyway,
+ requesting svn_depth_infinity really means "as deep as the various
+ parts of this working copy go". Of course, the information that
+ comes back from the server will be different for svn_depth_unknown
+ than for svn_depth_infinity.)
+
    If TRAVERSAL_INFO is non-null, record this directory's
    value of svn:externals in both TRAVERSAL_INFO->externals_old and
    TRAVERSAL_INFO->externals_new, using wc_path + dir_path as the key,
@@ -323,11 +341,11 @@
                                         FALSE,
                                         current_entry->lock_token,
                                         iterpool));
- /* ... or perhaps just a differing revision or depth or
- lock token. */
- else if (current_entry->revision != dir_rev
- || current_entry->depth != depth
- || current_entry->lock_token)
+ /* ... or perhaps just a differing revision or lock token,
+ or the mere presence of the file in a depth-empty dir. */
+ else if (current_entry->revision != dir_rev
+ || current_entry->lock_token
+ || dot_entry->depth == svn_depth_empty)
             SVN_ERR(reporter->set_path(report_baton,
                                        this_path,
                                        current_entry->revision,
@@ -339,15 +357,10 @@
       
       /*** Directories (in recursive mode) ***/
       else if (current_entry->kind == svn_node_dir
- && depth == svn_depth_infinity)
+ && (depth == svn_depth_immediates
+ || depth == svn_depth_infinity
+ || depth == svn_depth_unknown))
         {
- /* ### TODO(sd): I think it's correct to check whether
- ### 'depth == svn_depth_infinity' above. If the
- ### specified depth is not infinity, then we don't want
- ### to recurse at all. If it is, then we want recursion
- ### to be dependent on the subdirs' entries, right?
- ### That's what the code below does. */
-
           svn_wc_adm_access_t *subdir_access;
           const svn_wc_entry_t *subdir_entry;
 
@@ -413,18 +426,25 @@
                                        subdir_entry->lock_token,
                                        iterpool));
 
- /* ### TODO(sd): See TODO comment in svn_wc_crawl_revisions3()
- ### about depth treatment here. */
- if (depth == svn_depth_infinity)
- SVN_ERR(report_revisions_and_depths(adm_access, this_path,
- subdir_entry->revision,
- reporter, report_baton,
- notify_func, notify_baton,
- restore_files, depth,
- subdir_entry->incomplete,
- use_commit_times,
- traversal_info,
- iterpool));
+ /* Recurse only if requested depth could require it. */
+ if (depth != svn_depth_empty && depth != svn_depth_files)
+ {
+ svn_depth_t depth_beneath_here = depth;
+
+ if (depth == svn_depth_immediates)
+ depth_beneath_here = svn_depth_empty;
+
+ SVN_ERR(report_revisions_and_depths(adm_access, this_path,
+ subdir_entry->revision,
+ reporter, report_baton,
+ notify_func, notify_baton,
+ restore_files,
+ depth_beneath_here,
+ subdir_entry->incomplete,
+ use_commit_times,
+ traversal_info,
+ iterpool));
+ }
         } /* end directory case */
     } /* end main entries loop */
 
@@ -474,9 +494,9 @@
                                       adm_access, FALSE, pool));
 
       base_rev = parent_entry->revision;
- if (depth == svn_depth_unknown)
- depth = parent_entry->depth;
- SVN_ERR(reporter->set_path(report_baton, "", base_rev, depth,
+ /* Just use svn_depth_infinity here, because it doesn't matter. */
+ SVN_ERR(reporter->set_path(report_baton, "", base_rev,
+ svn_depth_infinity,
                                  entry ? entry->incomplete : TRUE,
                                  NULL, pool));
       SVN_ERR(reporter->delete_path(report_baton, "", pool));
@@ -497,13 +517,10 @@
       base_rev = parent_entry->revision;
     }
 
- if (depth == svn_depth_unknown)
- depth = entry->depth;
-
   /* The first call to the reporter merely informs it that the
      top-level directory being updated is at BASE_REV. Its PATH
      argument is ignored. */
- SVN_ERR(reporter->set_path(report_baton, "", base_rev, depth,
+ SVN_ERR(reporter->set_path(report_baton, "", base_rev, entry->depth,
                              entry->incomplete , /* start_empty ? */
                              NULL, pool));
 
@@ -532,14 +549,6 @@
         }
       else
         {
- /* ### TODO(sd): Just passing depth here is not enough. There
- ### can be circumstances where the root is depth 0 or 1,
- ### but some child directories are present at depth
- ### infinity. We need to detect them and recurse into
- ### them *unless* there is a passed-in depth that is not
- ### infinity. I think.
- */
-
           /* Recursively crawl ROOT_DIRECTORY and report differing
              revisions. */
           err = report_revisions_and_depths(adm_access,
Index: subversion/libsvn_client/update.c
===================================================================
--- subversion/libsvn_client/update.c (revision 25073)
+++ subversion/libsvn_client/update.c (working copy)
@@ -60,7 +60,6 @@
   void *report_baton;
   const svn_wc_entry_t *entry;
   const char *anchor, *target;
- svn_node_kind_t kind;
   const char *repos_root;
   svn_error_t *err;
   svn_revnum_t revnum;
@@ -119,42 +118,6 @@
                              _("Entry '%s' has no URL"),
                              svn_path_local_style(anchor, pool));
 
- /* Figure out the real depth for the update, using sophisticated
- techniques from http://en.wikipedia.org/wiki/Bone_divination. */
- if (depth == svn_depth_unknown)
- {
- SVN_ERR(svn_io_check_path(path, &kind, pool));
- if (kind == svn_node_none || kind == svn_node_file)
- depth = svn_depth_infinity;
- else if (kind == svn_node_dir)
- {
- if (adm_access == dir_access)
- depth = entry->depth;
- else
- {
- /* I'm not sure this situation can ever happen in real
- life, but svn_wc_adm_open_anchor() doesn't explicitly
- prohibit it, so we must be prepared. */
- const svn_wc_entry_t *tmp_ent;
- SVN_ERR(svn_wc_entry(&tmp_ent, path, dir_access, FALSE, pool));
- if (tmp_ent)
- depth = tmp_ent->depth;
- else
- return svn_error_createf
- (SVN_ERR_WC_NOT_DIRECTORY, NULL,
- _("'%s' is not a working copy directory"),
- svn_path_local_style(path, pool));
- }
- }
- else
- {
- return svn_error_createf
- (SVN_ERR_NODE_UNKNOWN_KIND, NULL,
- _("'%s' is neither a file nor a directory"),
- svn_path_local_style(path, pool));
- }
- }
-
   /* Get revnum set to something meaningful, so we can fetch the
      update editor. */
   if (revision->kind == svn_opt_revision_number)
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 25073)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -799,6 +799,19 @@
       if (!name)
         break;
 
+ /* Skip this path and continue looping, if received depth says
+ so and this path's depth doesn't contradict it. */
+ if (depth == svn_depth_empty)
+ {
+ if (info)
+ {
+ if (info->depth <= depth)
+ continue;
+ }
+ else
+ continue;
+ }
+
       if (info && !SVN_IS_VALID_REVNUM(info->rev))
         {
           /* We want to perform deletes before non-replacement adds,
@@ -976,14 +989,24 @@
 
   /* If the anchor is the operand, diff the two directories; otherwise
      update the operand within the anchor directory. */
- if (!*b->s_operand)
- SVN_ERR(delta_dirs(b, s_rev, s_fullpath, b->t_path, root_baton,
- "", info->start_empty, info->depth, pool));
- else
- SVN_ERR(update_entry(b, s_rev, s_fullpath, s_entry, b->t_path,
- t_entry, root_baton, b->s_operand, info,
- info->depth, pool));
+ {
+ svn_depth_t depth_from_here;
 
+ if (b->requested_depth != svn_depth_unknown
+ && b->requested_depth < info->depth)
+ depth_from_here = b->requested_depth;
+ else
+ depth_from_here = info->depth;
+
+ if (!*b->s_operand)
+ SVN_ERR(delta_dirs(b, s_rev, s_fullpath, b->t_path, root_baton,
+ "", info->start_empty, depth_from_here, pool));
+ else
+ SVN_ERR(update_entry(b, s_rev, s_fullpath, s_entry, b->t_path,
+ t_entry, root_baton, b->s_operand, info,
+ depth_from_here, pool));
+ }
+
   SVN_ERR(b->editor->close_directory(root_baton, pool));
   SVN_ERR(b->editor->close_edit(b->edit_baton, pool));
   return SVN_NO_ERROR;

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun May 20 11:27:14 2007

This is an archived mail posted to the Subversion Dev mailing list.