You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2011/04/15 00:04:56 UTC

svn commit: r1092530 - in /subversion/trunk/subversion/libsvn_wc: status.c update_editor.c wc.h

Author: rhuijben
Date: Thu Apr 14 22:04:55 2011
New Revision: 1092530

URL: http://svn.apache.org/viewvc?rev=1092530&view=rev
Log:
Replace a (slow) custom status walker in the update editor, with the normal
status walker. The normal status walker is optimized for this scenario and
we can just exit from the status walk whenever we find a modification.

* subversion/libsvn_wc/status.c
  (svn_wc_walk_status): Rename to ...
  (svn_wc__internal_walk_status): ... this.
  (svn_wc_walk_status): New function, wrapping svn_wc__internal_walk_status.

* subversion/libsvn_wc/update_editor.c
  (entry_has_local_mods): Remove function.
  (modcheck_baton_t): Rename variable, to make all defaults 0.
  (modcheck_found_node): Rename to ...
  (modcheck_callback): ... this and implement svn_wc_status_func4_t.
  (tree_has_local_mods): Rename to ...
  (node_has_local_mods): ... this to signal that it works for nodes and trees.
  (check_tree_conflict): Update caller.

* subversion/libsvn_wc/wc.h
  (svn_wc__internal_walk_status): New function.

Modified:
    subversion/trunk/subversion/libsvn_wc/status.c
    subversion/trunk/subversion/libsvn_wc/update_editor.c
    subversion/trunk/subversion/libsvn_wc/wc.h

Modified: subversion/trunk/subversion/libsvn_wc/status.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/status.c?rev=1092530&r1=1092529&r2=1092530&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/status.c (original)
+++ subversion/trunk/subversion/libsvn_wc/status.c Thu Apr 14 22:04:55 2011
@@ -2370,28 +2370,30 @@ svn_wc_get_status_editor5(const svn_delt
 }
 
 svn_error_t *
-svn_wc_walk_status(svn_wc_context_t *wc_ctx,
-                   const char *local_abspath,
-                   svn_depth_t depth,
-                   svn_boolean_t get_all,
-                   svn_boolean_t no_ignore,
-                   svn_boolean_t ignore_text_mods,
-                   const apr_array_header_t *ignore_patterns,
-                   svn_wc_status_func4_t status_func,
-                   void *status_baton,
-                   svn_wc_external_update_t external_func,
-                   void *external_baton,
-                   svn_cancel_func_t cancel_func,
-                   void *cancel_baton,
-                   apr_pool_t *scratch_pool)
+svn_wc__internal_walk_status(svn_wc__db_t *db,
+                             const char *local_abspath,
+                             svn_depth_t depth,
+                             svn_boolean_t get_all,
+                             svn_boolean_t no_ignore,
+                             svn_boolean_t ignore_text_mods,
+                             const apr_array_header_t *ignore_patterns,
+                             svn_wc_status_func4_t status_func,
+                             void *status_baton,
+                             svn_wc_external_update_t external_func,
+                             void *external_baton,
+                             svn_cancel_func_t cancel_func,
+                             void *cancel_baton,
+                             apr_pool_t *scratch_pool)
 {
-  svn_node_kind_t kind;
   struct walk_status_baton wb;
   const svn_io_dirent2_t *dirent;
   const char *anchor_abspath, *target_name;
   svn_boolean_t skip_root;
+  svn_error_t *err;
+  svn_wc__db_status_t status;
+  svn_wc__db_kind_t kind;
 
-  wb.db = wc_ctx->db;
+  wb.db = db;
   wb.target_abspath = local_abspath;
   wb.externals = apr_hash_make(scratch_pool);
   wb.external_func = external_func;
@@ -2409,17 +2411,17 @@ svn_wc_walk_status(svn_wc_context_t *wc_
       ignore_patterns = ignores;
     }
 
-  SVN_ERR(svn_wc_read_kind(&kind, wc_ctx, local_abspath, FALSE, scratch_pool));
+  SVN_ERR(svn_wc__db_read_kind(&kind, db, local_abspath, TRUE, scratch_pool));
   SVN_ERR(svn_io_stat_dirent(&dirent, local_abspath, TRUE,
                              scratch_pool, scratch_pool));
 
-  if (kind == svn_node_file && dirent->kind == svn_node_file)
+  if (kind == svn_wc__db_kind_file && dirent->kind == svn_node_file)
     {
       anchor_abspath = svn_dirent_dirname(local_abspath, scratch_pool);
       target_name = svn_dirent_basename(local_abspath, NULL);
       skip_root = TRUE;
     }
-  else if (kind == svn_node_dir && dirent->kind == svn_node_dir)
+  else if (kind == svn_wc__db_kind_dir && dirent->kind == svn_node_dir)
     {
       anchor_abspath = local_abspath;
       target_name = NULL;
@@ -2450,6 +2452,39 @@ svn_wc_walk_status(svn_wc_context_t *wc_
   return SVN_NO_ERROR;
 }
 
+svn_error_t *
+svn_wc_walk_status(svn_wc_context_t *wc_ctx,
+                   const char *local_abspath,
+                   svn_depth_t depth,
+                   svn_boolean_t get_all,
+                   svn_boolean_t no_ignore,
+                   svn_boolean_t ignore_text_mods,
+                   const apr_array_header_t *ignore_patterns,
+                   svn_wc_status_func4_t status_func,
+                   void *status_baton,
+                   svn_wc_external_update_t external_func,
+                   void *external_baton,
+                   svn_cancel_func_t cancel_func,
+                   void *cancel_baton,
+                   apr_pool_t *scratch_pool)
+{
+  return svn_error_return(
+           svn_wc__internal_walk_status(wc_ctx->db,
+                                        local_abspath,
+                                        depth,
+                                        get_all,
+                                        no_ignore,
+                                        ignore_text_mods,
+                                        ignore_patterns,
+                                        status_func,
+                                        status_baton,
+                                        external_func,
+                                        external_baton,
+                                        cancel_func,
+                                        cancel_baton,
+                                        scratch_pool));
+}
+
 
 svn_error_t *
 svn_wc_status_set_repos_locks(void *edit_baton,

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1092530&r1=1092529&r2=1092530&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Thu Apr 14 22:04:55 2011
@@ -1171,84 +1171,43 @@ open_root(void *edit_baton,
 /* ===================================================================== */
 /* Checking for local modifications. */
 
-/* Set *MODIFIED to true iff the item described by (LOCAL_ABSPATH, KIND)
- * has local modifications. For a file, this means text mods or property mods.
- * For a directory, this means property mods.
- *
- * Use SCRATCH_POOL for temporary allocations.
- */
-static svn_error_t *
-entry_has_local_mods(svn_boolean_t *modified,
-                     svn_wc__db_t *db,
-                     const char *local_abspath,
-                     svn_wc__db_kind_t kind,
-                     apr_pool_t *scratch_pool)
-{
-  svn_boolean_t text_modified;
-  svn_boolean_t props_modified;
-
-  /* Check for text modifications */
-  if (kind == svn_wc__db_kind_file
-      || kind == svn_wc__db_kind_symlink)
-    SVN_ERR(svn_wc__internal_file_modified_p(&text_modified, NULL, NULL,
-                                             db, local_abspath,
-                                             FALSE, TRUE, scratch_pool));
-  else
-    text_modified = FALSE;
-
-  /* Check for property modifications */
-  SVN_ERR(svn_wc__props_modified(&props_modified, db, local_abspath,
-                                 scratch_pool));
-
-  *modified = (text_modified || props_modified);
-
-  return SVN_NO_ERROR;
-}
-
 /* A baton for use with modcheck_found_entry(). */
 typedef struct modcheck_baton_t {
   svn_wc__db_t *db;         /* wc_db to access nodes */
   svn_boolean_t found_mod;  /* whether a modification has been found */
-  svn_boolean_t all_edits_are_deletes;  /* If all the mods found, if any,
-                                          were deletes.  If FOUND_MOD is false
-                                          then this field has no meaning. */
+  svn_boolean_t found_not_delete;  /* Found a not-delete modification */
 } modcheck_baton_t;
 
-/* An implementation of svn_wc__node_found_func_t. */
+/* An implementation of svn_wc_status_func4_t. */
 static svn_error_t *
-modcheck_found_node(const char *local_abspath,
-                    svn_node_kind_t kind,
-                    void *walk_baton,
-                    apr_pool_t *scratch_pool)
-{
-  modcheck_baton_t *baton = walk_baton;
-  svn_wc__db_kind_t db_kind;
-  svn_wc__db_status_t status;
-  svn_boolean_t modified;
+modcheck_callback(void *baton,
+                  const char *local_abspath,
+                  const svn_wc_status3_t *status,
+                  apr_pool_t *scratch_pool)
+{
+  modcheck_baton_t *mb = baton;
+
+  switch (status->node_status)
+    {
+      case svn_wc_status_normal:
+      case svn_wc_status_obstructed:
+      case svn_wc_status_ignored:
+      case svn_wc_status_none:
+      case svn_wc_status_unversioned:
+        break;
 
-  /* ### The walker could in theory pass status and db kind as arguments.
-   * ### So this read_info call is probably redundant. */
-  SVN_ERR(svn_wc__db_read_info(&status, &db_kind, NULL, NULL, NULL, NULL, NULL,
-                               NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                               NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                               NULL, NULL, NULL,
-                               baton->db, local_abspath, scratch_pool,
-                               scratch_pool));
+      case svn_wc_status_deleted:
+        mb->found_mod = TRUE;
+        break;
 
-  if (status != svn_wc__db_status_normal)
-    modified = TRUE;
-  /* No need to check if we already have at least one modification */
-  else if (!baton->found_mod)
-    SVN_ERR(entry_has_local_mods(&modified, baton->db, local_abspath,
-                                 db_kind, scratch_pool));
-  else
-    modified = FALSE;
-
-  if (modified)
-    {
-      baton->found_mod = TRUE;
-      if (status != svn_wc__db_status_deleted)
-        baton->all_edits_are_deletes = FALSE;
+      default:
+      case svn_wc_status_added:
+      case svn_wc_status_replaced:
+      case svn_wc_status_modified:
+        mb->found_mod = TRUE;
+        mb->found_not_delete = TRUE;
+        /* Exit from the status walker: We know what we want to know */
+        return svn_error_create(SVN_ERR_CEASE_INVOCATION, NULL, NULL);
     }
 
   return SVN_NO_ERROR;
@@ -1261,30 +1220,39 @@ modcheck_found_node(const char *local_ab
  * *ALL_EDITS_ARE_DELETES to true, set it to false otherwise.  LOCAL_ABSPATH
  * may be a file or a directory. */
 static svn_error_t *
-tree_has_local_mods(svn_boolean_t *modified,
+node_has_local_mods(svn_boolean_t *modified,
                     svn_boolean_t *all_edits_are_deletes,
                     svn_wc__db_t *db,
                     const char *local_abspath,
                     svn_cancel_func_t cancel_func,
                     void *cancel_baton,
-                    apr_pool_t *pool)
+                    apr_pool_t *scratch_pool)
 {
-  modcheck_baton_t modcheck_baton = { NULL, FALSE, TRUE };
+  modcheck_baton_t modcheck_baton = { NULL, FALSE, FALSE };
+  svn_error_t *err;
 
   modcheck_baton.db = db;
 
-  /* Walk the WC tree to its full depth, looking for any local modifications.
-   * If it's a "sparse" directory, that's OK: there can be no local mods in
-   * the pieces that aren't present in the WC. */
-
-  SVN_ERR(svn_wc__internal_walk_children(db, local_abspath,
-                                         FALSE /* show_hidden */,
-                                         modcheck_found_node, &modcheck_baton,
-                                         svn_depth_infinity, cancel_func,
-                                         cancel_baton, pool));
+  /* Walk the WC tree for status with depth infinity, looking for any local
+   * modifications. If it's a "sparse" directory, that's OK: there can be
+   * no local mods in the pieces that aren't present in the WC. */
+
+  err = svn_wc__internal_walk_status(db, local_abspath,
+                                     svn_depth_infinity,
+                                     FALSE, FALSE, FALSE, NULL,
+                                     modcheck_callback, &modcheck_baton,
+                                     NULL, NULL,
+                                     cancel_func, cancel_baton,
+                                     scratch_pool);
+
+  if (err && err->apr_err == SVN_ERR_CEASE_INVOCATION)
+    svn_error_clear(err);
+  else
+    SVN_ERR(err);
 
   *modified = modcheck_baton.found_mod;
-  *all_edits_are_deletes = modcheck_baton.all_edits_are_deletes;
+  *all_edits_are_deletes = !modcheck_baton.found_not_delete;
+
   return SVN_NO_ERROR;
 }
 
@@ -1596,32 +1564,16 @@ check_tree_conflict(svn_wc_conflict_desc
 
         /* Check if the update wants to delete or replace a locally
          * modified node. */
-        switch (working_kind)
-          {
-            case svn_wc__db_kind_file:
-            case svn_wc__db_kind_symlink:
-              all_mods_are_deletes = FALSE;
-              SVN_ERR(entry_has_local_mods(&modified, eb->db, local_abspath,
-                                           working_kind, pool));
-              break;
 
-            case svn_wc__db_kind_dir:
-              /* We must detect deep modifications in a directory tree,
-               * but the update editor will not visit the subdirectories
-               * of a directory that it wants to delete.  Therefore, we
-               * need to start a separate crawl here. */
-              SVN_ERR(tree_has_local_mods(&modified, &all_mods_are_deletes,
-                                          eb->db, local_abspath,
-                                          eb->cancel_func, eb->cancel_baton,
-                                          pool));
-              break;
 
-            default:
-              /* It's supposed to be in 'normal' status. So how can it be
-               * neither file nor folder? */
-              SVN_ERR_MALFUNCTION();
-              break;
-          }
+        /* Do a deep tree detection of local changes. The update editor will
+         * not visit the subdirectories of a directory that it wants to delete.
+         * Therefore, we need to start a separate crawl here. */
+              
+        SVN_ERR(node_has_local_mods(&modified, &all_mods_are_deletes,
+                                    eb->db, local_abspath,
+                                    eb->cancel_func, eb->cancel_baton,
+                                    pool));
 
         if (modified)
           {

Modified: subversion/trunk/subversion/libsvn_wc/wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc.h?rev=1092530&r1=1092529&r2=1092530&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc.h Thu Apr 14 22:04:55 2011
@@ -567,6 +567,22 @@ svn_wc__internal_changelist_match(svn_wc
                                   apr_hash_t *clhash,
                                   apr_pool_t *scratch_pool);
 
+/* Library-internal version of svn_wc_walk_status(), which see. */
+svn_error_t *
+svn_wc__internal_walk_status(svn_wc__db_t *db,
+                             const char *local_abspath,
+                             svn_depth_t depth,
+                             svn_boolean_t get_all,
+                             svn_boolean_t no_ignore,
+                             svn_boolean_t ignore_text_mods,
+                             const apr_array_header_t *ignore_patterns,
+                             svn_wc_status_func4_t status_func,
+                             void *status_baton,
+                             svn_wc_external_update_t external_func,
+                             void *external_baton,
+                             svn_cancel_func_t cancel_func,
+                             void *cancel_baton,
+                             apr_pool_t *scratch_pool);
 
 /* Library-internal version of svn_wc__node_walk_children(), which see. */
 svn_error_t *



RE: svn commit: r1092530 - in /subversion/trunk/subversion/libsvn_wc: status.c update_editor.c wc.h

Posted by Greg Stein <gs...@gmail.com>.
On Apr 14, 2011 6:35 PM, "Bert Huijben" <be...@qqmail.nl> wrote:
>
>
>
> > -----Original Message-----
> > From: Greg Stein [mailto:gstein@gmail.com]
> > Sent: vrijdag 15 april 2011 0:25
> > To: dev@subversion.apache.org
> > Subject: Re: svn commit: r1092530 - in
> > /subversion/trunk/subversion/libsvn_wc: status.c update_editor.c wc.h
>
> > > -  SVN_ERR(svn_wc__internal_walk_children(db, local_abspath,
> > > -                                         FALSE /* show_hidden */,
> > > -                                         modcheck_found_node,
> &modcheck_baton,
> > > -                                         svn_depth_infinity,
> cancel_func,
> > > -                                         cancel_baton, pool));
> > > +  /* Walk the WC tree for status with depth infinity, looking for any
> local
> > > +   * modifications. If it's a "sparse" directory, that's OK: there
can
> be
> > > +   * no local mods in the pieces that aren't present in the WC. */
> > > +
> > > +  err = svn_wc__internal_walk_status(db, local_abspath,
> >
> > How can this work for a file? It looks like walk_status will fail for
> > non-directory nodes. It calls get_dir_status() which then tries to
> > read children info and fetch dirents.
>
> Walk status properly detects this case and has no problem with walking
only
> a single file in a directory. This is essentially what libsvn_client does
> for its status walk when it is not looking at a repository.
> (This is managed by the 'selected' argument of get_dir_status)

Ah! I see now. It pops up to the containing directory and picks out that
one.

Thanks,
-g

RE: svn commit: r1092530 - in /subversion/trunk/subversion/libsvn_wc: status.c update_editor.c wc.h

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: vrijdag 15 april 2011 0:25
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1092530 - in
> /subversion/trunk/subversion/libsvn_wc: status.c update_editor.c wc.h

> > -  SVN_ERR(svn_wc__internal_walk_children(db, local_abspath,
> > -                                         FALSE /* show_hidden */,
> > -                                         modcheck_found_node,
&modcheck_baton,
> > -                                         svn_depth_infinity,
cancel_func,
> > -                                         cancel_baton, pool));
> > +  /* Walk the WC tree for status with depth infinity, looking for any
local
> > +   * modifications. If it's a "sparse" directory, that's OK: there can
be
> > +   * no local mods in the pieces that aren't present in the WC. */
> > +
> > +  err = svn_wc__internal_walk_status(db, local_abspath,
> 
> How can this work for a file? It looks like walk_status will fail for
> non-directory nodes. It calls get_dir_status() which then tries to
> read children info and fetch dirents.

Walk status properly detects this case and has no problem with walking only
a single file in a directory. This is essentially what libsvn_client does
for its status walk when it is not looking at a repository.
(This is managed by the 'selected' argument of get_dir_status)

	Bert


Re: svn commit: r1092530 - in /subversion/trunk/subversion/libsvn_wc: status.c update_editor.c wc.h

Posted by Greg Stein <gs...@gmail.com>.
On Thu, Apr 14, 2011 at 18:04,  <rh...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/status.c Thu Apr 14 22:04:55 2011
> @@ -2370,28 +2370,30 @@ svn_wc_get_status_editor5(const svn_delt
>...
> +svn_wc__internal_walk_status(svn_wc__db_t *db,
> +                             const char *local_abspath,
> +                             svn_depth_t depth,
> +                             svn_boolean_t get_all,
> +                             svn_boolean_t no_ignore,
> +                             svn_boolean_t ignore_text_mods,
> +                             const apr_array_header_t *ignore_patterns,
> +                             svn_wc_status_func4_t status_func,
> +                             void *status_baton,
> +                             svn_wc_external_update_t external_func,
> +                             void *external_baton,
> +                             svn_cancel_func_t cancel_func,
> +                             void *cancel_baton,
> +                             apr_pool_t *scratch_pool)
>  {
> -  svn_node_kind_t kind;
>   struct walk_status_baton wb;
>   const svn_io_dirent2_t *dirent;
>   const char *anchor_abspath, *target_name;
>   svn_boolean_t skip_root;
> +  svn_error_t *err;
> +  svn_wc__db_status_t status;
> +  svn_wc__db_kind_t kind;

status is not used.

>...
> +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Thu Apr 14 22:04:55 2011
>...
> @@ -1261,30 +1220,39 @@ modcheck_found_node(const char *local_ab
>  * *ALL_EDITS_ARE_DELETES to true, set it to false otherwise.  LOCAL_ABSPATH
>  * may be a file or a directory. */
>  static svn_error_t *
> -tree_has_local_mods(svn_boolean_t *modified,
> +node_has_local_mods(svn_boolean_t *modified,
>                     svn_boolean_t *all_edits_are_deletes,
>                     svn_wc__db_t *db,
>                     const char *local_abspath,
>                     svn_cancel_func_t cancel_func,
>                     void *cancel_baton,
> -                    apr_pool_t *pool)
> +                    apr_pool_t *scratch_pool)
>  {
> -  modcheck_baton_t modcheck_baton = { NULL, FALSE, TRUE };
> +  modcheck_baton_t modcheck_baton = { NULL, FALSE, FALSE };
> +  svn_error_t *err;
>
>   modcheck_baton.db = db;
>
> -  /* Walk the WC tree to its full depth, looking for any local modifications.
> -   * If it's a "sparse" directory, that's OK: there can be no local mods in
> -   * the pieces that aren't present in the WC. */
> -
> -  SVN_ERR(svn_wc__internal_walk_children(db, local_abspath,
> -                                         FALSE /* show_hidden */,
> -                                         modcheck_found_node, &modcheck_baton,
> -                                         svn_depth_infinity, cancel_func,
> -                                         cancel_baton, pool));
> +  /* Walk the WC tree for status with depth infinity, looking for any local
> +   * modifications. If it's a "sparse" directory, that's OK: there can be
> +   * no local mods in the pieces that aren't present in the WC. */
> +
> +  err = svn_wc__internal_walk_status(db, local_abspath,

How can this work for a file? It looks like walk_status will fail for
non-directory nodes. It calls get_dir_status() which then tries to
read children info and fetch dirents.

>...
> @@ -1596,32 +1564,16 @@ check_tree_conflict(svn_wc_conflict_desc
>
>         /* Check if the update wants to delete or replace a locally
>          * modified node. */
> -        switch (working_kind)
> -          {
> -            case svn_wc__db_kind_file:
> -            case svn_wc__db_kind_symlink:

Seems that something like this needs to stick around.

If not, then note that the 'working_kind' parameter is no longer used.

>...

Cheers,
-g