Hi Dan,
Find the attached patch and log incorporating your review.
Somehow merge_tests 48 (new testcase added by Paul yesterday, earlier it
was in XFail) is failing with this patch.
>
>> --- subversion/svn/main.c (revision 24527)
>> +++ subversion/svn/main.c (working copy)
>> @@ -513,7 +513,7 @@
>> ("Apply the differences between two sources to a working copy path.\n"
>> "usage: 1. merge sourceURL1[@N] sourceURL2[@M] [WCPATH]\n"
>> " 2. merge sourceWCPATH1@N sourceWCPATH2@M [WCPATH]\n"
>> - " 3. merge [-c M | -r N:M] SOURCE[@REV] [WCPATH]\n"
>> + " 3. merge [-c M | -r N:M] [SOURCE[@REV]] [WCPATH]\n"
>>
>
> I'd expect to this to be:
>
> merge [-c M | -r N:M] [SOURCE[@REV] [WCPATH]]
>
No. It fails to cover the following case.
merge [-c M | -r N:M] MY_WCPATH
According to above syntax MY_WCPATH would be equated with SOURCE which
is not correct.
> ...
>
>> - " 3. In the third form, SOURCE can be a URL, or working copy item\n"
>> - " in which case the corresponding URL is used. This URL in\n"
>> + " 3. In the third form, SOURCE should be a URL if given. \n"
>>
>
> Since SOURCE can be a WCPATH, the above wording needs to be changed.
>
>
I am referring SOURCE in the above context not in a generic sense. All I
mean is SOURCE(If given) should be a absolute URI, If SOURCE is omitted,
we will derive the SOURCE URL from from WCPATH target.
<snip from my original log>
This feature causes conflicts with one use case of existing feature.
Currently revision based ranges can be of the form
"3. merge [-c M | -r N:M] SOURCE[@REV] [WCPATH]\n"
Making SOURCE and REV_RANGE optional in the above command results in the
following type of invocation.
"merge WC_SOURCE[@REV] [WCPATH]\n", this causes the ambiguity with
"2. merge sourceWCPATH1@N sourceWCPATH2@M [WCPATH]\n".
So to fix this we mandate SOURCE in 3rd style of merges to be a URL if
given.
</snip>
I think I should use 'sourceURL' instead of 'SOURCE' to make things less
ambiguous.
>> + " If SOURCE is not given SOURCE URL is deduced from the \n"
>> + " copy source URL of WCPATH. If the WCPATH is not a copy of some \n"
>> + " other node error is thrown asking for URL. This URL in\n"
>>
>
> We should talk about SOURCE here, rather than WCPATH.
>
I believe you meant we should use 'SOURCE' rather than 'URL'.
>
>> " revision REV is compared as it existed between revisions N and \n"
>> " M. If REV is not specified, HEAD is assumed.\n"
>> " The '-c M' option is equivalent to '-r N:M' where N = M-1.\n"
>> " Using '-c -M' does the reverse: '-r M:N' where N = M-1.\n"
>> + " If revision range is not specified, it is assumed to be \n"
>> + " -r oldest_source_node_rev:HEAD.\n"
>>
>
> I don't have a great alternate suggestion, but think that
> "oldest_source_node_rev" should be formatted differently.
>
Changed it to OLDEST_SOURCE_NODE_REV.
> ...
>
>> --- subversion/svn/merge-cmd.c (revision 24527)
>> +++ subversion/svn/merge-cmd.c (working copy)
>> @@ -43,12 +43,36 @@
>> svn_cl__opt_state_t *opt_state = ((svn_cl__cmd_baton_t *) baton)->opt_state;
>> svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
>> apr_array_header_t *targets;
>> - const char *sourcepath1, *sourcepath2, *targetpath;
>> + const char *sourcepath1=NULL, *sourcepath2=NULL;
>> + /* where to apply the diffs, defaulting to '.' */
>> + const char *targetpath="";
>>
>
> Need better use of whitespace. Don't need to put source/target path
> variables on their own line.
>
Placed them on different lines to make a comment on 'targetpath' appear
only closer to targetpath declaration.
> Do we want to default target_path to "" here, or later on?
>
>
Down in the code we have duplicate else blocks to populate the
target_path to "". Instead of multiple duplicate blocks we can have a
default "".
>> svn_boolean_t using_rev_range_syntax = FALSE;
>> svn_error_t *err;
>> - svn_opt_revision_t peg_revision;
>> + svn_opt_revision_t peg_revision, peg_revision1, peg_revision2;
>>
>
> Do we need three peg revision variables? In the
> using_rev_range_syntax block, it looks like the two new ones are only
> temporary.
>
Yes we need this new variables, they are there to avoid multiple
'svn_opt_parse_path' calls on different invocation scenarios.
> ...
>
>> + if (targets->nelts <= 1)
>> + {
>> + using_rev_range_syntax = TRUE;
>> + }
>> + else if (targets->nelts == 2)
>> + {
>> + /* First target should be URL and second target should be WCPATH.*/
>> + if (svn_path_is_url(sourcepath1) && !svn_path_is_url(sourcepath2))
>> + using_rev_range_syntax = TRUE;
>> + }
>>
>
> This "if/else" block could be one big conditional.
>
May be. I felt the above one is easy to understand.
> ...
>
>> + peg_revision = peg_revision1;
>>
>
> I think we can do away with this statement by removing the two new peg
> rev variables.
>
> ...
>
No. We need them as explained above. Thanks for catching I need such
assignments inside !using_rev_range block.
>> + else
>> + {
>> + /* targets->nelts is 1 or 2 here. */
>> + if (targets->nelts == 1)
>> + sourcepath2 = sourcepath1;
>> + /* Set the default peg revision if one was not specified. */
>> + if (peg_revision.kind == svn_opt_revision_unspecified)
>> + peg_revision.kind = svn_path_is_url(sourcepath1)
>> + ? svn_opt_revision_head : svn_opt_revision_working;
>> + }
>> if (targets->nelts == 2)
>> - targetpath = APR_ARRAY_IDX(targets, 1, const char *);
>> - else
>> - targetpath = "";
>> + {
>> + targetpath = APR_ARRAY_IDX(targets, 1, const char *);
>> + }
>>
>
> We can put the setting of targetpath up inside the "else" block. We
> can avoid setting it to "" if that is its default value.
>
Yes I already avoid setting targetpath to "". Moved the 'if
(targets->nelts == 2)' block inside the above else(It could have been
just else, I wanted it be more explicit).
>
>> }
>> else /* using @rev syntax */
>> {
>> @@ -96,14 +119,6 @@
>> return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
>> _("Too many arguments given"));
>>
>> - /* the first two paths become the 'sources' */
>> - SVN_ERR(svn_opt_parse_path(&opt_state->start_revision, &sourcepath1,
>> - APR_ARRAY_IDX(targets, 0, const char *),
>> - pool));
>> - SVN_ERR(svn_opt_parse_path(&opt_state->end_revision, &sourcepath2,
>> - APR_ARRAY_IDX(targets, 1, const char *),
>> - pool));
>>
>
> Hrm. Can we really remove this block? What then will set
> opt_state->start_revision and opt_state->end_revision?
>
> ...
>
Good catch, as mentioned above copied 'opt_state->start_revision' to
peg_revision1 and 'opt_state->end_revision' to 'peg_revision2'.
>> @@ -123,10 +138,9 @@
>> else
>> targetpath = "";
>> }
>> -
>> /* If no targetpath was specified, see if we can infer it from the
>> sourcepaths. */
>> - if (! strcmp(targetpath, ""))
>> + if (sourcepath1 && sourcepath2 && ! strcmp(targetpath, ""))
>>
>
> Prefer the expression "strcmp(...) == 0".
>
done.
>
>> {
>> char *sp1_basename, *sp2_basename;
>> sp1_basename = svn_path_basename(sourcepath1, pool);
>> @@ -144,10 +158,9 @@
>> }
>> }
>>
>> - if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
>> + if ((opt_state->start_revision.kind == svn_opt_revision_unspecified)
>> + && !using_rev_range_syntax)
>> opt_state->start_revision.kind = svn_opt_revision_head;
>> - if (opt_state->end_revision.kind == svn_opt_revision_unspecified)
>> - opt_state->end_revision.kind = svn_opt_revision_head;
>>
>
> I don't think we can get rid of this second check in the
> non-using_rev_range_syntax case, either.
>
> ...
>
Corrected.
>> --- subversion/libsvn_client/client.h (revision 24527)
>> +++ subversion/libsvn_client/client.h (working copy)
>> @@ -887,6 +887,34 @@
>> svn_client_ctx_t *ctx,
>> apr_pool_t *pool);
>>
>
> You probably didn't mean to put these API declarations into the
> "Externals (Modules)" area of client.h. They should probably go into
> a merge or copy-related area.
>
Fixed.
> They're also using Doxygen markup, and formatting inconsistent with
> the rest of the APIs in this header.
>
Fixed.
>
>
>> +/** Retrieve the oldest revision of the node at @a REL_PATH at @a REV
>> + * since it was last copied (if applicable), and store it in @a
>> + * OLDEST_REV.
>> + * If REL_PATH does not exist in that REV, sets *OLDEST_REV to
>> + * SVN_INVALID_REVNUM.
>> + *
>> + */
>> +svn_error_t *
>> +svn_client__oldest_rev_at_path(svn_revnum_t *oldest_rev,
>> + svn_ra_session_t *ra_session,
>> + const char *rel_path,
>> + svn_revnum_t rev,
>> + apr_pool_t *pool);
>> +
>> +
>> +/** Retrieve the copy source the node at @a REL_PATH at @a REV
>>
> ^
> of
>
>
>> + * and store it in @a COPY_SOURCE. If REL_PATH is not a copy or
>> + * non-existent @a REV sets @a COPY_SOURCE to NULL.
>>
>
> This reads a little funky, too.
>
Corrected it. Let me know if it needs some more cleaning.
>
>> + *
>> + */
>> +svn_error_t *
>> +svn_client__get_copy_source(const char **copy_source,
>> + svn_ra_session_t *ra_session,
>> + const char *rel_path,
>> + svn_revnum_t rev,
>> + apr_pool_t *pool);
>>
>
> There's a mailing list thread about these APIs that I need to follow
> up on. We'll probably also want to bring existing merge sources into
> consideration where we use the svn_client__get_copy_source() API
> below.
>
> ...
>
>> --- subversion/libsvn_client/log.c (revision 24527)
>> +++ subversion/libsvn_client/log.c (working copy)
>>
> ...
>
>> +/* A log callback conforming to the svn_log_message_receiver_t
>> + interface for obtaining the last revision of a node at a path and
>> + storing it in *BATON (an svn_revnum_t). */
>> +static svn_error_t *
>> +revnum_receiver(void *baton,
>> + apr_hash_t *changed_paths,
>> + svn_revnum_t revision,
>> + const char *author,
>> + const char *date,
>> + const char *message,
>> + apr_pool_t *pool)
>> +{
>> + *((svn_revnum_t *) baton) = revision;
>> + return SVN_NO_ERROR;
>> +}
>> +
>> +svn_error_t *
>> +svn_client__oldest_rev_at_path(svn_revnum_t *oldest_rev,
>> + svn_ra_session_t *ra_session,
>> + const char *rel_path,
>> + svn_revnum_t rev,
>> + apr_pool_t *pool)
>> +{
>> + svn_error_t *err;
>> + *oldest_rev = SVN_INVALID_REVNUM;
>> + apr_array_header_t *rel_paths = apr_array_make(pool, 1, sizeof(rel_path));
>> + APR_ARRAY_PUSH(rel_paths, const char *) = rel_path;
>> +
>> + /* Trace back in history to find the revision at which this node
>> + was created (copied or added). */
>> + err = svn_ra_get_log(ra_session, rel_paths, 1, rev, 1, FALSE, TRUE,
>> + revnum_receiver, oldest_rev, pool);
>> + if (err && (err->apr_err == SVN_ERR_FS_NOT_FOUND ||
>> + err->apr_err == SVN_ERR_RA_DAV_REQUEST_FAILED))
>> + {
>> + /* A locally-added but uncommitted versioned resource won't
>> + exist in the repository. */
>> + svn_error_clear(err);
>> + err = SVN_NO_ERROR;
>> + }
>> + return err;
>> +}
>> +
>> +struct copy_source_baton
>> +{
>> + const char **copy_source;
>> + apr_pool_t *pool;
>> +};
>>
>
> Doesn't this need a copyfrom_rev field, too? Did you happen to see
> the patch I sent to the dev list on "Fri, 30 Mar 2007", attached to
> the message with "Subject: [RFC] Identifying copy/move sources", which
> supplies a very similar API?
>
>
Sorry, I did not look at it.
Here I don't need copy_from_rev.
>> +/* A log callback conforming to the svn_log_message_receiver_t
>> + interface for obtaining the copy source of a node at a path and
>> + storing it in *BATON (a struct copy_source_baton *). */
>> +static svn_error_t *
>> +copy_source_receiver(void *baton,
>> + apr_hash_t *changed_paths,
>> + svn_revnum_t revision,
>> + const char *author,
>> + const char *date,
>> + const char *message,
>> + apr_pool_t *pool)
>> +{
>> + apr_hash_index_t *hi;
>> + void *val;
>> + const char *copy_source_path;
>> + svn_log_changed_path_t *changed_path;
>> + struct copy_source_baton *copy_source_baton;
>> + copy_source_baton = baton;
>> + /*FIXME: if the rev at which this node is created has few other node
>> + changes too extract only our node. */
>> + for (hi = apr_hash_first(pool, changed_paths); hi; hi = apr_hash_next(hi))
>> + {
>> + apr_hash_this(hi, NULL, NULL, &val);
>> + changed_path = val;
>> + }
>> + copy_source_path = changed_path->copyfrom_path;
>> +
>> + *((char **) copy_source_baton->copy_source) =
>> + apr_pstrdup(copy_source_baton->pool, copy_source_path);
>> + return SVN_NO_ERROR;
>> +}
>>
>
> You probably want to look at my receiver implementation.
>
> ...
>
Will look at it.
>> --- subversion/libsvn_client/merge.c (revision 24527)
>> +++ subversion/libsvn_client/merge.c (working copy)
>>
> ...
>
>> +/*Assumes the default values of REVISION1 and REVISION2 to be
>> + oldest rev at which ra_session's root got created and HEAD
>> + respectively if REVISION1 and REVISION2 are unspecified.
>> + This assumed value is set at ASSUMED_REVISION1 and ASSUMED_REVISION2.
>> + RA_SESSION is used to retrieve the value of current HEAD revision.
>> + POOL is used for temporary allocations.
>> +*/
>>
>
> Doc string wording could be improved a bit. For some reason, this
> implementation strikes me as overly complex, but I can't put my finger
> on how to do it more simply.
>
>
>> +static svn_error_t *
>> +assume_default_rev_range(const svn_opt_revision_t *revision1,
>> + svn_opt_revision_t *assumed_revision1,
>> + const svn_opt_revision_t *revision2,
>> + svn_opt_revision_t *assumed_revision2,
>> + svn_ra_session_t *ra_session,
>> + apr_pool_t *pool)
>> +{
>> + svn_opt_revision_t head_rev_opt;
>> + svn_revnum_t head_revnum = SVN_INVALID_REVNUM;
>> + head_rev_opt.kind = svn_opt_revision_head;
>> + /* Provide reasonable defaults for unspecified revisions. */
>> + if (revision1->kind == svn_opt_revision_unspecified)
>> + {
>> + SVN_ERR(svn_client__get_revision_number(&head_revnum, ra_session,
>> + &head_rev_opt, "", pool));
>> + SVN_ERR(svn_client__oldest_rev_at_path(&assumed_revision1->value.number,
>> + ra_session, "",
>> + head_revnum,
>> + pool));
>> + if (assumed_revision1->value.number != SVN_INVALID_REVNUM)
>>
>
> The SVN_IS_VALID_REVNUM() macro would be more appropriate here.
>
>
Fixed.
>> + {
>> + assumed_revision1->kind = svn_opt_revision_number;
>> + }
>> + }
>> + else
>> + {
>> + *assumed_revision1 = *revision1;
>> + }
>> + if (revision2->kind == svn_opt_revision_unspecified)
>> + {
>> + if (head_revnum == SVN_INVALID_REVNUM)
>>
>
> ...and here (reversing the blocks).
>
I could not get you here.
>
>> + assumed_revision2->kind = svn_opt_revision_head;
>> + else
>> + {
>> + assumed_revision2->value.number = head_revnum;
>> + assumed_revision2->kind = svn_opt_revision_number;
>> + }
>> + }
>> + else
>> + {
>> + *assumed_revision2 = *revision2;
>> + }
>> + return SVN_NO_ERROR;
>>
> ...
>
>> @@ -1709,17 +1771,30 @@
>> svn_opt_revision_t *revision1, *revision2;
>> svn_error_t *err;
>> svn_merge_range_t range;
>> - svn_ra_session_t *ra_session1, *ra_session2;
>> + svn_ra_session_t *ra_session, *ra_session1, *ra_session2;
>>
>
> Rather than introducing a third ra_session, can't we just re-parent
> ra_session1 after using it to determine the assumed revision range?
>
> ...
>
Fixed.
>
>> + /* Establish first RA session to URL1. */
>> + /*FIXME: use initial_URL1 for now which may barf for pegged rev merges */
>>
>
> What are these FIXME's all about? Can you give more context?
>
I was not sure how things would behave if URL given to
'svn_client__open_ra_session_internal' if of form
scheme://hostname/path/to/repo/subdir_at_173. So I put a FIXME marker.
>
>> + SVN_ERR(svn_client__open_ra_session_internal(&ra_session, initial_URL1, NULL,
>> + NULL, NULL, FALSE, TRUE,
>> + ctx, pool));
>> + SVN_ERR(assume_default_rev_range(initial_revision1,
>> + &assumed_initial_revision1,
>> + initial_revision2,
>> + &assumed_initial_revision2,
>> + ra_session,
>> + pool));
>>
> ...
>
>> @@ -2226,22 +2301,51 @@
>> svn_wc_adm_access_t *adm_access;
>> const svn_wc_entry_t *entry;
>> struct merge_cmd_baton merge_cmd_baton;
>> - const char *URL;
>> + const char *URL = NULL;
>>
>
> There's only one spot where we use this default value, so let's set it
> there instead.
>
> ...
>
Fixed.
>
>> + if (source)
>> + {
>> + /* If source is a path, we need to get the underlying URL
>> + * from the wc and save the initial path we were passed so we can use it as
>> + * a path parameter (either in the baton or not). otherwise, the path
>> + * will just be NULL, which means we won't be able to figure out some kind
>> + * of revision specifications, but in that case it won't matter, because
>> + * those ways of specifying a revision are meaningless for a url.
>> + */
>> + SVN_ERR(svn_client_url_from_path(&URL, source, pool));
>> + }
>> + else
>> + {
>> + /* If a merge source was not specified, try to derive it from
>> + copy source. */
>> + svn_ra_session_t *ra_session;
>> + svn_revnum_t working_rev_num;
>> + svn_opt_revision_t working_copy_rev_num_opt;
>> + working_copy_rev_num_opt.kind = svn_opt_revision_working;
>> + const char *working_copy_url;
>> + const char *repos_root;
>> + const char *copy_source_path;
>> + SVN_ERR(svn_client__ra_session_from_path(&ra_session,
>> + &working_rev_num,
>> + &working_copy_url,
>> + target_wcpath,
>> + &working_copy_rev_num_opt,
>> + &working_copy_rev_num_opt,
>>
>
> We want to use this same working_copy_rev_num_opt variable twice?
>
Yes. Idea is to feed rWORKING for both peg_revision and start_revision.
>
>> + ctx,
>> + pool));
>> + SVN_ERR(svn_ra_get_repos_root(ra_session, &repos_root, pool));
>> + SVN_ERR(svn_client__get_copy_source(©_source_path, ra_session,
>> + "", working_rev_num, pool));
>> + if (copy_source_path)
>>
>
> We can reduce the scope of repos_root to this block.
>
Fixed.
>
>> + URL = apr_pstrcat(pool, repos_root, copy_source_path, NULL);
>>
>
> else
> URL = NULL;
>
>
>> + }
>> +
>> if (! URL)
>> return svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL,
>> - _("'%s' has no URL"),
>> - svn_path_local_style(source, pool));
>> + _("'%s' has no implicit URL to merge from"),
>> + source ? svn_path_local_style(source, pool) : ".");
>>
>
> Hmm. It seems like we could have either the old problem (mising URL
> field in entry file), or the new problem (unable to automatically
> determine a merge source URL). In the latter case, this isn't the
> appropriate error code. We should use two separate error creation
> statement here, one in the "if" block and one in the "else" block.
>
> ...
>
Fixed.
>
>> --- subversion/tests/cmdline/merge_tests.py (revision 24527)
>> +++ subversion/tests/cmdline/merge_tests.py (working copy)
>> @@ -1243,7 +1243,7 @@
>> def merge_with_implicit_target_helper(sbox, arg_flav):
>> "ARG_FLAV is one of 'r' (revision range) or 'c' (single change)."
>>
>> - if arg_flav not in ('r', 'c'):
>> + if arg_flav not in ('r', 'c', '*'):
>> raise svntest.Failure("Unrecognized flavor of merge argument")
>>
>> sbox.build()
>> @@ -1294,9 +1294,15 @@
>> svntest.actions.run_and_verify_svn(None, ['U mu\n'], [],
>> 'merge', '-c', '-2', mu_url)
>>
>> + #implicit rev and url detection is for forward merge only.
>> + elif arg_flav == '*':
>> + svntest.actions.run_and_verify_svn(None, ['U mu\n'], [],
>> + 'merge', '-c', '-2', mu_url)
>>
>
> We should document this in the command-line client. In the test case,
> we should skip the '*' flavor, since we're already have tested it for
> the 'r' and 'c' flavors.
>
Fixed the command line help text.
Here we should undo the forward merge of r2 to continue with rest of the
testcase, instead of reverting, I would prefer merge -c -2 for now.
Later would see if possible to make it something like this(implicitly
detecting the source atleast).
+ elif arg_flav == '*':
+ svntest.actions.run_and_verify_svn(None, ['U mu\n'], [],
+ 'merge', '-c', '-2')
I did not explore this yet.
>
>> # sanity-check resulting file
>> if (svntest.tree.get_text('mu') != orig_mu_text):
>> - raise svntest.Failure("Unexpected text in 'mu'")
>> + raise svntest.Failure("Unexpected text '%s' in 'mu', expected '%s'" %
>> + (svntest.tree.get_text('mu'), orig_mu_text))
>>
>> # merge using filename for sourcepath
>> # Cannot use run_and_verify_merge with a file target
>> @@ -1306,6 +1312,9 @@
>> elif arg_flav == 'c':
>> svntest.actions.run_and_verify_svn(None, ['G mu\n'], [],
>> 'merge', '-c', '2', 'mu')
>> + elif arg_flav == '*':
>> + svntest.actions.run_and_verify_svn(None, ['G mu\n'], [],
>> + 'merge', 'mu')
>>
>> # sanity-check resulting file
>> if (svntest.tree.get_text('mu') != orig_mu_text + added_mu_text):
>> @@ -1322,6 +1331,11 @@
>> "merging a file w/no explicit target path using -c"
>> merge_with_implicit_target_helper(sbox, 'c')
>>
>> +def merge_with_implicit_target_and_revs(sbox):
>> + "merging a file w/no explicit target path or revs"
>> + merge_with_implicit_target_helper(sbox, '*')
>> +
>> +
>> #----------------------------------------------------------------------
>>
>> def merge_with_prev (sbox):
>> @@ -5370,6 +5384,7 @@
>> simple_property_merges,
>> merge_with_implicit_target_using_r,
>> merge_with_implicit_target_using_c,
>> + merge_with_implicit_target_and_revs,
>>
>
> I seem to recall a second existing test case which could house this
> type of regression testing...merge_one_file_helper() it is. An
> additional consumer -- a la merge_one_file_using_r() and
> merge_one_file_using_c() -- could be added. (I didn't do this.)
>
Will write such a test later.
Thanks
With regards
Kamesh Jayachandran
[[[
'merge' subcommand should automatically identify the source from which to
merge and the applicable revisions to merge for 'revision based merges'.
This feature causes conflicts with one use case of existing feature.
Currently revision based ranges can be of the form
"3. merge [-c M | -r N:M] SOURCE[@REV] [WCPATH]\n"
Making SOURCE and REV_RANGE optional in the above command results in the
following type of invocation.
"merge WC_SOURCE[@REV] [WCPATH]\n", this causes the ambiguity with
"2. merge sourceWCPATH1@N sourceWCPATH2@M [WCPATH]\n".
So to fix this we mandate SOURCE in 3rd style of merges to be a URL if given.
* subversion/svn/main.c
(svn_cl__cmd_table):
Update help text for merge subcommand.
* subversion/svn/merge-cmd.c
(svn_cl__merge):
Implementing the above change.
* subversion/libsvn_client/client.h
(svn_client__oldest_rev_at_path): Prototype the new function.
(svn_client__get_copy_source): Prototype the new function.
* subversion/libsvn_client/copy.c
(revnum_receiver): Moved to subversion/libsvn_client/log.c.
(get_implied_merge_info): Implementation to find the rev at which a
given path is created has been moved to 'svn_client__oldest_rev_at_path'.
This function calls 'svn_client__oldest_rev_at_path'.
* subversion/libsvn_client/log.c
(revnum_receiver): Moved from subversion/libsvn_client/copy.c.
(svn_client__oldest_rev_at_path): New function.
(struct copy_source_baton): New baton to hold the copy_source_path.
(copy_source_receiver): New function.
(svn_client__get_copy_source): New function.
* subversion/libsvn_client/merge.c
(assume_default_rev_range): New function.
(do_merge, do_single_file_merge): Deduce the revison range to merge in case
not provided.
(svn_client_merge_peg3): Deduce source if not given. Fix the error message
for 'undeterminable source URL'
* subversion/include/svn_error_codes.h
(SVN_ERR_CLIENT_PATH_HAS_NO_COPY_SOURCE): define new error code if merge
source could not be deduced.
* subversion/tests/cmdline/merge_tests.py
(merge_with_implicit_target_helper):
Support implicit merge URL and revisions feature like other 2 variants.
(merge_with_implicit_target_and_revs): New function.
(test_list): include 'merge_with_implicit_target_and_revs' test case.
Patch by: kameshj
dlr
]]]
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 24750)
+++ subversion/svn/main.c (working copy)
@@ -513,7 +513,7 @@
("Apply the differences between two sources to a working copy path.\n"
"usage: 1. merge sourceURL1[@N] sourceURL2[@M] [WCPATH]\n"
" 2. merge sourceWCPATH1@N sourceWCPATH2@M [WCPATH]\n"
- " 3. merge [-c M | -r N:M] SOURCE[@REV] [WCPATH]\n"
+ " 3. merge [-c M | -r N:M] [SOURCEURL[@REV]] [WCPATH]\n"
"\n"
" 1. In the first form, the source URLs are specified at revisions\n"
" N and M. These are the two sources to be compared. The revisions\n"
@@ -523,12 +523,15 @@
" copy paths define the sources to be compared. The revisions must\n"
" be specified.\n"
"\n"
- " 3. In the third form, SOURCE can be a URL, or working copy item\n"
- " in which case the corresponding URL is used. This URL in\n"
- " revision REV is compared as it existed between revisions N and \n"
+ " 3. If SOURCEURL is not given it is deduced from the \n"
+ " copy source URL of WCPATH. If the WCPATH is not a copy of some \n"
+ " other node error is thrown asking for SOURCEURL. This SOURCEURL \n"
+ " in revision REV is compared as it existed between revisions N and \n"
" M. If REV is not specified, HEAD is assumed.\n"
" The '-c M' option is equivalent to '-r N:M' where N = M-1.\n"
" Using '-c -M' does the reverse: '-r M:N' where N = M-1.\n"
+ " If revision range is not specified, it is assumed to be \n"
+ " -r OLDEST_SOURCE_NODE_REV:HEAD, which is a forward merge.\n"
"\n"
" WCPATH is the working copy path that will receive the changes.\n"
" If WCPATH is omitted, a default value of '.' is assumed, unless\n"
Index: subversion/svn/merge-cmd.c
===================================================================
--- subversion/svn/merge-cmd.c (revision 24750)
+++ subversion/svn/merge-cmd.c (working copy)
@@ -43,12 +43,36 @@
svn_cl__opt_state_t *opt_state = ((svn_cl__cmd_baton_t *) baton)->opt_state;
svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
apr_array_header_t *targets;
- const char *sourcepath1, *sourcepath2, *targetpath;
+ const char *sourcepath1=NULL, *sourcepath2=NULL;
+ /* where to apply the diffs, defaulting to '.' */
+ const char *targetpath="";
svn_boolean_t using_rev_range_syntax = FALSE;
svn_error_t *err;
- svn_opt_revision_t peg_revision;
+ svn_opt_revision_t peg_revision, peg_revision1, peg_revision2;
apr_array_header_t *options;
+ SVN_ERR(svn_opt_args_to_target_array2(&targets, os,
+ opt_state->targets, pool));
+ if (targets->nelts >= 1)
+ SVN_ERR(svn_opt_parse_path(&peg_revision1, &sourcepath1,
+ APR_ARRAY_IDX(targets, 0, const char *),
+ pool));
+ if (targets->nelts >= 2)
+ SVN_ERR(svn_opt_parse_path(&peg_revision2, &sourcepath2,
+ APR_ARRAY_IDX(targets, 1, const char *),
+ pool));
+
+ if (targets->nelts <= 1)
+ {
+ using_rev_range_syntax = TRUE;
+ }
+ else if (targets->nelts == 2)
+ {
+ /* First target should be URL and second target should be WCPATH.*/
+ if (svn_path_is_url(sourcepath1) && !svn_path_is_url(sourcepath2))
+ using_rev_range_syntax = TRUE;
+ }
+
/* If the first opt_state revision is filled in at this point, then
we know the user must have used the '-r' or '-c' switch. */
if (opt_state->start_revision.kind != svn_opt_revision_unspecified)
@@ -60,33 +84,29 @@
using_rev_range_syntax = TRUE;
}
-
- SVN_ERR(svn_opt_args_to_target_array2(&targets, os,
- opt_state->targets, pool));
-
if (using_rev_range_syntax)
{
- if (targets->nelts < 1)
- return svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, NULL, NULL);
if (targets->nelts > 2)
return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
_("Too many arguments given"));
-
- SVN_ERR(svn_opt_parse_path(&peg_revision, &sourcepath1,
- APR_ARRAY_IDX(targets, 0, const char *),
- pool));
- sourcepath2 = sourcepath1;
-
- /* Set the default peg revision if one was not specified. */
- if (peg_revision.kind == svn_opt_revision_unspecified)
- peg_revision.kind = svn_path_is_url(sourcepath1)
- ? svn_opt_revision_head : svn_opt_revision_working;
-
- /* decide where to apply the diffs, defaulting to '.' */
- if (targets->nelts == 2)
- targetpath = APR_ARRAY_IDX(targets, 1, const char *);
+ peg_revision = peg_revision1;
+ if (targets->nelts == 0)
+ {
+ /* Set the default peg revision if one was not specified. */
+ if (peg_revision.kind == svn_opt_revision_unspecified)
+ peg_revision.kind = svn_opt_revision_head;
+ }
else
- targetpath = "";
+ {
+ /* targets->nelts is 1 or 2 here. */
+ sourcepath2 = sourcepath1;
+ if (targets->nelts == 2)
+ targetpath = APR_ARRAY_IDX(targets, 1, const char *);
+ /* Set the default peg revision if one was not specified. */
+ if (peg_revision.kind == svn_opt_revision_unspecified)
+ peg_revision.kind = svn_path_is_url(sourcepath1)
+ ? svn_opt_revision_head : svn_opt_revision_working;
+ }
}
else /* using @rev syntax */
{
@@ -96,14 +116,9 @@
return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
_("Too many arguments given"));
- /* the first two paths become the 'sources' */
- SVN_ERR(svn_opt_parse_path(&opt_state->start_revision, &sourcepath1,
- APR_ARRAY_IDX(targets, 0, const char *),
- pool));
- SVN_ERR(svn_opt_parse_path(&opt_state->end_revision, &sourcepath2,
- APR_ARRAY_IDX(targets, 1, const char *),
- pool));
-
+ opt_state->start_revision = peg_revision1;
+ opt_state->end_revision = peg_revision2;
+
/* Catch 'svn merge wc_path1 wc_path2 [target]' without explicit
revisions--since it ignores local modifications it may not do what
the user expects. Forcing the user to specify a repository
@@ -120,13 +135,10 @@
/* decide where to apply the diffs, defaulting to '.' */
if (targets->nelts == 3)
targetpath = APR_ARRAY_IDX(targets, 2, const char *);
- else
- targetpath = "";
}
-
/* If no targetpath was specified, see if we can infer it from the
sourcepaths. */
- if (! strcmp(targetpath, ""))
+ if (sourcepath1 && sourcepath2 && (strcmp(targetpath, "") == 0))
{
char *sp1_basename, *sp2_basename;
sp1_basename = svn_path_basename(sourcepath1, pool);
@@ -144,11 +156,15 @@
}
}
- if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
+ if ((opt_state->start_revision.kind == svn_opt_revision_unspecified)
+ && !using_rev_range_syntax)
opt_state->start_revision.kind = svn_opt_revision_head;
- if (opt_state->end_revision.kind == svn_opt_revision_unspecified)
+
+ if ((opt_state->end_revision.kind == svn_opt_revision_unspecified)
+ && !using_rev_range_syntax)
opt_state->end_revision.kind = svn_opt_revision_head;
+
if (! opt_state->quiet)
svn_cl__get_notifier(&ctx->notify_func2, &ctx->notify_baton2, FALSE,
FALSE, FALSE, pool);
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 24750)
+++ subversion/libsvn_client/client.h (working copy)
@@ -209,6 +209,34 @@
svn_boolean_t recurse, svn_client_ctx_t *ctx,
apr_pool_t *pool);
+/** Retrieve the *OLDEST_REV of the node at REL_PATH at REV
+ * since it was last copied (if applicable).
+ * RA_SESSION is used to retrieve this information.
+ * If REL_PATH does not exist in that REV, sets *OLDEST_REV to
+ * SVN_INVALID_REVNUM. POOL is used for all temporary allocations.
+ *
+ */
+svn_error_t *
+svn_client__oldest_rev_at_path(svn_revnum_t *oldest_rev,
+ svn_ra_session_t *ra_session,
+ const char *rel_path,
+ svn_revnum_t rev,
+ apr_pool_t *pool);
+
+
+/** Retrieve the copy source of the REL_PATH at REV
+ * and store it in *COPY_SOURCE. RA_SESSION is used to retrieve this
+ * information. If REL_PATH is non-existent at REV or not a copy of some
+ * node, sets *COPY_SOURCE to NULL. POOL is used for all temporary allocations.
+ *
+ */
+svn_error_t *
+svn_client__get_copy_source(const char **copy_source,
+ svn_ra_session_t *ra_session,
+ const char *rel_path,
+ svn_revnum_t rev,
+ apr_pool_t *pool);
+
/* ---------------------------------------------------------------- */
/*** RA callbacks ***/
@@ -857,6 +885,7 @@
svn_client_ctx_t *ctx,
apr_pool_t *pool);
+
#ifdef __cplusplus
}
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 24750)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -351,22 +351,6 @@
svn_boolean_t is_move;
};
-/* A log callback conforming to the svn_log_message_receiver_t
- interface for obtaining the last revision of a node at a path and
- storing it in *BATON (an svn_revnum_t). */
-static svn_error_t *
-revnum_receiver(void *baton,
- apr_hash_t *changed_paths,
- svn_revnum_t revision,
- const char *author,
- const char *date,
- const char *message,
- apr_pool_t *pool)
-{
- *((svn_revnum_t *) baton) = revision;
- return SVN_NO_ERROR;
-}
-
/* Obtain the implied merge info of repository-relative path PATH in
*IMPLIED_MERGEINFO (e.g. every revision of the node at PATH since
it last appeared). REL_PATH corresponds to PATH, but is relative
@@ -379,33 +363,17 @@
svn_revnum_t rev,
apr_pool_t *pool)
{
- svn_error_t *err;
- svn_revnum_t oldest_rev = SVN_INVALID_REVNUM;
+ svn_revnum_t oldest_rev;
svn_merge_range_t *range;
apr_array_header_t *rangelist;
- apr_array_header_t *rel_paths = apr_array_make(pool, 1, sizeof(rel_path));
*implied_mergeinfo = apr_hash_make(pool);
- APR_ARRAY_PUSH(rel_paths, const char *) = rel_path;
- /* Trace back in history to find the revision at which this node
- was created (copied or added). */
- err = svn_ra_get_log(ra_session, rel_paths, 1, rev, 1, FALSE, TRUE,
- revnum_receiver, &oldest_rev, pool);
- if (err)
- {
- if (err->apr_err == SVN_ERR_FS_NOT_FOUND ||
- err->apr_err == SVN_ERR_RA_DAV_REQUEST_FAILED)
- {
- /* A locally-added but uncommitted versioned resource won't
- exist in the repository. */
- svn_error_clear(err);
- err = SVN_NO_ERROR;
- }
+ SVN_ERR(svn_client__oldest_rev_at_path(&oldest_rev, ra_session, rel_path,
+ rev, pool));
+ if (oldest_rev == SVN_INVALID_REVNUM)
+ return SVN_NO_ERROR;
- return err;
- }
-
range = apr_palloc(pool, sizeof(*range));
range->start = oldest_rev;
range->end = rev;
@@ -413,7 +381,7 @@
APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = range;
apr_hash_set(*implied_mergeinfo, path, APR_HASH_KEY_STRING, rangelist);
- return err;
+ return SVN_NO_ERROR;
}
/* Obtain the implied merge info and the existing merge info of the
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 24750)
+++ subversion/libsvn_client/log.c (working copy)
@@ -371,3 +371,114 @@
return err;
}
+
+/* A log callback conforming to the svn_log_message_receiver_t
+ interface for obtaining the last revision of a node at a path and
+ storing it in *BATON (an svn_revnum_t). */
+static svn_error_t *
+revnum_receiver(void *baton,
+ apr_hash_t *changed_paths,
+ svn_revnum_t revision,
+ const char *author,
+ const char *date,
+ const char *message,
+ apr_pool_t *pool)
+{
+ *((svn_revnum_t *) baton) = revision;
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__oldest_rev_at_path(svn_revnum_t *oldest_rev,
+ svn_ra_session_t *ra_session,
+ const char *rel_path,
+ svn_revnum_t rev,
+ apr_pool_t *pool)
+{
+ svn_error_t *err;
+ *oldest_rev = SVN_INVALID_REVNUM;
+ apr_array_header_t *rel_paths = apr_array_make(pool, 1, sizeof(rel_path));
+ APR_ARRAY_PUSH(rel_paths, const char *) = rel_path;
+
+ /* Trace back in history to find the revision at which this node
+ was created (copied or added). */
+ err = svn_ra_get_log(ra_session, rel_paths, 1, rev, 1, FALSE, TRUE,
+ revnum_receiver, oldest_rev, pool);
+ if (err && (err->apr_err == SVN_ERR_FS_NOT_FOUND ||
+ err->apr_err == SVN_ERR_RA_DAV_REQUEST_FAILED))
+ {
+ /* A locally-added but uncommitted versioned resource won't
+ exist in the repository. */
+ svn_error_clear(err);
+ err = SVN_NO_ERROR;
+ }
+ return err;
+}
+
+struct copy_source_baton
+{
+ const char **copy_source;
+ apr_pool_t *pool;
+};
+
+/* A log callback conforming to the svn_log_message_receiver_t
+ interface for obtaining the copy source of a node at a path and
+ storing it in *BATON (a struct copy_source_baton *). */
+static svn_error_t *
+copy_source_receiver(void *baton,
+ apr_hash_t *changed_paths,
+ svn_revnum_t revision,
+ const char *author,
+ const char *date,
+ const char *message,
+ apr_pool_t *pool)
+{
+ apr_hash_index_t *hi;
+ void *val;
+ const char *copy_source_path;
+ svn_log_changed_path_t *changed_path;
+ struct copy_source_baton *copy_source_baton;
+ copy_source_baton = baton;
+ /*FIXME: if the rev at which this node is created has few other node
+ changes too extract only our node. */
+ for (hi = apr_hash_first(pool, changed_paths); hi; hi = apr_hash_next(hi))
+ {
+ apr_hash_this(hi, NULL, NULL, &val);
+ changed_path = val;
+ }
+ copy_source_path = changed_path->copyfrom_path;
+
+ *((char **) copy_source_baton->copy_source) =
+ apr_pstrdup(copy_source_baton->pool, copy_source_path);
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__get_copy_source(const char **copy_source,
+ svn_ra_session_t *ra_session,
+ const char *rel_path,
+ svn_revnum_t rev,
+ apr_pool_t *pool)
+{
+ svn_error_t *err;
+ *copy_source = NULL;
+ struct copy_source_baton copy_source_baton;
+ copy_source_baton.copy_source = copy_source;
+ copy_source_baton.pool = pool;
+ apr_array_header_t *rel_paths = apr_array_make(pool, 1, sizeof(rel_path));
+ APR_ARRAY_PUSH(rel_paths, const char *) = rel_path;
+
+ /* Trace back in history to find the revision at which this node
+ was created (copied or added). */
+ err = svn_ra_get_log(ra_session, rel_paths, 1, rev, 1, TRUE, TRUE,
+ copy_source_receiver, ©_source_baton, pool);
+ if (err && (err->apr_err == SVN_ERR_FS_NOT_FOUND ||
+ err->apr_err == SVN_ERR_RA_DAV_REQUEST_FAILED))
+ {
+ /* A locally-added but uncommitted versioned resource won't
+ exist in the repository. */
+ svn_error_clear(err);
+ err = SVN_NO_ERROR;
+ }
+ return err;
+}
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 24750)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -1604,6 +1604,59 @@
return SVN_NO_ERROR;
}
+/*Assumes the default values of REVISION1 and REVISION2 to be
+ oldest rev at which ra_session's root got created and HEAD
+ respectively if REVISION1 and REVISION2 are unspecified.
+ This assumed value is set at ASSUMED_REVISION1 and ASSUMED_REVISION2.
+ RA_SESSION is used to retrieve the value of current HEAD revision.
+ POOL is used for temporary allocations.
+*/
+static svn_error_t *
+assume_default_rev_range(const svn_opt_revision_t *revision1,
+ svn_opt_revision_t *assumed_revision1,
+ const svn_opt_revision_t *revision2,
+ svn_opt_revision_t *assumed_revision2,
+ svn_ra_session_t *ra_session,
+ apr_pool_t *pool)
+{
+ svn_opt_revision_t head_rev_opt;
+ svn_revnum_t head_revnum = SVN_INVALID_REVNUM;
+ head_rev_opt.kind = svn_opt_revision_head;
+ /* Provide reasonable defaults for unspecified revisions. */
+ if (revision1->kind == svn_opt_revision_unspecified)
+ {
+ SVN_ERR(svn_client__get_revision_number(&head_revnum, ra_session,
+ &head_rev_opt, "", pool));
+ SVN_ERR(svn_client__oldest_rev_at_path(&assumed_revision1->value.number,
+ ra_session, "",
+ head_revnum,
+ pool));
+ if (SVN_IS_VALID_REVNUM(assumed_revision1->value.number))
+ {
+ assumed_revision1->kind = svn_opt_revision_number;
+ }
+ }
+ else
+ {
+ *assumed_revision1 = *revision1;
+ }
+ if (revision2->kind == svn_opt_revision_unspecified)
+ {
+ if (head_revnum == SVN_INVALID_REVNUM)
+ assumed_revision2->kind = svn_opt_revision_head;
+ else
+ {
+ assumed_revision2->value.number = head_revnum;
+ assumed_revision2->kind = svn_opt_revision_number;
+ }
+ }
+ else
+ {
+ *assumed_revision2 = *revision2;
+ }
+ return SVN_NO_ERROR;
+}
+
/* URL1/PATH1, URL2/PATH2, and TARGET_WCPATH all better be
directories. For the single file case, the caller does the merging
manually. PATH1 and PATH2 can be NULL.
@@ -1658,11 +1711,25 @@
const svn_wc_entry_t *entry;
int i;
svn_boolean_t inherited;
+ svn_opt_revision_t assumed_initial_revision1, assumed_initial_revision2;
apr_size_t target_count, merge_target_count;
- ENSURE_VALID_REVISION_KINDS(initial_revision1->kind,
- initial_revision2->kind);
+ /* Establish first RA session to initial_URL1. */
+ /* FIXME: use initial_URL1 for now which may barf for pegged rev merges */
+ SVN_ERR(svn_client__open_ra_session_internal(&ra_session, initial_URL1, NULL,
+ NULL, NULL, FALSE, TRUE,
+ ctx, pool));
+ SVN_ERR(assume_default_rev_range(initial_revision1,
+ &assumed_initial_revision1,
+ initial_revision2,
+ &assumed_initial_revision2,
+ ra_session,
+ pool));
+ ENSURE_VALID_REVISION_KINDS(assumed_initial_revision1.kind,
+ assumed_initial_revision2.kind);
+
+
/* If we are performing a pegged merge, we need to find out what our
actual URLs will be. */
if (peg_revision->kind != svn_opt_revision_unspecified)
@@ -1673,8 +1740,8 @@
initial_path2 ? initial_path2
: initial_URL2,
peg_revision,
- initial_revision1,
- initial_revision2,
+ &assumed_initial_revision1,
+ &assumed_initial_revision2,
ctx, pool));
merge_b->url = URL2;
@@ -1689,15 +1756,11 @@
path1 = initial_path1;
path2 = initial_path2;
revision1 = apr_pcalloc(pool, sizeof(*revision1));
- *revision1 = *initial_revision1;
+ *revision1 = assumed_initial_revision1;
revision2 = apr_pcalloc(pool, sizeof(*revision2));
- *revision2 = *initial_revision2;
+ *revision2 = assumed_initial_revision2;
}
- /* Establish first RA session to URL1. */
- SVN_ERR(svn_client__open_ra_session_internal(&ra_session, URL1, NULL,
- NULL, NULL, FALSE, TRUE,
- ctx, pool));
notify_b.same_urls = (strcmp(URL1, URL2) == 0);
if (!notify_b.same_urls && merge_b->record_only)
@@ -1988,10 +2051,23 @@
int i;
svn_boolean_t inherited = FALSE;
apr_size_t target_count, merge_target_count;
+ svn_opt_revision_t assumed_initial_revision1, assumed_initial_revision2;
- ENSURE_VALID_REVISION_KINDS(initial_revision1->kind,
- initial_revision2->kind);
+ /* Establish first RA session to URL1. */
+ /*FIXME: use initial_URL1 for now which may barf for pegged rev merges */
+ SVN_ERR(svn_client__open_ra_session_internal(&ra_session1, initial_URL1, NULL,
+ NULL, NULL, FALSE, TRUE,
+ ctx, pool));
+ SVN_ERR(assume_default_rev_range(initial_revision1,
+ &assumed_initial_revision1,
+ initial_revision2,
+ &assumed_initial_revision2,
+ ra_session1,
+ pool));
+ ENSURE_VALID_REVISION_KINDS(assumed_initial_revision1.kind,
+ assumed_initial_revision2.kind);
+
/* If we are performing a pegged merge, we need to find out what our
actual URLs will be. */
if (peg_revision->kind != svn_opt_revision_unspecified)
@@ -2007,8 +2083,8 @@
initial_path2 ? initial_path2
: initial_URL2,
peg_revision,
- initial_revision1,
- initial_revision2,
+ &assumed_initial_revision1,
+ &assumed_initial_revision2,
ctx, pool);
if (err)
{
@@ -2032,15 +2108,14 @@
path1 = initial_path1;
path2 = initial_path2;
revision1 = apr_pcalloc(pool, sizeof(*revision1));
- *revision1 = *initial_revision1;
+ *revision1 = assumed_initial_revision1;
revision2 = apr_pcalloc(pool, sizeof(*revision2));
- *revision2 = *initial_revision2;
+ *revision2 = assumed_initial_revision2;
}
- /* Establish RA sessions to both URLs. */
- SVN_ERR(svn_client__open_ra_session_internal(&ra_session1, URL1, NULL,
- NULL, NULL, FALSE, TRUE,
- ctx, pool));
+ /* reparent RA session to URL1. */
+ SVN_ERR(svn_ra_reparent(ra_session1, URL1, pool));
+ /* Establish RA session to URL2. */
SVN_ERR(svn_client__open_ra_session_internal(&ra_session2, URL2, NULL,
NULL, NULL, FALSE, TRUE,
ctx, pool));
@@ -2547,18 +2622,53 @@
const char *path;
apr_array_header_t *children_with_mergeinfo;
- /* If source is a path, we need to get the underlying URL
- * from the wc and save the initial path we were passed so we can use it as
- * a path parameter (either in the baton or not). otherwise, the path
- * will just be NULL, which means we won't be able to figure out some kind
- * of revision specifications, but in that case it won't matter, because
- * those ways of specifying a revision are meaningless for a url.
- */
- SVN_ERR(svn_client_url_from_path(&URL, source, pool));
- if (! URL)
- return svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL,
- _("'%s' has no URL"),
- svn_path_local_style(source, pool));
+ if (source)
+ {
+ /* If source is a path, we need to get the underlying URL
+ * from the wc and save the initial path we were passed so we can use it as
+ * a path parameter (either in the baton or not). otherwise, the path
+ * will just be NULL, which means we won't be able to figure out some kind
+ * of revision specifications, but in that case it won't matter, because
+ * those ways of specifying a revision are meaningless for a url.
+ */
+ SVN_ERR(svn_client_url_from_path(&URL, source, pool));
+ if (! URL)
+ return svn_error_createf(SVN_ERR_ENTRY_MISSING_URL, NULL,
+ _("'%s' has no URL"),
+ svn_path_local_style(source, pool));
+ }
+ else
+ {
+ /* If a merge source was not specified, try to derive it from
+ copy source. */
+ svn_ra_session_t *ra_session;
+ svn_revnum_t working_rev_num;
+ svn_opt_revision_t working_copy_rev_num_opt;
+ const char *working_copy_url;
+ const char *copy_source_path;
+ working_copy_rev_num_opt.kind = svn_opt_revision_working;
+ SVN_ERR(svn_client__ra_session_from_path(&ra_session,
+ &working_rev_num,
+ &working_copy_url,
+ target_wcpath,
+ &working_copy_rev_num_opt,
+ &working_copy_rev_num_opt,
+ ctx,
+ pool));
+ SVN_ERR(svn_client__get_copy_source(©_source_path, ra_session,
+ "", working_rev_num, pool));
+ if (copy_source_path)
+ {
+ const char *repos_root;
+ SVN_ERR(svn_ra_get_repos_root(ra_session, &repos_root, pool));
+ URL = apr_pstrcat(pool, repos_root, copy_source_path, NULL);
+ }
+ else
+ return svn_error_createf(SVN_ERR_CLIENT_PATH_HAS_NO_COPY_SOURCE, NULL,
+ _("'%s' has no implicit URL to merge from"),
+ svn_path_local_style(target_wcpath, pool));
+ }
+
if (URL == source)
path = NULL;
else
Index: subversion/tests/cmdline/merge_tests.py
===================================================================
--- subversion/tests/cmdline/merge_tests.py (revision 24750)
+++ subversion/tests/cmdline/merge_tests.py (working copy)
@@ -1243,7 +1243,7 @@
def merge_with_implicit_target_helper(sbox, arg_flav):
"ARG_FLAV is one of 'r' (revision range) or 'c' (single change)."
- if arg_flav not in ('r', 'c'):
+ if arg_flav not in ('r', 'c', '*'):
raise svntest.Failure("Unrecognized flavor of merge argument")
sbox.build()
@@ -1294,10 +1294,17 @@
svntest.actions.run_and_verify_svn(None, ['U mu\n'], [],
'merge', '-c', '-2', mu_url)
+ #implicit rev and url detection is for forward merge only.
+ elif arg_flav == '*':
+ svntest.actions.run_and_verify_svn(None, ['U mu\n'], [],
+ 'merge', '-c', '-2', mu_url)
+
# sanity-check resulting file
if (svntest.tree.get_text('mu') != orig_mu_text):
- raise svntest.Failure("Unexpected text in 'mu'")
+ raise svntest.Failure("Unexpected text '%s' in 'mu', expected '%s'" %
+ (svntest.tree.get_text('mu'), orig_mu_text))
+
# merge using filename for sourcepath
# Cannot use run_and_verify_merge with a file target
if arg_flav == 'r':
@@ -1307,6 +1314,10 @@
svntest.actions.run_and_verify_svn(None, ['G mu\n'], [],
'merge', '-c', '2', 'mu')
+ elif arg_flav == '*':
+ svntest.actions.run_and_verify_svn(None, ['G mu\n'], [],
+ 'merge', 'mu')
+
# sanity-check resulting file
if (svntest.tree.get_text('mu') != orig_mu_text + added_mu_text):
raise svntest.Failure("Unexpected text in 'mu'")
@@ -1322,6 +1333,11 @@
"merging a file w/no explicit target path using -c"
merge_with_implicit_target_helper(sbox, 'c')
+def merge_with_implicit_target_and_revs(sbox):
+ "merging a file w/no explicit target path or revs"
+ merge_with_implicit_target_helper(sbox, '*')
+
+
#----------------------------------------------------------------------
def merge_with_prev (sbox):
@@ -5566,6 +5582,7 @@
simple_property_merges,
merge_with_implicit_target_using_r,
merge_with_implicit_target_using_c,
+ merge_with_implicit_target_and_revs,
merge_catches_nonexistent_target,
merge_tree_deleted_in_target,
merge_similar_unrelated_trees,
Index: subversion/include/svn_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h (revision 24750)
+++ subversion/include/svn_error_codes.h (working copy)
@@ -952,6 +948,10 @@
SVN_ERR_CLIENT_CATEGORY_START + 14,
"Operation does not support multiple sources")
+ SVN_ERRDEF(SVN_ERR_CLIENT_PATH_HAS_NO_COPY_SOURCE,
+ SVN_ERR_CLIENT_CATEGORY_START + 15,
+ "Path has no copy source")
+
/* misc errors */
SVN_ERRDEF(SVN_ERR_BASE,
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 24 18:12:30 2007