You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2010/05/12 17:09:57 UTC

svn commit: r943546 - in /subversion/trunk/subversion: libsvn_wc/props.c libsvn_wc/status.c tests/cmdline/prop_tests.py

Author: gstein
Date: Wed May 12 15:09:56 2010
New Revision: 943546

URL: http://svn.apache.org/viewvc?rev=943546&view=rev
Log:
Add some robustness to assemble_status() to prevent it from trying to
fetch metadata/properties from missing/obstructed subdirectories.

Return errors a bit more often in get_pristine_props() for obstructions.

* subversion/libsvn_wc/status.c:
  (assemble_status): also fetch DB_STATUS and BASE_SHADOWED in read_info.
    in the future, we'll use BASE_SHADOWED to avoid entry->schedule. use
    DB_STATUS to determine certain directory states. avoid the bulk of
    work when a directory is incomplete/missing/obstructed. tighten
    declaration of WC_SPECIAL.
  (send_status_structure): assert that ENTRY is not NULL. this assertion,
    plus the behavior in send_unversioned_item clarifies that
    assemble_status can easily be split into two functions.
  (send_unversioned_item): add comment labels to params to assemble_status.
    pass NULL for REPOS_LOCKS since it won't be used anyways.

* subversion/libsvn_wc/props.c:
  (svn_wc__get_pristine_props): return errors for all three types of
    obstructions. when an obstruction happens, we cannot determine whether
    props are supposed to be available or not.

* subversion/tests/cmdline/prop_tests.py:
  (obstructed_subdirs): new test that verifies 'svn status' works for
    obstructed subdirs. recently, the crawler was broken, and
    assemble_status also error'd trying to fetch prop information. this
    test should ensure those issues do not return.

Modified:
    subversion/trunk/subversion/libsvn_wc/props.c
    subversion/trunk/subversion/libsvn_wc/status.c
    subversion/trunk/subversion/tests/cmdline/prop_tests.py

Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=943546&r1=943545&r2=943546&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Wed May 12 15:09:56 2010
@@ -2030,16 +2030,27 @@ svn_wc__get_pristine_props(apr_hash_t **
 #endif
       || status == svn_wc__db_status_excluded
       || status == svn_wc__db_status_absent
-      || status == svn_wc__db_status_not_present
-      || status == svn_wc__db_status_obstructed
-      || status == svn_wc__db_status_obstructed_add)
+      || status == svn_wc__db_status_not_present)
     {
       *props = NULL;
       return SVN_NO_ERROR;
     }
-  if (status == svn_wc__db_status_obstructed_delete)
+
+  /* The node is obstructed:
+
+     - subdir is missing, obstructed by a file, or missing admin area
+     - a file is obstructed by a versioned subdir   (### not reported)
+
+     Thus, properties are not available for this node. Returning NULL
+     would indicate "not defined" for its state. For obstructions, we
+     cannot *determine* whether properties should be here or not.
+
+     ### it would be nice to report an obstruction, rather than simply
+     ### PROPERTY_NOT_FOUND. but this is transitional until single-db.  */
+  if (status == svn_wc__db_status_obstructed_delete
+      || status == svn_wc__db_status_obstructed
+      || status == svn_wc__db_status_obstructed_add)
     return svn_error_createf(SVN_ERR_PROPERTY_NOT_FOUND, NULL,
-                             /* ### temporary until single-db  */
                              U_("Directory '%s' is missing on disk, so the "
                                 "properties are not available."),
                              svn_dirent_local_style(local_abspath,

Modified: subversion/trunk/subversion/libsvn_wc/status.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/status.c?rev=943546&r1=943545&r2=943546&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/status.c (original)
+++ subversion/trunk/subversion/libsvn_wc/status.c Wed May 12 15:09:56 2010
@@ -293,6 +293,7 @@ assemble_status(svn_wc_status3_t **statu
                 apr_pool_t *scratch_pool)
 {
   svn_wc_status3_t *stat;
+  svn_wc__db_status_t db_status;
   svn_wc__db_kind_t db_kind;
   svn_boolean_t locked_p = FALSE;
   svn_boolean_t switched_p = FALSE;
@@ -303,10 +304,8 @@ assemble_status(svn_wc_status3_t **statu
   svn_revnum_t changed_rev;
   const char *changed_author;
   apr_time_t changed_date;
+  svn_boolean_t base_shadowed;
   svn_boolean_t conflicted;
-#ifdef HAVE_SYMLINK
-  svn_boolean_t wc_special;
-#endif /* HAVE_SYMLINK */
 
   /* Defaults for two main variables. */
   enum svn_wc_status_kind final_text_status = svn_wc_status_normal;
@@ -403,28 +402,15 @@ assemble_status(svn_wc_status3_t **statu
       return SVN_NO_ERROR;
     }
 
-  SVN_ERR(svn_wc__db_read_info(NULL, &db_kind, &revision, NULL, NULL, NULL,
+  SVN_ERR(svn_wc__db_read_info(&db_status, &db_kind, &revision,
+                               NULL, NULL, NULL,
                                &changed_rev, &changed_date, &changed_author,
                                NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                               NULL, NULL, NULL, NULL, NULL, &conflicted,
+                               NULL, NULL, NULL, NULL,
+                               &base_shadowed, &conflicted,
                                &lock, db, local_abspath, result_pool,
                                scratch_pool));
 
-  /* Someone either deleted the administrative directory in the versioned
-     subdir, or deleted the directory altogether and created a new one.
-     In any case, what is currently there is in the way.
-   */
-  if (db_kind == svn_wc__db_kind_dir)
-    {
-      if (path_kind == svn_node_dir)
-        {
-          if (svn_wc__adm_missing(db, local_abspath, scratch_pool))
-            final_text_status = svn_wc_status_obstructed;
-        }
-      else if (path_kind != svn_node_none)
-        final_text_status = svn_wc_status_obstructed;
-    }
-
   /** File externals are switched files, but they are not shown as
       such.  To be switched it must have both an URL and a parent with
       an URL, at the very least.  If this is the root folder on the
@@ -446,11 +432,65 @@ assemble_status(svn_wc_status3_t **statu
                           scratch_pool), entry->url) != 0);
     }
 
-  if (final_text_status != svn_wc_status_obstructed)
+  /* Examine whether our directory metadata is present, and compensate
+     if it is missing.
+
+     There are a several kinds of obstruction that we detect here:
+
+     - versioned subdir is missing
+     - the versioned subdir's admin area is missing
+     - the versioned subdir has been replaced with a file/symlink
+
+     Net result: the target is obstructed and the metadata is unavailable.
+
+     Note: wc_db can also detect a versioned file that has been replaced
+     with a versioned subdir (moved from somewhere). We don't look for
+     that right away because the file's metadata is still present, so we
+     can examine properties and conflicts and whatnot.
+
+     ### note that most obstruction concepts disappear in single-db mode
+  */
+  if (db_kind == svn_wc__db_kind_dir)
+    {
+      if (db_status == svn_wc__db_status_incomplete)
+        {
+          /* Highest precedence.  */
+          final_text_status = svn_wc_status_incomplete;
+        }
+      else if (db_status == svn_wc__db_status_obstructed_delete)
+        {
+          /* Deleted directories are never reported as missing.  */
+          if (path_kind == svn_node_none)
+            final_text_status = svn_wc_status_deleted;
+          else
+            final_text_status = svn_wc_status_obstructed;
+        }
+      else if (db_status == svn_wc__db_status_obstructed
+               || db_status == svn_wc__db_status_obstructed_add)
+        {
+          /* A present or added directory should be on disk, so it is
+             reported missing or obstructed.  */
+          if (path_kind == svn_node_none)
+            final_text_status = svn_wc_status_missing;
+          else
+            final_text_status = svn_wc_status_obstructed;
+        }
+    }
+
+  /* If FINAL_TEXT_STATUS is still normal, after the above checks, then
+     we should proceed to refine the status.
+
+     If it was changed, then the subdir is incomplete or missing/obstructed.
+     It means that no further information is available, and we should skip
+     all this work.  */
+  if (final_text_status == svn_wc_status_normal)
     {
       svn_boolean_t has_props;
       svn_boolean_t prop_modified_p = FALSE;
       svn_boolean_t text_modified_p = FALSE;
+#ifdef HAVE_SYMLINK
+      svn_boolean_t wc_special;
+#endif /* HAVE_SYMLINK */
 
       /* Implement predecence rules: */
 
@@ -555,6 +595,9 @@ assemble_status(svn_wc_status3_t **statu
             be in the prop_status field at this point, although they do not
             override a C text status.*/
 
+      /* ### db_status, base_shadowed, and fetching base_status can
+         ### fully replace entry->schedule here.  */
+
       if (entry->schedule == svn_wc_schedule_add
           && final_text_status != svn_wc_status_conflicted)
         {
@@ -689,6 +732,8 @@ send_status_structure(const struct walk_
 {
   svn_wc_status3_t *statstruct;
 
+  SVN_ERR_ASSERT(entry != NULL);
+
   SVN_ERR(assemble_status(&statstruct, wb->db, local_abspath, entry,
                           parent_entry, path_kind, path_special, get_all,
                           is_ignored, wb->repos_locks, wb->repos_root,
@@ -821,9 +866,15 @@ send_unversioned_item(const struct walk_
 
   is_external = is_external_path(wb->externals, local_abspath, pool);
 
-  SVN_ERR(assemble_status(&status, wb->db, local_abspath, NULL, NULL,
-                          path_kind, path_special, FALSE, ignore,
-                          wb->repos_locks, wb->repos_root, pool, pool));
+  SVN_ERR(assemble_status(&status, wb->db, local_abspath,
+                          NULL /* entry */,
+                          NULL /* parent_entry */,
+                          path_kind, path_special,
+                          FALSE /* get_all */,
+                          ignore,
+                          NULL /* repos_locks */,
+                          wb->repos_root,
+                          pool, pool));
 
   if (is_external)
     status->text_status = svn_wc_status_external;

Modified: subversion/trunk/subversion/tests/cmdline/prop_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/prop_tests.py?rev=943546&r1=943545&r2=943546&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/prop_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/prop_tests.py Wed May 12 15:09:56 2010
@@ -1849,6 +1849,51 @@ def prop_reject_grind(sbox):
   ### note that del.add has been erroneously deleted!
 
 
+def obstructed_subdirs(sbox):
+  """test properties of obstructed subdirectories"""
+
+  sbox.build()
+  wc_dir = sbox.wc_dir
+
+  # at one point during development, obstructed subdirectories threw
+  # errors trying to fetch property information during 'svn status'.
+  # this test ensures we won't run into that problem again.
+
+  C_path = sbox.ospath('A/C')
+  sbox.simple_propset('red', 'blue', C_path)
+
+  expected_disk = svntest.main.greek_state.copy()
+  expected_disk.tweak('A/C', props={'red': 'blue'})
+  actual_disk_tree = svntest.tree.build_tree_from_wc(wc_dir, load_props=True)
+  svntest.tree.compare_trees("disk", actual_disk_tree,
+                             expected_disk.old_tree())
+
+  # Remove the subdir from disk, and validate the status
+  svntest.main.safe_rmtree(C_path)
+
+  expected_disk.remove('A/C')
+  actual_disk_tree = svntest.tree.build_tree_from_wc(wc_dir, load_props=True)
+  svntest.tree.compare_trees("disk", actual_disk_tree,
+                             expected_disk.old_tree())
+
+  expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+  expected_status.tweak('A/C', status='! ', wc_rev='?')
+  svntest.actions.run_and_verify_status(wc_dir, expected_status)
+
+  # Drop an empty file there to obstruct the now-deleted subdir
+  open(C_path, 'w')
+
+  expected_disk.add({'A/C': Item(contents='')})
+  actual_disk_tree = svntest.tree.build_tree_from_wc(wc_dir, load_props=True)
+  svntest.tree.compare_trees("disk", actual_disk_tree,
+                             expected_disk.old_tree())
+
+  # NOTE: r943346 fixes a problem with reporter processing, which
+  #   is necessary for this status to complete properly.
+  expected_status.tweak('A/C', status='~ ', wc_rev='?')
+  svntest.actions.run_and_verify_status(wc_dir, expected_status)
+
+
 ########################################################################
 # Run the tests
 
@@ -1891,6 +1936,7 @@ test_list = [ None,
               XFail(post_revprop_change_hook, svntest.main.is_ra_type_dav),
               rm_of_replaced_file,
               prop_reject_grind,
+              obstructed_subdirs,
              ]
 
 if __name__ == '__main__':