Thanks for your comments, Stefan.
A patch to create a skeleton for this feature is attached. Running it
looks like this:
$ SVN_ELEMENT_MERGE=1 svn merge -v --ignore-ancestry -c1700035
^/subversion/trunk
--- Merging
--- Merging by elements: left=^/subversion/trunk@1700034,
right=^/subversion/trunk@1700035, matching={...}
--- Assigning EIDs to trees
--- Merging trees
--- Writing merge result to WC
Stefan Fuhrmann wrote:
> Julian Foad wrote:
>> For the first practical application of element-based move tracking and
>> merging, I propose to write an element-based alternative to 'svn
>> merge'. This version of 'svn merge':
>>
>> * matches tree elements by path, like today...
>> * ... except where the user specifies a different matching (aka a
>> 'move')
>> * uses the element-based merge algorithm (which handles moves)
>
> That part is somewhat unclear to me. Possible interpretations include: [...]
I mean that if the user doesn't explicitly specify any moves, then it
will match by path only, which will result in behaviour similar to
today. If the user specifies one or more 'moves', those will override
the path matching for the specified element(s). e.g. the user might
specify
svn merge ... \
--match:(src_left="foo", src_right="x/bar", target="foo")
to specify that there was a move from 'foo' to 'x/bar' in the source,
and this node corresponds to 'foo' in the target.
>> This is like the implementation in 'svnmover' except it operates on a
>> real repository and WC, and has to be given the matching data
>> manually.
>>
>> Most of the required pieces are in place now, in svnmover.
>
> Excellent! Good to see all the hard work you put into move tracking
> over the years to come to fruition.
>
>> The benefits are:
>>
>> * a merge involving moves will Just Work, without the moves causing
>> any conflicts to be reported.
>
> Yay! Do you think it is realistic to complete this within (less than)
> a year in time for 1.10?
Yes, actually I do.
>> The initial implementation will work like this, as a minimum, with the
>> following limitations:
>>
>> * input: a requested merge from known, single-revision source trees,
>> either discovered by 'automatic merge' or specified by user
>> * input: a set of user-specified element matchings, each of the form
>> (src-left-path, src-right-path, target-path)
>> * assign EIDs (in memory) to the source-left, source-right and target
>> trees
>> * perform a tree merge, creating a temporary result (in memory)
>> * check for (new style) conflicts in the result; if any, bail out
>> * convert the result to a series of WC edits and apply those to the WC
>> * forget all the EID metadata
>>
>> Work needed:
>>
>> 1. A UI and data structure to input the user-specified matchings.
>> (Straightforward.)
>>
>> 2. A routine that assigns EIDs to the three trees based on the
>> user-specified matchings and path matching. (Straightforward.)
>
> A while ago I've been thinking about how to store the EID assignments
> in FSFS and FSX. Adding (root, branchID, EID) -> (noderev, parent)
> and (root, path) -> EID mappings should be easy enough. My guess
> is that it would take 1 week to do code + another one to get functional.
> And then X amount of time to test & stabilize.
>
> Is repo-side support for branches and EIDs even useful at this point?
> Do we know enough of what their semantics would be? If so, I'd be
> happy to post a design sketch.
Can we take that to another thread please?
>> 3. Fetching the contents of the source-side trees -- easiest would be
>> to fetch each tree separately in its entirety. A big performance
>> improvement would be to fetch src_left and diff(src_left:src_right),
>> and construct src_right from those. Better still, also construct
>> src_left from target and diff(target, src_left).
>>
>> 4. Conversion of the element-based result to a series of WC edits. The
>> code in branch_compat.c doesn't quite do this, as it assumes an Ev1
>> output (with only a 'copy' operation) whereas the WC API has a 'move'
>> operation that we probably need to use. In general it will need to
>> insert temporary moves e.g. to swap X and Y it may need to start by
>> moving X to temporary name Z. Unless the WC API moves can also be set
>> up using just 'copy' operations, in which case the approach in
>> branch_compat.c is on the right track although it is currently buggy.
>>
>> Items 1 and 2 are straightforward, and 3 can also be straightforward
>> for an initial, crude implementation, and 4 is probably the hardest
>> part.
>
> I agree, 4 sounds messy - just because edit state is inherently
> complex. OTOH, once you know where each element is and where it
> must end up, rebuilding the tree should not be too hard unless you
> try to be clever:
>
> for-each path recursively do:
> if final-eid(path) != current-eid(path)
> if (current-exists(path))
> move-to-temp(current-contents(path))
> if (final-exists(path))
> if (final-is-copy(path))
> copy-to-path(final-contents(path))
> else
> move-to-path(final-contents(path))
> update-locations-recursively(path)
> clear-temp
>
> So, you would always know where a specific contents is and never
> overwrite nor delete any until the very end. Implementation
> details not shown is whether you use trees or maps, are how many
> of them and whether to key them by path or EID.
>
> I'm not even sure that this should be mapped onto the current
> working copy editor. If the whole thing shall be transactional,
> then the following sequence is safer and adds less wc.db overhead:
>
> for-each path recursively do:
> if appears-somewhere-else(path)
> move-to-temp(path)
>
> for-each path recursively do:
> if to-be-deleted(path)
> if missing(path)
> schedule-metadata-removal(path)
> else
> schedule-deletion(path)
> else
> if missing(path)
> schedule-install(path)
>
> run-workqueue
>
> My 2ยข; I'm not a working copy expert.
Thanks. Yes, I agree something like those formulations should do it.
- Julian
Add skeleton code for a merge-by-elements feature.
See dev@ email "[RFC] An element-based 'svn merge'" by me on 2015-12-17,
archived at e.g. http://svn.haxx.se/dev/archive-2015-12/0061.shtml
This is conditional on an environment variable 'SVN_ELEMENT_MERGE' being
set.
* subversion/libsvn_client/client.h
(merge_source_t,
merge_target_t): Move these definitions to here from merge.c.
(svn_client__merge_elements): New.
* subversion/libsvn_client/merge.c
(merge_peg_locked): Call the merge-by-elements code under certain
conditions.
* subversion/libsvn_client/merge_elements.c
New file.
--This line, and those below, will be ignored--
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 1720361)
+++ subversion/libsvn_client/client.h (working copy)
@@ -1182,11 +1182,53 @@ svn_client__remote_propget(apr_hash_t *p
svn_revnum_t revnum,
svn_ra_session_t *ra_session,
svn_depth_t depth,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
+/* */
+typedef struct merge_source_t
+{
+ /* "left" side URL and revision (inclusive iff youngest) */
+ const svn_client__pathrev_t *loc1;
+
+ /* "right" side URL and revision (inclusive iff youngest) */
+ const svn_client__pathrev_t *loc2;
+
+ /* True iff LOC1 is an ancestor of LOC2 or vice-versa (history-wise). */
+ svn_boolean_t ancestral;
+} merge_source_t;
+
+/* Description of the merge target root node (a WC working node) */
+typedef struct merge_target_t
+{
+ /* Absolute path to the WC node */
+ const char *abspath;
+
+ /* The repository location of the base node of the target WC. If the node
+ * is locally added, then URL & REV are NULL & SVN_INVALID_REVNUM.
+ * REPOS_ROOT_URL and REPOS_UUID are always valid. */
+ svn_client__pathrev_t loc;
+
+} merge_target_t;
+
+/*
+ * Similar API to svn_client_merge_peg5().
+ */
+svn_error_t *
+svn_client__merge_elements(svn_boolean_t *use_sleep,
+ apr_array_header_t *merge_sources,
+ merge_target_t *target,
+ svn_ra_session_t *ra_session,
+ svn_boolean_t diff_ignore_ancestry,
+ svn_boolean_t force_delete,
+ svn_boolean_t dry_run,
+ const apr_array_header_t *merge_options,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
#endif /* SVN_LIBSVN_CLIENT_H */
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 1720361)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -212,38 +212,12 @@
/*-----------------------------------------------------------------------*/
/*** Repos-Diff Editor Callbacks ***/
-/* */
-typedef struct merge_source_t
-{
- /* "left" side URL and revision (inclusive iff youngest) */
- const svn_client__pathrev_t *loc1;
-
- /* "right" side URL and revision (inclusive iff youngest) */
- const svn_client__pathrev_t *loc2;
-
- /* True iff LOC1 is an ancestor of LOC2 or vice-versa (history-wise). */
- svn_boolean_t ancestral;
-} merge_source_t;
-
-/* Description of the merge target root node (a WC working node) */
-typedef struct merge_target_t
-{
- /* Absolute path to the WC node */
- const char *abspath;
-
- /* The repository location of the base node of the target WC. If the node
- * is locally added, then URL & REV are NULL & SVN_INVALID_REVNUM.
- * REPOS_ROOT_URL and REPOS_UUID are always valid. */
- svn_client__pathrev_t loc;
-
-} merge_target_t;
-
typedef struct merge_cmd_baton_t {
svn_boolean_t force_delete; /* Delete a file/dir even if modified */
svn_boolean_t dry_run;
svn_boolean_t record_only; /* Whether to merge only mergeinfo
differences. */
svn_boolean_t same_repos; /* Whether the merge source repository
@@ -11850,12 +11824,27 @@ merge_peg_locked(conflict_report_t **con
/* Check for same_repos. */
same_repos = is_same_repos(&target->loc, source_loc, TRUE /* strict_urls */);
/* Do the real merge! (We say with confidence that our merge
sources are both ancestral and related.) */
+ if (getenv("SVN_ELEMENT_MERGE")
+ && same_repos
+ && (depth == svn_depth_infinity || depth == svn_depth_unknown)
+ && ignore_mergeinfo
+ && !record_only)
+ {
+ err = svn_client__merge_elements(&use_sleep,
+ merge_sources, target, ra_session,
+ diff_ignore_ancestry, force_delete,
+ dry_run, merge_options,
+ ctx, result_pool, scratch_pool);
+ /* ### Currently this merge just errors out on any conflicts */
+ *conflict_report = NULL;
+ }
+ else
err = do_merge(NULL, NULL, conflict_report, &use_sleep,
merge_sources, target, ra_session,
TRUE /*sources_related*/, same_repos, ignore_mergeinfo,
diff_ignore_ancestry, force_delete, dry_run,
record_only, NULL, FALSE, FALSE, depth, merge_options,
ctx, result_pool, scratch_pool);
Index: subversion/libsvn_client/merge_elements.c
===================================================================
--- subversion/libsvn_client/merge_elements.c (nonexistent)
+++ subversion/libsvn_client/merge_elements.c (working copy)
@@ -0,0 +1,248 @@
+/*
+ * merge_elements.c: element-based merging
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+
+#include <assert.h>
+#include <apr_strings.h>
+#include <apr_tables.h>
+#include <apr_hash.h>
+#include "svn_types.h"
+#include "svn_error.h"
+#include "svn_pools.h"
+#include "svn_hash.h"
+#include "svn_wc.h"
+#include "svn_client.h"
+#include "svn_dirent_uri.h"
+
+#include "client.h"
+#include "private/svn_element.h"
+
+#include "svn_private_config.h"
+
+
+/* Print a notification.
+ * ### TODO: Send notifications through ctx->notify_func2().
+ * ### TODO: Only when 'verbose' output is requested.
+ */
+static
+__attribute__((format(printf, 1, 2)))
+void
+verbose_notify(const char *fmt,
+ ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ vprintf(fmt, ap);
+ if (fmt[strlen(fmt) - 1] != '\n')
+ printf("\n");
+ va_end(ap);
+}
+
+/* Return a string representation of PATHREV. */
+static const char *
+pathrev_str(const svn_client__pathrev_t *pathrev,
+ apr_pool_t *pool)
+{
+ const char *rrpath
+ = svn_uri_skip_ancestor(pathrev->repos_root_url, pathrev->url, pool);
+
+ return apr_psprintf(pool, "^/%s@%ld", rrpath, pathrev->rev);
+}
+
+/* Element matching info.
+ */
+typedef struct element_matching_info_t
+{
+ void *info;
+} element_matching_info_t;
+
+/* Return a string representation of INFO. */
+static const char *
+element_matching_info_str(const element_matching_info_t *info,
+ apr_pool_t *result_pool)
+{
+ /* ### */
+ const char *str = "{...}";
+
+ return str;
+}
+
+/* Assign EIDs (in memory) to the source-left, source-right and target
+ * trees.
+ */
+static svn_error_t *
+assign_eids_to_trees(svn_element__tree_t **tree_left_p,
+ svn_element__tree_t **tree_right_p,
+ svn_element__tree_t **tree_target_p,
+ const svn_client__pathrev_t *src_left,
+ const svn_client__pathrev_t *src_right,
+ merge_target_t *target,
+ svn_ra_session_t *ra_session,
+ element_matching_info_t *element_matching_info,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ verbose_notify("--- Assigning EIDs to trees");
+
+ /* ### */
+ return SVN_NO_ERROR;
+}
+
+/* Perform a three-way tree merge. Write the result to *MERGE_RESULT_P.
+ *
+ * Set *CONFLICTS_P to describe any conflicts, or set *CONFLICTS_P to
+ * null if there are none.
+ */
+static svn_error_t *
+merge_trees(svn_element__tree_t **merge_result_p,
+ void **conflicts_p,
+ svn_element__tree_t *tree_left,
+ svn_element__tree_t *tree_right,
+ svn_element__tree_t *tree_target,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ verbose_notify("--- Merging trees");
+
+ /* ### */
+ *merge_result_p = NULL;
+ *conflicts_p = NULL;
+ return SVN_NO_ERROR;
+}
+
+/* Convert the MERGE_RESULT to a series of WC edits and apply those to
+ * the WC described in TARGET.
+ */
+static svn_error_t *
+apply_merge_result_to_wc(merge_target_t *target,
+ svn_element__tree_t *merge_result,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ verbose_notify("--- Writing merge result to WC");
+
+ return SVN_NO_ERROR;
+}
+
+/* Do a three-way element-based merge for one merge source range,
+ * SRC_LEFT:SRC_RIGHT. If there are no conflicts, write the result to the
+ * WC described in TARGET.
+ */
+static svn_error_t *
+merge_elements_one_source(svn_boolean_t *use_sleep,
+ const svn_client__pathrev_t *src_left,
+ const svn_client__pathrev_t *src_right,
+ merge_target_t *target,
+ svn_ra_session_t *ra_session,
+ element_matching_info_t *element_matching_info,
+ svn_boolean_t diff_ignore_ancestry,
+ svn_boolean_t force_delete,
+ svn_boolean_t dry_run,
+ const apr_array_header_t *merge_options,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ svn_element__tree_t *tree_left, *tree_right, *tree_target;
+ svn_element__tree_t *merge_result;
+ void *conflicts;
+
+ verbose_notify("--- Merging by elements: "
+ "left=%s, right=%s, matching=%s",
+ pathrev_str(src_left, scratch_pool),
+ pathrev_str(src_right, scratch_pool),
+ element_matching_info_str(element_matching_info,
+ scratch_pool));
+
+ /* assign EIDs (in memory) to the source-left, source-right and target
+ trees */
+ SVN_ERR(assign_eids_to_trees(&tree_left, &tree_right, &tree_target,
+ src_left, src_right, target, ra_session,
+ element_matching_info,
+ ctx, scratch_pool, scratch_pool));
+
+ /* perform a tree merge, creating a temporary result (in memory) */
+ SVN_ERR(merge_trees(&merge_result, &conflicts,
+ tree_left, tree_right, tree_target,
+ scratch_pool, scratch_pool));
+
+ /* check for (new style) conflicts in the result; if any, bail out */
+ if (conflicts)
+ {
+ return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
+ _("Merge had conflicts; "
+ "this is not yet supported"));
+ }
+
+ /* convert the result to a series of WC edits and apply those to the WC */
+ if (dry_run)
+ {
+ verbose_notify("--- Dry run; not writing merge result to WC");
+ }
+ else
+ {
+ SVN_ERR(apply_merge_result_to_wc(target, merge_result,
+ ctx, scratch_pool));
+ *use_sleep = TRUE;
+ }
+
+ /* forget all the EID metadata */
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__merge_elements(svn_boolean_t *use_sleep,
+ apr_array_header_t *merge_sources,
+ merge_target_t *target,
+ svn_ra_session_t *ra_session,
+ svn_boolean_t diff_ignore_ancestry,
+ svn_boolean_t force_delete,
+ svn_boolean_t dry_run,
+ const apr_array_header_t *merge_options,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ int i;
+
+ /* Merge each source range in turn */
+ for (i = 0; i < merge_sources->nelts; i++)
+ {
+ merge_source_t *source
+ = APR_ARRAY_IDX(merge_sources, i, void *);
+ element_matching_info_t *element_matching_info;
+
+ /* ### TODO: get element matching info from the user */
+ element_matching_info = NULL;
+
+ SVN_ERR(merge_elements_one_source(use_sleep,
+ source->loc1, source->loc2,
+ target, ra_session,
+ element_matching_info,
+ diff_ignore_ancestry,
+ force_delete, dry_run, merge_options,
+ ctx, scratch_pool));
+ }
+
+ return SVN_NO_ERROR;
+}
Property changes on: subversion/libsvn_client/merge_elements.c
___________________________________________________________________
Added: svn:eol-style
## -0,0 +1 ##
+native
\ No newline at end of property
