Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@subversion.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1241748 - in /subversion/trunk/subversion:
 include/svn_editor.h
 libsvn_delta/editor.c
Date: Wed, 08 Feb 2012 02:37:34 -0000
To: commits@subversion.apache.org
From: gstein@apache.org
Message-Id: <20120208023735.23753238897D@eris.apache.org>

Author: gstein
Date: Wed Feb  8 02:37:34 2012
New Revision: 1241748

URL: http://svn.apache.org/viewvc?rev=1241748&view=rev
Log:
Strengthen the discussion and checks around passing all children into
an add_directory() call.

* subversion/include/svn_editor.h:
  (...): clarify that children must be mentioned in the parent's
    add_directory call.

* subversion/libsvn_delta/editor.c:
  (...): reorder some of the marker declarations and add docs
  (marker_added_dir, MARKER_ADDED_DIR): new marker
  (MARK_RELPATH): helper macro for marking stuff in completed_nodes
  (MARK_COMPLETED, MARK_ALLOW_ADD, MARK_ALLOW_ALTER): use MARK_RELPATH
  (MARK_ADDED_DIR): new marking helper for nodes from add_directory.
    another variant for when checking is disabled.
  (CHECK_UNKNOWN_CHILD): new validation to ensure a child under an
    added directory was mentioned as a child. plus a variant for when
    checking is disabled.
  (check_unknown_child): helper for above
  (svn_editor_add_directory): mark this specifically as an added dir.
    use the new CHECK_UNKNOWN_CHILD() checking macro.
  (svn_editor_add_file, svn_editor_add_symlink,
      svn_editor_add_absent): use the new CHECK_UNKNOWN_CHILD macro.

Modified:
    subversion/trunk/subversion/include/svn_editor.h
    subversion/trunk/subversion/libsvn_delta/editor.c

Modified: subversion/trunk/subversion/include/svn_editor.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_editor.h?rev=1241748&r1=1241747&r2=1241748&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_editor.h (original)
+++ subversion/trunk/subversion/include/svn_editor.h Wed Feb  8 02:37:34 2012
@@ -241,10 +241,11 @@ extern "C" {
  *   follow for each child mentioned in the @a children argument of any
  *   svn_editor_add_directory() call.
  *
- * - ### add section describing: add_* cannot be called for a child of
- *   ### a directory added via add_directory in this edit, where that
- *   ### child was not mentioned. ie. it should have been passed to the
- *   ### parent add_directory so it could be marked as incomplete.
+ * - For each node created with add_*, if its parent was created using
+ *   svn_editor_add_directory(), then the new child node MUST have been
+ *   mentioned in the @a children parameter of the parent's creation.
+ *   This allows the parent directory to properly mark the child as
+ *   "incomplete" until the child's add_* call arrives.
  *
  * - A path should
  *   never be referenced more than once by the add_*, alter_*, and

Modified: subversion/trunk/subversion/libsvn_delta/editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_delta/editor.c?rev=1241748&r1=1241747&r2=1241748&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_delta/editor.c (original)
+++ subversion/trunk/subversion/libsvn_delta/editor.c Wed Feb  8 02:37:34 2012
@@ -65,13 +65,26 @@ struct svn_editor_t
 
 #ifdef ENABLE_ORDERING_CHECK
 
+/* Marker to indicate no further changes are allowed on this node.  */
 static const int marker_done;
-static const int marker_allow_add;
-static const int marker_allow_alter;
 #define MARKER_DONE (&marker_done)
+
+/* Marker indicating that add_* may be called for this path, or that it
+   can be the destination of a copy or move. For copy/move, the path
+   will switch to MARKER_ALLOW_ALTER, to enable further tweaks.  */
+static const int marker_allow_add;
 #define MARKER_ALLOW_ADD (&marker_allow_add)
+
+/* Marker indicating that alter_* may be called for this path.  */
+static const int marker_allow_alter;
 #define MARKER_ALLOW_ALTER (&marker_allow_alter)
 
+/* Just like MARKER_DONE, but also indicates that the node was created
+   via add_directory(). This allows us to verify that the CHILDREN param
+   was comprehensive.  */
+static const int marker_added_dir;
+#define MARKER_ADDED_DIR (&marker_added_dir)
+
 #define MARK_FINISHED(editor) ((editor)->finished = TRUE)
 #define SHOULD_NOT_BE_FINISHED(editor)  SVN_ERR_ASSERT(!(editor)->finished)
 
@@ -79,28 +92,32 @@ static const int marker_allow_alter;
   apr_hash_set((editor)->pending_incomplete_children, relpath,  \
                APR_HASH_KEY_STRING, NULL);
 
-#define MARK_COMPLETED(editor, relpath)   \
+#define MARK_RELPATH(editor, relpath, value) \
   apr_hash_set((editor)->completed_nodes, \
                apr_pstrdup((editor)->result_pool, relpath), \
-               APR_HASH_KEY_STRING, MARKER_DONE)
+               APR_HASH_KEY_STRING, value)
+
+#define MARK_COMPLETED(editor, relpath) \
+  MARK_RELPATH(editor, relpath, MARKER_DONE)
 #define SHOULD_NOT_BE_COMPLETED(editor, relpath) \
   SVN_ERR_ASSERT(apr_hash_get((editor)->completed_nodes, relpath, \
                               APR_HASH_KEY_STRING) == NULL)
 
 #define MARK_ALLOW_ADD(editor, relpath) \
-  apr_hash_set((editor)->completed_nodes, \
-               apr_pstrdup((editor)->result_pool, relpath), \
-               APR_HASH_KEY_STRING, MARKER_ALLOW_ADD)
+  MARK_RELPATH(editor, relpath, MARKER_ALLOW_ADD)
 #define SHOULD_ALLOW_ADD(editor, relpath) \
   SVN_ERR_ASSERT(allow_either(editor, relpath, MARKER_ALLOW_ADD, NULL))
 
 #define MARK_ALLOW_ALTER(editor, relpath) \
-  apr_hash_set((editor)->completed_nodes, \
-               apr_pstrdup((editor)->result_pool, relpath), \
-               APR_HASH_KEY_STRING, MARKER_ALLOW_ALTER)
+  MARK_RELPATH(editor, relpath, MARKER_ALLOW_ALTER)
 #define SHOULD_ALLOW_ALTER(editor, relpath) \
   SVN_ERR_ASSERT(allow_either(editor, relpath, MARKER_ALLOW_ALTER, NULL))
 
+#define MARK_ADDED_DIR(editor, relpath) \
+  MARK_RELPATH(editor, relpath, MARKER_ADDED_DIR)
+#define CHECK_UNKNOWN_CHILD(editor, relpath) \
+  SVN_ERR_ASSERT(check_unknown_child(editor, relpath))
+
 static svn_boolean_t
 allow_either(const svn_editor_t *editor,
              const char *relpath,
@@ -112,6 +129,32 @@ allow_either(const svn_editor_t *editor,
   return value == marker1 || value == marker2;
 }
 
+static svn_boolean_t
+check_unknown_child(const svn_editor_t *editor,
+                    const char *relpath)
+{
+  const char *parent;
+
+  /* If we already know about the new child, then exit early.  */
+  if (apr_hash_get(editor->pending_incomplete_children, relpath,
+                   APR_HASH_KEY_STRING) != NULL)
+    return TRUE;
+
+  parent = svn_relpath_dirname(relpath, editor->scratch_pool);
+
+  /* Was this parent created via svn_editor_add_directory() ?  */
+  if (apr_hash_get(editor->completed_nodes, relpath, APR_HASH_KEY_STRING)
+      == MARKER_ADDED_DIR)
+    {
+      /* Whoops. This child should have been listed in that add call,
+         and placed into ->pending_incomplete_children.  */
+      return FALSE;
+    }
+
+  /* The parent was not added in this drive.  */
+  return TRUE;
+}
+
 #else
 
 /* Be wary with the definition of these macros so that we don't
@@ -132,6 +175,9 @@ allow_either(const svn_editor_t *editor,
 #define MARK_ALLOW_ALTER(editor, relpath)  /* empty */
 #define SHOULD_ALLOW_ALTER(editor, relpath)  /* empty */
 
+#define MARK_ADDED_DIR(editor, relpath)  /* empty */
+#define CHECK_UNKNOWN_CHILD(editor, relpath)  /* empty */
+
 #endif /* ENABLE_ORDERING_CHECK */
 
 
@@ -331,6 +377,7 @@ svn_editor_add_directory(svn_editor_t *e
   SVN_ERR_ASSERT(props != NULL);
   SHOULD_NOT_BE_FINISHED(editor);
   SHOULD_ALLOW_ADD(editor, relpath);
+  CHECK_UNKNOWN_CHILD(editor, relpath);
 
   if (editor->cancel_func)
     SVN_ERR(editor->cancel_func(editor->cancel_baton));
@@ -340,7 +387,7 @@ svn_editor_add_directory(svn_editor_t *e
                                          props, replaces_rev,
                                          editor->scratch_pool);
 
-  MARK_COMPLETED(editor, relpath);
+  MARK_ADDED_DIR(editor, relpath);
   CLEAR_INCOMPLETE(editor, relpath);
 
 #ifdef ENABLE_ORDERING_CHECK
@@ -378,6 +425,7 @@ svn_editor_add_file(svn_editor_t *editor
   SVN_ERR_ASSERT(props != NULL);
   SHOULD_NOT_BE_FINISHED(editor);
   SHOULD_ALLOW_ADD(editor, relpath);
+  CHECK_UNKNOWN_CHILD(editor, relpath);
 
   if (editor->cancel_func)
     SVN_ERR(editor->cancel_func(editor->cancel_baton));
@@ -407,6 +455,7 @@ svn_editor_add_symlink(svn_editor_t *edi
   SVN_ERR_ASSERT(props != NULL);
   SHOULD_NOT_BE_FINISHED(editor);
   SHOULD_ALLOW_ADD(editor, relpath);
+  CHECK_UNKNOWN_CHILD(editor, relpath);
 
   if (editor->cancel_func)
     SVN_ERR(editor->cancel_func(editor->cancel_baton));
@@ -433,6 +482,7 @@ svn_editor_add_absent(svn_editor_t *edit
 
   SHOULD_NOT_BE_FINISHED(editor);
   SHOULD_ALLOW_ADD(editor, relpath);
+  CHECK_UNKNOWN_CHILD(editor, relpath);
 
   if (editor->cancel_func)
     SVN_ERR(editor->cancel_func(editor->cancel_baton));