You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2021/04/07 13:28:00 UTC
svn commit: r1888474 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_tests.py
Author: julianfoad
Date: Wed Apr 7 13:28:00 2021
New Revision: 1888474
URL: http://svn.apache.org/viewvc?rev=1888474&view=rev
Log:
Fix issue #4874 "Avoid foreign repository merge if repository UUIDs match"
Summary: Repository root URLs must be consistent in a merge. A merge with
differing source and target repository root URLs pointing to the same
repository (same UUID) previously did a foreign-repository merge and is now
disallowed. A two-URL merge with differing source URLs pointing to the same
repository (same UUID) was previously broken and is now disallowed.
The initial problem, encountered in real life, was that Subversion did a
foreign-repository merge when the repository root URLs of source and target
differed but pointed to equivalent repositories (having the same UUID).
In principle Subversion should do a normal merge, not a foreign-repository
merge. However, for both historical reasons (the behaviour had previously
changed twice, in Subversion 1.6 and 1.8) and practical reasons (the code
base is not written to respect different URLs pointing to the same or
equivalent repositories), and because we expect this case to be unnecessary
and rarely encountered, we instead disallow this case. Work-arounds are
noted in the issue.
We also fix a similar issue in the comparison of the two source URLs in a
two-URL merge. In this comparison, there is no "foreign repository" case to
consider. Previously this comparison only ensured their UUIDs matched,
which is logically correct. However, in practice the code base requires
that the two given repository root URLs must be identical in order to ensure
that all subsequent operations on those URLs are consistent, including the
source-target comparison which in fact just compares source1 with target.
New behaviour:
* comparing the two source repository roots in a two-URL merge:
- same repository root URL -> ok
- different repository root URL -> error
* comparing source and target repository roots:
- same repository root URL -> same-repository merge
- different URLs, different UUIDs -> foreign-repository merge
- different URLs, same UUID -> error
(This does not change the fact that the foreign-repository merge case is
not available in "automatic" or "reintegrate" merges.)
* subversion/libsvn_client/merge.c
(is_same_repos, check_same_repos): Implement the stricter checks, returning
an error in the case of different URLs but same UUIDs. Drop the option to
choose "strict" URL comparison.
(svn_client__merge_locked,
open_reintegrate_source_and_target,
merge_peg_locked,
client_find_automatic_merge): Use the stricter checks in all cases. Add
context-specific error messages in all cases.
* subversion/tests/cmdline/merge_tests.py
(merge_error_if_source_urls_differ,
merge_error_if_target_url_differs_and_same_uuid): New tests.
(test_list): Run them.
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
subversion/trunk/subversion/tests/cmdline/merge_tests.py
Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1888474&r1=1888473&r2=1888474&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Wed Apr 7 13:28:00 2021
@@ -440,38 +440,56 @@ check_repos_match(const merge_target_t *
return SVN_NO_ERROR;
}
-/* Return TRUE iff the repository of LOCATION1 is the same as
- * that of LOCATION2. If STRICT_URLS is true, the URLs must
- * match (and the UUIDs, just to be sure), otherwise just the UUIDs must
- * match and the URLs can differ (a common case is http versus https). */
-static svn_boolean_t
-is_same_repos(const svn_client__pathrev_t *location1,
+/* Decide whether LOCATION1 and LOCATION2 point to the same repository
+ * (with the same root URL) or to two different repositories.
+ * - same repository root URL -> set *SAME_REPOS true
+ * - different repositories -> set *SAME_REPOS false
+ * - different URLs but same UUID -> return an error
+ *
+ * The last case is unsupported for practical and historical reasons
+ * (see issue #4874) even though different URLs pointing to the same or
+ * equivalent repositories could be supported in principle.
+ */
+static svn_error_t *
+is_same_repos(svn_boolean_t *same_repos,
+ const svn_client__pathrev_t *location1,
+ const char *path1,
const svn_client__pathrev_t *location2,
- svn_boolean_t strict_urls)
+ const char *path2)
{
- if (strict_urls)
- return (strcmp(location1->repos_root_url, location2->repos_root_url) == 0
- && strcmp(location1->repos_uuid, location2->repos_uuid) == 0);
+ if (strcmp(location1->repos_root_url, location2->repos_root_url) == 0)
+ *same_repos = TRUE;
+ else if (strcmp(location1->repos_uuid, location2->repos_uuid) != 0)
+ *same_repos = FALSE;
else
- return (strcmp(location1->repos_uuid, location2->repos_uuid) == 0);
+ return svn_error_createf(SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
+ _("The locations '%s' and '%s' point to repositories with the "
+ "same repository UUID using different repository root URLs "
+ "('%s' and '%s')"),
+ path1, path2, location1->repos_root_url, location2->repos_root_url);
+ return SVN_NO_ERROR;
}
-/* If the repository identified of LOCATION1 is not the same as that
- * of LOCATION2, throw a SVN_ERR_CLIENT_UNRELATED_RESOURCES
- * error mentioning PATH1 and PATH2. For STRICT_URLS, see is_same_repos().
+/* Check that LOCATION1 and LOCATION2 point to the same repository, with
+ * the same root URL. If not, throw a SVN_ERR_CLIENT_UNRELATED_RESOURCES
+ * error mentioning PATH_OR_URL1 and PATH_OR_URL2.
*/
static svn_error_t *
check_same_repos(const svn_client__pathrev_t *location1,
- const char *path1,
+ const char *path_or_url1,
const svn_client__pathrev_t *location2,
- const char *path2,
- svn_boolean_t strict_urls,
- apr_pool_t *scratch_pool)
+ const char *path_or_url2)
{
- if (! is_same_repos(location1, location2, strict_urls))
+ svn_boolean_t same_repos;
+
+ SVN_ERR(is_same_repos(&same_repos,
+ location1, path_or_url1, location2, path_or_url2));
+ if (! same_repos)
return svn_error_createf(SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
- _("'%s' must be from the same repository as "
- "'%s'"), path1, path2);
+ _("The locations '%s' and '%s' point to different repositories "
+ "(root URLs '%s' and '%s', and differing UUIDs)"),
+ path_or_url1, path_or_url2,
+ location1->repos_root_url, location2->repos_root_url);
return SVN_NO_ERROR;
}
@@ -10531,16 +10549,17 @@ svn_client__merge_locked(svn_client__con
source2, NULL, revision2, revision2, ctx, sesspool));
/* We can't do a diff between different repositories. */
- /* ### We should also insist that the root URLs of the two sources match,
- * as we are only carrying around a single source-repos-root from now
- * on, and URL calculations will go wrong if they differ.
- * Alternatively, teach the code to cope with differing root URLs. */
- SVN_ERR(check_same_repos(source1_loc, source1_loc->url,
- source2_loc, source2_loc->url,
- FALSE /* strict_urls */, scratch_pool));
+ SVN_ERR_W(check_same_repos(source1_loc, source1, source2_loc, source2),
+ _("The repository root URLs of the two sources must be identical "
+ "in a two-URL merge"));
/* Do our working copy and sources come from the same repository? */
- same_repos = is_same_repos(&target->loc, source1_loc, TRUE /* strict_urls */);
+ SVN_ERR_W(is_same_repos(&same_repos,
+ source1_loc, source1, &target->loc, target_abspath),
+ _("The given merge source must have the same repository "
+ "root URL as the target WC, in the usual case; "
+ "or, if a foreign repository merge is intended, the repositories "
+ "must have different root URLs and different UUIDs"));
/* Unless we're ignoring ancestry, see if the two sources are related. */
if (! ignore_mergeinfo)
@@ -11746,13 +11765,12 @@ open_reintegrate_source_and_target(svn_r
/* source_loc and target->loc are required to be in the same repository,
as mergeinfo doesn't come into play for cross-repository merging. */
- SVN_ERR(check_same_repos(source_loc,
- svn_dirent_local_style(source_path_or_url,
- scratch_pool),
- &target->loc,
- svn_dirent_local_style(target->abspath,
- scratch_pool),
- TRUE /* strict_urls */, scratch_pool));
+ SVN_ERR_W(check_same_repos(source_loc, source_path_or_url,
+ &target->loc,
+ svn_dirent_local_style(target->abspath,
+ scratch_pool)),
+ _("The repository root URLs of the source and the target WC "
+ "must be identical in a reintegrate merge"));
*source_loc_p = source_loc;
*target_p = target;
@@ -11914,7 +11932,13 @@ merge_peg_locked(svn_client__conflict_re
scratch_pool, scratch_pool));
/* Check for same_repos. */
- same_repos = is_same_repos(&target->loc, source_loc, TRUE /* strict_urls */);
+ SVN_ERR_W(is_same_repos(&same_repos,
+ source_loc, source_path_or_url,
+ &target->loc, target_abspath),
+ _("The given merge source must have the same repository "
+ "root URL as the target WC, in the usual case; "
+ "or, if a foreign repository merge is intended, the repositories "
+ "must have different root URLs and different UUIDs"));
/* Do the real merge! (We say with confidence that our merge
sources are both ancestral and related.) */
@@ -12702,9 +12726,12 @@ client_find_automatic_merge(automatic_me
ctx, result_pool));
/* Check source is in same repos as target. */
- SVN_ERR(check_same_repos(s_t->source, source_path_or_url,
- &s_t->target->loc, target_abspath,
- TRUE /* strict_urls */, scratch_pool));
+ SVN_ERR_W(check_same_repos(s_t->source, source_path_or_url,
+ &s_t->target->loc,
+ svn_dirent_local_style(target_abspath,
+ scratch_pool)),
+ _("The repository root URLs of the source and the target WC "
+ "must be identical in an automatic merge"));
SVN_ERR(find_automatic_merge(&merge->base, &merge->is_reintegrate_like, s_t,
ctx, result_pool, scratch_pool));
Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1888474&r1=1888473&r2=1888474&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Wed Apr 7 13:28:00 2021
@@ -18658,6 +18658,78 @@ def merge_deleted_folder_with_mergeinfo_
os.chdir(was_cwd)
+#----------------------------------------------------------------------
+# Test that mismatched source repository root URLs in a two-URL merge
+# throws an error.
+# For mismatched URLs we use two repositories with the same UUID.
+@Issue(4874)
+def merge_error_if_source_urls_differ(sbox):
+ "merge error if source urls differ"
+
+ sbox.build()
+ expected_disk, expected_status = set_up_branch(sbox)
+ wc_dir = sbox.wc_dir
+ repo_dir = sbox.repo_dir
+
+ # Create a second repository with the same content, same UUID
+ other_repo_dir, other_repo_url = sbox.add_repo_path("other")
+ other_wc_dir = sbox.add_wc_path("other")
+ svntest.main.copy_repos(repo_dir, other_repo_dir, 6, ignore_uuid=False)
+
+ # With Issue #4874 implemented, the attempted merge should error out with
+ # SVN_ERR_CLIENT_UNRELATED_RESOURCES, because of mismatched source URLs.
+ G_COPY_path = sbox.ospath('A_COPY/D/G')
+ svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ 'merge',
+ sbox.repo_url + '/A/D/G@3',
+ other_repo_url + '/A/D/G@4',
+ G_COPY_path)
+
+#----------------------------------------------------------------------
+# Test that a merge with mismatched source and target repository root URLs
+# throws an error.
+# For mismatched URLs we use two repositories with the same UUID.
+# We test the several entry points to merge.
+@Issue(4874)
+def merge_error_if_target_url_differs_and_same_uuid(sbox):
+ "merge error if target url differs and same uuid"
+
+ sbox.build()
+ expected_disk, expected_status = set_up_branch(sbox)
+ wc_dir = sbox.wc_dir
+ repo_dir = sbox.repo_dir
+
+ # Create a second repository with the same content, same UUID
+ other_repo_dir, other_repo_url = sbox.add_repo_path("other")
+ other_wc_dir = sbox.add_wc_path("other")
+ svntest.main.copy_repos(repo_dir, other_repo_dir, 6, ignore_uuid=False)
+
+ # With Issue #4874 implemented, the attempted merges should error out with
+ # SVN_ERR_CLIENT_UNRELATED_RESOURCES, because of mismatched source and
+ # target URLs.
+ G_COPY_path = sbox.ospath('A_COPY/D/G')
+ # two-URL merge
+ svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ 'merge',
+ other_repo_url + '/A/D/G@3',
+ other_repo_url + '/A/D/G@4',
+ G_COPY_path)
+ # pegged merge
+ svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ 'merge', '-c4',
+ other_repo_url + '/A/D/G',
+ G_COPY_path)
+ # automatic merge
+ svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ 'merge',
+ other_repo_url + '/A/D/G',
+ G_COPY_path)
+ # reintegrate merge
+ svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ 'merge', '--reintegrate',
+ other_repo_url + '/A/D/G',
+ G_COPY_path)
+
########################################################################
# Run the tests
@@ -18807,6 +18879,8 @@ test_list = [ None,
merge_dir_delete_force,
merge_deleted_folder_with_mergeinfo,
merge_deleted_folder_with_mergeinfo_2,
+ merge_error_if_source_urls_differ,
+ merge_error_if_target_url_differs_and_same_uuid,
]
if __name__ == '__main__':