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/11 05:12:37 UTC

svn commit: r942991 - in /subversion/trunk: notes/api-errata/ subversion/include/ subversion/libsvn_client/ subversion/libsvn_wc/

Author: gstein
Date: Tue May 11 03:12:36 2010
New Revision: 942991

URL: http://svn.apache.org/viewvc?rev=942991&view=rev
Log:
Remove the BASE_MERGE parameter from svn_wc_merge_props3().

For this function's ancestors, which continue to have BASE_MERGE, we will
now THROW AN ERROR. With more work, we can restore the operation of this
parameter, but I do not believe it to be useful/advisable/worthwhile. The
code is mostly still present, pending discussion.

A new errata has been filed to detail this issue, also noting that several
clients do not attempt to use this removed functionality.

* subversion/include/svn_wc.h:
  (svn_wc_merge_props3): remove the BASE_MERGE parameter. update the
    docstring.
  (svn_wc_merge_props2): discuss the BASE_MERGE parameter.

* subversion/libsvn_wc/props.h:
  (SVN__SUPPORT_BASE_MERGE): temporary(?) define to wrap functionality
    that can/should be removed, or can be restored (with some fixes) to
    support BASE_MERGE.
  (svn_wc__perform_props_merge): a new internal function that is wrapped
    by svn_wc_merge_props3, but *does* support BASE_MERGE.

* subversion/libsvn_wc/props.c:
  (queue_install_props): remove this function from compilation, based on
    the SVN__SUPPORT_BASE_MERGE define.
  (svn_wc_merge_props3): renamed to ...
  (svn_wc__perform_props_merge): ... this. swap a DB param for a WC_CTX.
    only make the call to queue_install_props if SVN__SUPPORT_BASE_MERGE
    is defined. otherwise, update the actual props along with queueing a
    work item to establish the old-style props file.
  (svn_wc_merge_props3): reintroduced as a simple wrapper for
    svn_wc__perform_props_merge.

* subversion/libsvn_wc/deprecated.c:
  (svn_wc_merge_props2): call svn_wc__perform_props_merge instead of
    svn_wc_merge_props3. side benefit of no wc_ctx needed.

* subversion/libsvn_wc/workqueue.h:
  (svn_wc__wq_add_install_properties): only declare when
    SVN__SUPPORT_BASE_MERGE is defined.

* subversion/libsvn_wc/workqueue.c:
  (run_install_propertes, svn_wc__wq_add_install_properties): only
    declare when SVN__SUPPORT_BASE_MERGE is defined.
  (dispatch_table): add handler for OP_INSTALL_PROPERTIES only when
    SVN__SUPPORT_BASE_MERGE.

* subversion/libsvn_client/merge.c:
  (merge_props_changed): remove BASE_MERGE param from call

* notes/api-errata/wc006.txt: new errata

Added:
    subversion/trunk/notes/api-errata/wc006.txt   (with props)
Modified:
    subversion/trunk/subversion/include/svn_wc.h
    subversion/trunk/subversion/libsvn_client/merge.c
    subversion/trunk/subversion/libsvn_wc/deprecated.c
    subversion/trunk/subversion/libsvn_wc/props.c
    subversion/trunk/subversion/libsvn_wc/props.h
    subversion/trunk/subversion/libsvn_wc/workqueue.c
    subversion/trunk/subversion/libsvn_wc/workqueue.h

Added: subversion/trunk/notes/api-errata/wc006.txt
URL: http://svn.apache.org/viewvc/subversion/trunk/notes/api-errata/wc006.txt?rev=942991&view=auto
==============================================================================
--- subversion/trunk/notes/api-errata/wc006.txt (added)
+++ subversion/trunk/notes/api-errata/wc006.txt Tue May 11 03:12:36 2010
@@ -0,0 +1,54 @@
+API ERRATA -- $Id$
+
+Root Cause of Errata: incompatible
+ Library(s) Affected: libsvn_wc
+Function(s) Affected: svn_wc_merge_props2
+                      svn_wc_merge_props
+                      svn_wc_merge_prop_diffs
+     New Behavior in: 1.7
+      Related Issues: 953
+
+
+== Details of Previous Behavior ==
+
+The affected functions have a BASE_MERGE parameter that will cause the
+function to apply the specified property changes to the pristine
+("base") properties when set to TRUE.
+
+This capability originated in r843714 (aka r3640) to solve issue #953.
+Prior to that change, merges were updating the pristine properties,
+and this flag enabled merge to *avoid* that effect.
+
+The affected functions are wrappers around internal functions. The
+(internal) svn_wc__merge_prop_diffs() and its later equivalent,
+svn_wc__merge_props(), are used by the update editor, so its BASE_MERGE
+parameter is necessary and appropriate.
+
+
+== Details of New Behavior ==
+
+Passing TRUE for BASE_MERGE to any of the affected functions, while
+DRY_RUN is FALSE, will result in SVN_ERR_UNSUPPORTED_FEATURE.
+
+The parameter has been removed from svn_wc_merge_props3().
+
+Internal functions still support the parameter, for the benefit up the
+update editor.
+
+
+== Rationale for Change ==
+
+Allowing third-party clients to alter pristine data, to a state NOT
+defined by the repository could easily lead to working copy corruption
+(as seen in issue #953), and potentially lead to improper changes
+applied against a repository.
+
+
+== Impact on API Users ==
+
+Clients using the old (deprecated) interfaces which pass TRUE for
+BASE_MERGE, and FALSE for DRY_RUN, will no longer work after an
+upgrade to the 1.7 libsvn_wc library.
+
+Subversion itself does not use this feature, nor does TortoiseSVN, nor
+AnkhSVN.

Propchange: subversion/trunk/notes/api-errata/wc006.txt
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: subversion/trunk/notes/api-errata/wc006.txt
------------------------------------------------------------------------------
    svn:keywords = Id

Modified: subversion/trunk/subversion/include/svn_wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=942991&r1=942990&r2=942991&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_wc.h (original)
+++ subversion/trunk/subversion/include/svn_wc.h Tue May 11 03:12:36 2010
@@ -6513,17 +6513,14 @@ svn_wc_merge(const char *left,
  * representing the original set of properties that @a propchanges is
  * working against.  @a wc_ctx contains a lock for @a local_abspath.
  *
- * If @a base_merge is @c FALSE only the working properties will be changed,
- * if it is @c TRUE both the base and working properties will be changed.
+ * Only the working properties will be changed.
  *
  * If @a state is non-NULL, set @a *state to the state of the properties
  * after the merge.
  *
  * If conflicts are found when merging working properties, they are
  * described in a temporary .prej file (or appended to an already-existing
- * .prej file), and the entry is marked "conflicted".  Base properties
- * are changed unconditionally, if @a base_merge is @c TRUE, they never result
- * in a conflict.
+ * .prej file), and the entry is marked "conflicted".
  *
  * If @a cancel_func is non-NULL, invoke it with @a cancel_baton at various
  * points during the operation.  If it returns an error (typically
@@ -6542,7 +6539,6 @@ svn_wc_merge_props3(svn_wc_notify_state_
                     const svn_wc_conflict_version_t *right_version,
                     apr_hash_t *baseprops,
                     const apr_array_header_t *propchanges,
-                    svn_boolean_t base_merge,
                     svn_boolean_t dry_run,
                     svn_wc_conflict_resolver_func_t conflict_func,
                     void *conflict_baton,
@@ -6550,8 +6546,14 @@ svn_wc_merge_props3(svn_wc_notify_state_
                     void *cancel_baton,
                     apr_pool_t *scratch_pool);
 
+
 /** Similar to svn_wc_merge_props3, but takes an access baton and relative
- * path, no cancel_function and no left and right version.
+ * path, no cancel_function, and no left and right version.
+ *
+ * This function has the @a base_merge parameter which (when TRUE) will
+ * apply @a propchanges to this node's pristine set of properties. This
+ * functionality is not supported on newer APIs -- pristine information
+ * should only be changed through an update editor drive.
  *
  * @since New in 1.5.
  * @deprecated Provided for backward compatibility with the 1.6 API.

Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=942991&r1=942990&r2=942991&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Tue May 11 03:12:36 2010
@@ -1140,7 +1140,7 @@ merge_props_changed(const char *local_di
                                                   subpool));
 
       err = svn_wc_merge_props3(state, ctx->wc_ctx, local_abspath, NULL, NULL,
-                                original_props, props, FALSE, merge_b->dry_run,
+                                original_props, props, merge_b->dry_run,
                                 ctx->conflict_func, ctx->conflict_baton,
                                 ctx->cancel_func, ctx->cancel_baton,
                                 subpool);

Modified: subversion/trunk/subversion/libsvn_wc/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/deprecated.c?rev=942991&r1=942990&r2=942991&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/deprecated.c (original)
+++ subversion/trunk/subversion/libsvn_wc/deprecated.c Tue May 11 03:12:36 2010
@@ -2130,29 +2130,25 @@ svn_wc_merge_props2(svn_wc_notify_state_
                     svn_boolean_t dry_run,
                     svn_wc_conflict_resolver_func_t conflict_func,
                     void *conflict_baton,
-                    apr_pool_t *pool)
+                    apr_pool_t *scratch_pool)
 {
-  svn_wc_context_t *wc_ctx;
   const char *local_abspath;
 
-  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
-  SVN_ERR(svn_wc__context_create_with_db(&wc_ctx, NULL /* config */,
-                                         svn_wc__adm_get_db(adm_access), pool));
-
-  SVN_ERR(svn_wc_merge_props3(state,
-                              wc_ctx,
-                              local_abspath,
-                              NULL, /* left_version */
-                              NULL, /* right_version */
-                              baseprops,
-                              propchanges,
-                              base_merge,
-                              dry_run,
-                              conflict_func, conflict_baton,
-                              NULL, NULL,
-                              pool));
+  SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, scratch_pool));
 
-  return svn_error_return(svn_wc_context_destroy(wc_ctx));
+  return svn_error_return(svn_wc__perform_props_merge(
+                            state,
+                            svn_wc__adm_get_db(adm_access),
+                            local_abspath,
+                            NULL /* left_version */,
+                            NULL /* right_version */,
+                            baseprops,
+                            propchanges,
+                            base_merge,
+                            dry_run,
+                            conflict_func, conflict_baton,
+                            NULL, NULL,
+                            scratch_pool));
 }
 
 svn_error_t *

Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=942991&r1=942990&r2=942991&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Tue May 11 03:12:36 2010
@@ -332,6 +332,9 @@ svn_wc__load_revert_props(apr_hash_t **r
 }
 
 
+/* See props.h  */
+#ifdef SVN__SUPPORT_BASE_MERGE
+
 /* Add a working queue item to install PROPS and, if INSTALL_PRISTINE_PROPS is
    TRUE, BASE_PROPS for the LOCAL_ABSPATH in DB, updating the node to reflect
    the changes.  PRISTINE_PROPS must be supplied even if INSTALL_PRISTINE_PROPS
@@ -396,6 +399,9 @@ queue_install_props(svn_wc__db_t *db,
   return SVN_NO_ERROR;
 }
 
+#endif /* SVN__SUPPORT_BASE_MERGE  */
+
+
 /* */
 static svn_error_t *
 immediate_install_props(svn_wc__db_t *db,
@@ -577,20 +583,20 @@ combine_forked_mergeinfo_props(const svn
 
 
 svn_error_t *
-svn_wc_merge_props3(svn_wc_notify_state_t *state,
-                    svn_wc_context_t *wc_ctx,
-                    const char *local_abspath,
-                    const svn_wc_conflict_version_t *left_version,
-                    const svn_wc_conflict_version_t *right_version,
-                    apr_hash_t *baseprops,
-                    const apr_array_header_t *propchanges,
-                    svn_boolean_t base_merge,
-                    svn_boolean_t dry_run,
-                    svn_wc_conflict_resolver_func_t conflict_func,
-                    void *conflict_baton,
-                    svn_cancel_func_t cancel_func,
-                    void *cancel_baton,
-                    apr_pool_t *pool /* scratch_pool */)
+svn_wc__perform_props_merge(svn_wc_notify_state_t *state,
+                            svn_wc__db_t *db,
+                            const char *local_abspath,
+                            const svn_wc_conflict_version_t *left_version,
+                            const svn_wc_conflict_version_t *right_version,
+                            apr_hash_t *baseprops,
+                            const apr_array_header_t *propchanges,
+                            svn_boolean_t base_merge,
+                            svn_boolean_t dry_run,
+                            svn_wc_conflict_resolver_func_t conflict_func,
+                            void *conflict_baton,
+                            svn_cancel_func_t cancel_func,
+                            void *cancel_baton,
+                            apr_pool_t *pool /* scratch_pool */)
 {
   svn_boolean_t hidden;
   int i;
@@ -604,7 +610,7 @@ svn_wc_merge_props3(svn_wc_notify_state_
      may be NULL. */
 
   /* Checks whether the node exists and returns the hidden flag */
-  SVN_ERR(svn_wc__db_node_hidden(&hidden, wc_ctx->db, local_abspath, pool));
+  SVN_ERR(svn_wc__db_node_hidden(&hidden, db, local_abspath, pool));
   if (hidden)
     return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
                              _("The node '%s' was not found."),
@@ -625,20 +631,20 @@ svn_wc_merge_props3(svn_wc_notify_state_
                                  svn_dirent_local_style(local_abspath, pool));
     }
 
-  SVN_ERR(svn_wc__db_read_kind(&kind, wc_ctx->db, local_abspath, FALSE, pool));
+  SVN_ERR(svn_wc__db_read_kind(&kind, db, local_abspath, FALSE, pool));
 
-  SVN_ERR(svn_wc__get_pristine_props(&base_props, wc_ctx->db, local_abspath,
+  SVN_ERR(svn_wc__get_pristine_props(&base_props, db, local_abspath,
                                      pool, pool));
   if (base_props == NULL)
     base_props = apr_hash_make(pool);  /* some nodes have no pristines  */
-  SVN_ERR(svn_wc__get_actual_props(&actual_props, wc_ctx->db, local_abspath,
+  SVN_ERR(svn_wc__get_actual_props(&actual_props, db, local_abspath,
                                    pool, pool));
 
   /* Note that while this routine does the "real" work, it's only
      prepping tempfiles and writing log commands.  */
   SVN_ERR(svn_wc__merge_props(state,
                               &new_base_props, &new_actual_props,
-                              wc_ctx->db, local_abspath, kind,
+                              db, local_abspath, kind,
                               left_version, right_version,
                               baseprops /* server_baseprops */,
                               base_props,
@@ -658,16 +664,50 @@ svn_wc_merge_props3(svn_wc_notify_state_
         dir_abspath = svn_dirent_dirname(local_abspath, pool);
 
       /* Verify that we're holding this directory's write lock.  */
-      SVN_ERR(svn_wc__write_check(wc_ctx->db, dir_abspath, pool));
+      SVN_ERR(svn_wc__write_check(db, dir_abspath, pool));
 
       /* After a (not-dry-run) merge, we ALWAYS have props to save.  */
       SVN_ERR_ASSERT(new_base_props != NULL && new_actual_props != NULL);
 
-      SVN_ERR(queue_install_props(wc_ctx->db, local_abspath, kind,
+/* See props.h  */
+#ifdef SVN__SUPPORT_BASE_MERGE
+      SVN_ERR(queue_install_props(db, local_abspath, kind,
                                   new_base_props, new_actual_props,
                                   base_merge, pool));
+#else
+      if (base_merge)
+        return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
+                                U_("base_merge=TRUE is no longer supported"));
 
-      SVN_ERR(svn_wc__wq_run(wc_ctx->db, local_abspath,
+      {
+        apr_array_header_t *prop_diffs;
+        const char *props_abspath;
+        svn_skel_t *work_item;
+
+        SVN_ERR(svn_prop_diffs(&prop_diffs, new_actual_props, new_base_props,
+                               pool));
+
+        /* Save the actual properties file if it differs from base. */
+        if (prop_diffs->nelts == 0)
+          new_actual_props = NULL; /* Remove actual properties*/
+
+        /* For the old school: write the properties into the "working"
+           (aka ACTUAL) location. Note that PROPS may be NULL, indicating
+           a removal of the props file.  */
+        SVN_ERR(svn_wc__prop_path(&props_abspath, local_abspath, kind,
+                                  svn_wc__props_working, pool));
+        SVN_ERR(svn_wc__wq_build_write_old_props(&work_item,
+                                                 props_abspath,
+                                                 new_actual_props,
+                                                 pool));
+
+        SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, new_actual_props,
+                                        NULL /* conflict */,
+                                        work_item, pool));
+      }
+#endif
+
+      SVN_ERR(svn_wc__wq_run(db, local_abspath,
                              cancel_func, cancel_baton,
                              pool));
     }
@@ -676,6 +716,36 @@ svn_wc_merge_props3(svn_wc_notify_state_
 }
 
 
+svn_error_t *
+svn_wc_merge_props3(svn_wc_notify_state_t *state,
+                    svn_wc_context_t *wc_ctx,
+                    const char *local_abspath,
+                    const svn_wc_conflict_version_t *left_version,
+                    const svn_wc_conflict_version_t *right_version,
+                    apr_hash_t *baseprops,
+                    const apr_array_header_t *propchanges,
+                    svn_boolean_t dry_run,
+                    svn_wc_conflict_resolver_func_t conflict_func,
+                    void *conflict_baton,
+                    svn_cancel_func_t cancel_func,
+                    void *cancel_baton,
+                    apr_pool_t *scratch_pool)
+{
+  return svn_error_return(svn_wc__perform_props_merge(
+                            state,
+                            wc_ctx->db,
+                            local_abspath,
+                            left_version, right_version,
+                            baseprops,
+                            propchanges,
+                            FALSE /* base_merge */,
+                            dry_run,
+                            conflict_func, conflict_baton,
+                            cancel_func, cancel_baton,
+                            scratch_pool));
+}
+
+
 /* Generate a message to describe the property conflict among these four
    values.
 

Modified: subversion/trunk/subversion/libsvn_wc/props.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.h?rev=942991&r1=942990&r2=942991&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.h (original)
+++ subversion/trunk/subversion/libsvn_wc/props.h Tue May 11 03:12:36 2010
@@ -37,6 +37,19 @@
 extern "C" {
 #endif /* __cplusplus */
 
+/* BASE_MERGE is a pre-1.7 concept on property merging. It allowed callers
+   to alter the pristine properties *outside* of an editor drive. That is
+   very dangerous: the pristines should always correspond to something from
+   the repository, and that should only arrive through the update editor.
+
+   For 1.7, we're removing this support. Some old code is being left around
+   in case we decide to change this.
+
+   For more information, see ^/notes/api-errata/wc006.txt
+*/
+#undef SVN__SUPPORT_BASE_MERGE
+
+
 typedef enum svn_wc__props_kind_t
 {
   svn_wc__props_base = 0,
@@ -210,6 +223,24 @@ svn_wc__create_prejfile(const char **tmp
                         apr_pool_t *result_pool,
                         apr_pool_t *scratch_pool);
 
+
+/* Just like svn_wc_merge_props3(), but WITH a BASE_MERGE parameter.  */
+svn_error_t *
+svn_wc__perform_props_merge(svn_wc_notify_state_t *state,
+                            svn_wc__db_t *db,
+                            const char *local_abspath,
+                            const svn_wc_conflict_version_t *left_version,
+                            const svn_wc_conflict_version_t *right_version,
+                            apr_hash_t *baseprops,
+                            const apr_array_header_t *propchanges,
+                            svn_boolean_t base_merge,
+                            svn_boolean_t dry_run,
+                            svn_wc_conflict_resolver_func_t conflict_func,
+                            void *conflict_baton,
+                            svn_cancel_func_t cancel_func,
+                            void *cancel_baton,
+                            apr_pool_t *scratch_pool);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/trunk/subversion/libsvn_wc/workqueue.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.c?rev=942991&r1=942990&r2=942991&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/workqueue.c (original)
+++ subversion/trunk/subversion/libsvn_wc/workqueue.c Tue May 11 03:12:36 2010
@@ -1616,6 +1616,9 @@ svn_wc__wq_add_postcommit(svn_wc__db_t *
 
 /* OP_INSTALL_PROPERTIES */
 
+/* See props.h  */
+#ifdef SVN__SUPPORT_BASE_MERGE
+
 /* Process the OP_INSTALL_PROPERTIES work item WORK_ITEM.
  * See svn_wc__wq_add_install_properties() which generates this work item.
  * Implements (struct work_item_dispatch).func. */
@@ -1718,6 +1721,8 @@ svn_wc__wq_add_install_properties(svn_wc
   return SVN_NO_ERROR;
 }
 
+#endif /* SVN__SUPPORT_BASE_MERGE  */
+
 
 /* ------------------------------------------------------------------------ */
 
@@ -2260,7 +2265,6 @@ static const struct work_item_dispatch d
   { OP_LOGGY, run_loggy },
   { OP_DELETION_POSTCOMMIT, run_deletion_postcommit },
   { OP_POSTCOMMIT, run_postcommit },
-  { OP_INSTALL_PROPERTIES, run_install_properties },
   { OP_DELETE, run_delete },
   { OP_FILE_INSTALL, run_file_install },
   { OP_FILE_REMOVE, run_file_remove },
@@ -2269,6 +2273,11 @@ static const struct work_item_dispatch d
   { OP_WRITE_OLD_PROPS, run_write_old_props },
   { OP_RECORD_FILEINFO, run_record_fileinfo },
 
+/* See props.h  */
+#ifdef SVN__SUPPORT_BASE_MERGE
+  { OP_INSTALL_PROPERTIES, run_install_properties },
+#endif
+
   /* Sentinel.  */
   { NULL }
 };

Modified: subversion/trunk/subversion/libsvn_wc/workqueue.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.h?rev=942991&r1=942990&r2=942991&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/workqueue.h (original)
+++ subversion/trunk/subversion/libsvn_wc/workqueue.h Tue May 11 03:12:36 2010
@@ -232,12 +232,16 @@ svn_wc__wq_add_postcommit(svn_wc__db_t *
                           svn_boolean_t keep_changelist,
                           apr_pool_t *scratch_pool);
 
+
+/* See props.h  */
+#ifdef SVN__SUPPORT_BASE_MERGE
 svn_error_t *
 svn_wc__wq_add_install_properties(svn_wc__db_t *db,
                                   const char *local_abspath,
                                   apr_hash_t *pristine_props,
                                   apr_hash_t *actual_props,
                                   apr_pool_t *scratch_pool);
+#endif
 
 /* Add a work item to delete a node.