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/20 13:40:28 UTC
svn commit: r1889012 - in /subversion/trunk/subversion: include/svn_wc.h
libsvn_client/merge.c svn/notify.c tests/cmdline/merge_tests.py
Author: julianfoad
Date: Tue Apr 20 13:40:28 2021
New Revision: 1889012
URL: http://svn.apache.org/viewvc?rev=1889012&view=rev
Log:
For issue #4874 "Avoid foreign repository merge if repository UUIDs match"
The initial fix in r1888474 made the ambiguous foreign merge cases
(different URL, same UUID) an error.
This patch makes the ambiguous foreign merge cases a warning in Subversion
1.15, and an error in Subversion >= 1.16.
This patch also restores a 1.14-compatible error code and error message for
the case of mismatched source URLs in a two-URL merge, in addition to the
new error which is more explicit.
Summary of issue #4874 changes since 1.14 up to and including this:
- two-URL or pegged merge, ambiguous foreign repo (different URL, same UUID):
1.14 silently do foreign-repo merge
1.15 warn and do foreign-repo merge
1.16 error
- automatic or reintegrate merge, source-target URL mismatch:
add a more explicit error
- two-URL merge, source URLs mismatch:
add a more explicit error
* subversion/libsvn_client/merge.c
(WITH_AMBIGUOUS_FOREIGN_MERGE_WARNING,
notify_pre_1_16_warning): New.
(is_same_repos,
check_same_repos): Add a caller-supplied message when returning an error.
(svn_client__merge_locked): Add Subversion 1.14 compatibility to the error
when source URLs are mismatched. Let the ambiguous foreign merge case be
a warning in Subversion 1.15.
(merge_peg_locked): Let the ambiguous foreign merge case be a warning in
Subversion 1.15.
(open_reintegrate_source_and_target,
client_find_automatic_merge): Adjust calls.
* subversion/include/svn_wc.h
(svn_wc_notify_action_t): Add a new notification action,
'svn_wc_notify_warning'.
* subversion/svn/notify.c
(notify_body): Handle the new notification: print the warning.
* subversion/tests/cmdline/merge_tests.py
(merge_error_if_source_urls_differ): Look for the 1.14-compatible
part of the error message, to verify compatibility.
(merge_error_if_target_url_differs_and_same_uuid): Split into...
(merge_error_if_ambiguous_foreign_merge,
merge_error_if_source_target_url_mismatch): ... these two parts, and
adjust to expect a warning (exit code 0) rather than an error for
Subversion 1.15.
(test_list): Run them.
Modified:
subversion/trunk/subversion/include/svn_wc.h
subversion/trunk/subversion/libsvn_client/merge.c
subversion/trunk/subversion/svn/notify.c
subversion/trunk/subversion/tests/cmdline/merge_tests.py
Modified: subversion/trunk/subversion/include/svn_wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=1889012&r1=1889011&r2=1889012&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_wc.h (original)
+++ subversion/trunk/subversion/include/svn_wc.h Tue Apr 20 13:40:28 2021
@@ -1303,7 +1303,11 @@ typedef enum svn_wc_notify_action_t
/** Done searching the repository for details about a conflict.
* @since New in 1.10. */
- svn_wc_notify_end_search_tree_conflict_details
+ svn_wc_notify_end_search_tree_conflict_details,
+
+ /** A warning, specified in #svn_wc_notify_t.err.
+ * @since New in 1.15. */
+ svn_wc_notify_warning,
} svn_wc_notify_action_t;
Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1889012&r1=1889011&r2=1889012&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Tue Apr 20 13:40:28 2021
@@ -51,6 +51,7 @@
#include "svn_sorts.h"
#include "svn_subst.h"
#include "svn_ra.h"
+#include "svn_version.h"
#include "client.h"
#include "mergeinfo.h"
@@ -422,6 +423,37 @@ merge_source_dup(const merge_source_t *s
return s;
}
+/* Decide whether ambiguous foreign merge should be a warning or an error */
+#define WITH_AMBIGUOUS_FOREIGN_MERGE_WARNING \
+ (SVN_VER_MAJOR == 1 && SVN_VER_MINOR < 16)
+
+#if WITH_AMBIGUOUS_FOREIGN_MERGE_WARNING
+/* Notify a warning, given in WARNING, if WARNING is non-null.
+ *
+ * We plan to replace this with a hard error in Subversion 1.16.
+ * This clears the error object WARNING before returning.
+ */
+static void
+notify_pre_1_16_warning(svn_error_t *warning,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ if (!warning)
+ return;
+
+ if (ctx->notify_func2)
+ {
+ svn_wc_notify_t *n
+ = svn_wc_create_notify("" /*path*/, svn_wc_notify_warning, pool);
+
+ n->err = svn_error_quick_wrap(warning,
+ _("In Subversion 1.16 this warning will become a fatal error"));
+ ctx->notify_func2(ctx->notify_baton2, n, pool);
+ }
+ svn_error_clear(warning);
+}
+#endif
+
/* Return SVN_ERR_UNSUPPORTED_FEATURE if URL is not inside the repository
of LOCAL_ABSPATH. Use SCRATCH_POOL for temporary allocations. */
static svn_error_t *
@@ -455,18 +487,25 @@ is_same_repos(svn_boolean_t *same_repos,
const svn_client__pathrev_t *location1,
const char *path1,
const svn_client__pathrev_t *location2,
- const char *path2)
+ const char *path2,
+ const char *message)
{
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 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);
+ {
+ svn_error_t *err
+ = svn_error_create(SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL, message);
+
+ return svn_error_quick_wrapf(err,
+ _("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;
}
@@ -478,18 +517,25 @@ static svn_error_t *
check_same_repos(const svn_client__pathrev_t *location1,
const char *path_or_url1,
const svn_client__pathrev_t *location2,
- const char *path_or_url2)
+ const char *path_or_url2,
+ const char *message)
{
svn_boolean_t same_repos;
SVN_ERR(is_same_repos(&same_repos,
- location1, path_or_url1, location2, path_or_url2));
+ location1, path_or_url1, location2, path_or_url2,
+ message));
if (! same_repos)
- return svn_error_createf(SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL,
- _("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);
+ {
+ svn_error_t *err
+ = svn_error_create(SVN_ERR_CLIENT_UNRELATED_RESOURCES, NULL, message);
+
+ return svn_error_quick_wrapf(err,
+ _("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;
}
@@ -10549,17 +10595,34 @@ svn_client__merge_locked(svn_client__con
source2, NULL, revision2, revision2, ctx, sesspool));
/* We can't do a diff between different repositories. */
- 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"));
+ err = 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"));
+ if (err)
+ /* Add an error code and message compatible with Subversion <= 1.14 */
+ return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, err,
+ _("'%s' isn't in the same repository as '%s'"),
+ source1, source2_loc->repos_root_url);
/* Do our working copy and sources come from the same repository? */
- 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"));
+ err = 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"));
+#if WITH_AMBIGUOUS_FOREIGN_MERGE_WARNING
+ if (err)
+ {
+ /* Subversion 1.14 compatibility: do a foreign-repository merge,
+ * even though the repository UUIDs are identical. Issue a warning. */
+ notify_pre_1_16_warning(err, ctx, scratch_pool);
+ same_repos = FALSE;
+ }
+#else
+ SVN_ERR(err);
+#endif
/* Unless we're ignoring ancestry, see if the two sources are related. */
if (! ignore_mergeinfo)
@@ -11765,12 +11828,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_W(check_same_repos(source_loc, source_path_or_url,
- &target->loc,
- svn_dirent_local_style(target->abspath,
- scratch_pool)),
+ SVN_ERR(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"));
+ "must be identical in a reintegrate merge")));
*source_loc_p = source_loc;
*target_p = target;
@@ -11932,13 +11995,24 @@ merge_peg_locked(svn_client__conflict_re
scratch_pool, scratch_pool));
/* Check for same_repos. */
- SVN_ERR_W(is_same_repos(&same_repos,
- source_loc, source_path_or_url,
- &target->loc, target_abspath),
+ err = 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"));
+#if WITH_AMBIGUOUS_FOREIGN_MERGE_WARNING
+ if (err)
+ {
+ /* Subversion 1.14 compatibility: do a foreign-repository merge,
+ * even though the repository UUIDs are identical. Issue a warning. */
+ notify_pre_1_16_warning(err, ctx, scratch_pool);
+ same_repos = FALSE;
+ }
+#else
+ SVN_ERR(err);
+#endif
/* Do the real merge! (We say with confidence that our merge
sources are both ancestral and related.) */
@@ -12726,12 +12800,12 @@ client_find_automatic_merge(automatic_me
ctx, result_pool));
/* Check source is in same repos as target. */
- 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)),
+ SVN_ERR(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"));
+ "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/svn/notify.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/notify.c?rev=1889012&r1=1889011&r2=1889012&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/notify.c (original)
+++ subversion/trunk/subversion/svn/notify.c Tue Apr 20 13:40:28 2021
@@ -1197,6 +1197,12 @@ notify_body(struct notify_baton *nb,
SVN_ERR(svn_cmdline_printf(pool, _("Committing transaction...\n")));
break;
+ case svn_wc_notify_warning:
+ /* using handle_error rather than handle_warning in order to show the
+ * whole error chain; the latter only shows one error in the chain */
+ svn_handle_error2(n->err, stderr, FALSE, "svn: warning: ");
+ break;
+
default:
break;
}
Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1889012&r1=1889011&r2=1889012&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Tue Apr 20 13:40:28 2021
@@ -18660,7 +18660,12 @@ def merge_deleted_folder_with_mergeinfo_
#----------------------------------------------------------------------
# Test that mismatched source repository root URLs in a two-URL merge
-# throws an error.
+# throws an error E170000 SVN_ERR_RA_ILLEGAL_URL.
+#
+# (Since issue 4874 (Subversion 1.15) that error is also wrapped in
+# E195012 SVN_ERR_CLIENT_UNRELATED_RESOURCES for consistency with
+# the source-target mismatch errors.)
+#
# For mismatched URLs we use two repositories with the same UUID.
@Issue(4874)
def merge_error_if_source_urls_differ(sbox):
@@ -18676,10 +18681,8 @@ def merge_error_if_source_urls_differ(sb
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,
+ svntest.actions.run_and_verify_svn2(None, '.*: E170000: .*', 1,
'merge',
sbox.repo_url + '/A/D/G@3',
other_repo_url + '/A/D/G@4',
@@ -18687,12 +18690,16 @@ def merge_error_if_source_urls_differ(sb
#----------------------------------------------------------------------
# Test that a merge with mismatched source and target repository root URLs
-# throws an error.
+# but identical repository UUIDs throws a warning error, for two-URL and
+# pegged merges.
+#
+# Issue #4874 makes this a warning in Subversion 1.15 and an error in 1.16.
+# Previously it was treated as a foreign repository merge.
+#
# 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"
+def merge_error_if_ambiguous_foreign_merge(sbox):
+ "merge error if ambiguous foreign merge"
sbox.build()
expected_disk, expected_status = set_up_branch(sbox)
@@ -18707,28 +18714,61 @@ def merge_error_if_target_url_differs_an
# 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')
+
+ # expect warning or error (E195012 SVN_ERR_CLIENT_UNRELATED_RESOURCES)?
+ expected_stdout = None #if SVN_VER_MINOR < 16 else []
+ expected_stderr = '.*: E195012: .*' #if SVN_VER_MINOR >= 15 else []
+ expected_exit = 0 #if SVN_VER_MINOR < 16 else 1
+
# two-URL merge
- svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ svntest.actions.run_and_verify_svn2(expected_stdout, expected_stderr,
+ expected_exit,
'merge',
other_repo_url + '/A/D/G@3',
other_repo_url + '/A/D/G@4',
- G_COPY_path)
+ sbox.ospath('A_COPY/D/G'))
+ svntest.main.run_svn(False, 'revert', '-qR', sbox.wc_dir)
# pegged merge
- svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ svntest.actions.run_and_verify_svn2(expected_stdout, expected_stderr,
+ expected_exit,
'merge', '-c4',
other_repo_url + '/A/D/G',
- G_COPY_path)
+ sbox.ospath('A_COPY/D/G'))
+ svntest.main.run_svn(False, 'revert', '-qR', sbox.wc_dir)
+
+#----------------------------------------------------------------------
+# Test that a merge with mismatched source and target repository root URLs
+# throws an error, for automatic and reintegrate merges.
+#
+# This behaviour is unchanged by issue #4874, just reinforced and with more
+# informative error messages.
+#
+# For mismatched URLs we use two repositories with the same UUID.
+@Issue(4874)
+def merge_error_if_source_target_url_mismatch(sbox):
+ "merge error if source target url mismatch"
+
+ 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)
+
+ expected_stderr = '.*: E195012: .*' # SVN_ERR_CLIENT_UNRELATED_RESOURCES
# automatic merge
- svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ svntest.actions.run_and_verify_svn2([], expected_stderr, 1,
'merge',
other_repo_url + '/A/D/G',
- G_COPY_path)
+ sbox.ospath('A_COPY/D/G'))
# reintegrate merge
- svntest.actions.run_and_verify_svn2(None, '.*: E195012: .*', 1,
+ svntest.actions.run_and_verify_svn2([], expected_stderr, 1,
'merge', '--reintegrate',
other_repo_url + '/A/D/G',
- G_COPY_path)
+ sbox.ospath('A_COPY/D/G'))
########################################################################
# Run the tests
@@ -18880,7 +18920,8 @@ test_list = [ None,
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,
+ merge_error_if_ambiguous_foreign_merge,
+ merge_error_if_source_target_url_mismatch,
]
if __name__ == '__main__':