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 2013/06/04 00:12:42 UTC
svn commit: r1489203 - /subversion/trunk/subversion/libsvn_client/merge.c
Author: julianfoad
Date: Mon Jun 3 22:12:41 2013
New Revision: 1489203
URL: http://svn.apache.org/r1489203
Log:
* subversion/libsvn_client/merge.c
(find_automatic_merge): Error instead of crashing if source branch is not
related to target branch.
Found by: steveking
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1489203&r1=1489202&r2=1489203&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Mon Jun 3 22:12:41 2013
@@ -12307,6 +12307,12 @@ find_automatic_merge(svn_client__pathrev
SVN_ERR(svn_client__get_youngest_common_ancestor(
&s_t->yca, s_t->source, &s_t->target->loc, s_t->source_ra_session,
ctx, result_pool, result_pool));
+ if (! s_t->yca)
+ return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
+ _("'%s@%ld' must be ancestrally related to "
+ "'%s@%ld'"),
+ s_t->source->url, s_t->source->rev,
+ s_t->target->loc.url, s_t->target->loc.rev);
/* Find the latest revision of A synced to B and the latest
* revision of B synced to A.
Re: svn commit: r1489203 - /subversion/trunk/subversion/libsvn_client/merge.c
Posted by Stefan Küng <to...@gmail.com>.
On Tue, Jun 4, 2013 at 11:36 PM, Julian Foad <ju...@btopenworld.com>wrote:
> Stefan Küng wrote:
>
> > On 04.06.2013 17:50, Julian Foad wrote:
> >> Greg Stein wrote:
> >>> Would "%s@%ld and %s@%ld must have a common ancestor" be easier to
> >>> translate?
> >>>
> >>> The term "ancestrally related" seems a bit complicated for
> >>> translation :-P
> >>
> >> Your suggestion is a better message, I agree.
> >>
> >> The present error code and error message exactly duplicate one that
> >> is already used in 'reintegrate'. As this is in the library, the
> >> message is perhaps less important than the error code. We can tweak
> >> the message, but perhaps we should also make it possible for the
> >> caller to distinguish this case, by using a different error code such
> >> as 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' (which is already for cases
> >> such as source and target being in different repositories), or even a
> >> new one.
> >>
> >> Stefan K or other GUI people, do you have an opinion on this?
> >
> > Using 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' is fine with me.
> > Thanks btw for fixing this.
>
> Can you clarify? Do you want me to change it to
> 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' and propose that change for back-port
> to 1.8.x? It's currently already approved for backport with error code
> SVN_ERR_CLIENT_NOT_READY_TO_MERGE.
>
>
I'm not checking for a specific error code (yet) so keeping
SVN_ERR_CLIENT_NOT_READY_TO_MERGE is fine. But I think that
'SVN_ERR_CLIENT_UNRELATED_RESOURCES' would be better for the future since
it indicates better why the merge failed.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: svn commit: r1489203 - /subversion/trunk/subversion/libsvn_client/merge.c
Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Küng wrote:
> On 04.06.2013 17:50, Julian Foad wrote:
>> Greg Stein wrote:
>>> Would "%s@%ld and %s@%ld must have a common ancestor" be easier to
>>> translate?
>>>
>>> The term "ancestrally related" seems a bit complicated for
>>> translation :-P
>>
>> Your suggestion is a better message, I agree.
>>
>> The present error code and error message exactly duplicate one that
>> is already used in 'reintegrate'. As this is in the library, the
>> message is perhaps less important than the error code. We can tweak
>> the message, but perhaps we should also make it possible for the
>> caller to distinguish this case, by using a different error code such
>> as 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' (which is already for cases
>> such as source and target being in different repositories), or even a
>> new one.
>>
>> Stefan K or other GUI people, do you have an opinion on this?
>
> Using 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' is fine with me.
> Thanks btw for fixing this.
Can you clarify? Do you want me to change it to 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' and propose that change for back-port to 1.8.x? It's currently already approved for backport with error code SVN_ERR_CLIENT_NOT_READY_TO_MERGE.
- Julian
Re: svn commit: r1489203 - /subversion/trunk/subversion/libsvn_client/merge.c
Posted by Stefan Küng <to...@gmail.com>.
On 04.06.2013 17:50, Julian Foad wrote:
> Greg Stein wrote:
>
>> Would "%s@%ld and %s@%ld must have a common ancestor" be easier to
>> translate?
>>
>> The term "ancestrally related" seems a bit complicated for
>> translation :-P
>
> Your suggestion is a better message, I agree.
>
> The present error code and error message exactly duplicate one that
> is already used in 'reintegrate'. As this is in the library, the
> message is perhaps less important than the error code. We can tweak
> the message, but perhaps we should also make it possible for the
> caller to distinguish this case, by using a different error code such
> as 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' (which is already for cases
> such as source and target being in different repositories), or even a
> new one.
>
> Stefan K or other GUI people, do you have an opinion on this?
Using 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' is fine with me.
Thanks btw for fixing this.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest interface to (Sub)version control
/_/ \_\ http://tortoisesvn.net
Re: svn commit: r1489203 - /subversion/trunk/subversion/libsvn_client/merge.c
Posted by Julian Foad <ju...@btopenworld.com>.
Greg Stein wrote:
> Would "%s@%ld and %s@%ld must have a common ancestor" be easier to
> translate?
>
> The term "ancestrally related" seems a bit complicated for translation
> :-P
Your suggestion is a better message, I agree.
The present error code and error message exactly duplicate one that is already used in 'reintegrate'. As this is in the library, the message is perhaps less important than the error code. We can tweak the message, but perhaps we should also make it possible for the caller to distinguish this case, by using a different error code such as 'SVN_ERR_CLIENT_UNRELATED_RESOURCES' (which is already for cases such as source and target being in different repositories), or even a new one.
Stefan K or other GUI people, do you have an opinion on this?
- Julian
> On Mon, Jun 3, 2013 at 6:12 PM, <ju...@apache.org> wrote:
>> Author: julianfoad
>> Date: Mon Jun 3 22:12:41 2013
>> New Revision: 1489203
>>
>> URL: http://svn.apache.org/r1489203
>> Log:
>> * subversion/libsvn_client/merge.c
>> (find_automatic_merge): Error instead of crashing if source branch is not
>> related to target branch.
>>
>> Found by: steveking
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_client/merge.c
>>
>> Modified: subversion/trunk/subversion/libsvn_client/merge.c
>> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1489203&r1=1489202&r2=1489203&view=diff
>>
> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
>> +++ subversion/trunk/subversion/libsvn_client/merge.c Mon Jun 3 22:12:41
> 2013
>> @@ -12307,6 +12307,12 @@ find_automatic_merge(svn_client__pathrev
>> SVN_ERR(svn_client__get_youngest_common_ancestor(
>> &s_t->yca, s_t->source, &s_t->target->loc,
> s_t->source_ra_session,
>> ctx, result_pool, result_pool));
>> + if (! s_t->yca)
>> + return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
>> + _("'%s@%ld' must be ancestrally
> related to "
>> + "'%s@%ld'"),
>> + s_t->source->url,
> s_t->source->rev,
>> + s_t->target->loc.url,
> s_t->target->loc.rev);
>>
>> /* Find the latest revision of A synced to B and the latest
>> * revision of B synced to A.
>>
>>
>
Re: svn commit: r1489203 - /subversion/trunk/subversion/libsvn_client/merge.c
Posted by Greg Stein <gs...@gmail.com>.
Would "%s@%ld and %s@%ld must have a common ancestor" be easier to translate?
The term "ancestrally related" seems a bit complicated for translation :-P
On Mon, Jun 3, 2013 at 6:12 PM, <ju...@apache.org> wrote:
> Author: julianfoad
> Date: Mon Jun 3 22:12:41 2013
> New Revision: 1489203
>
> URL: http://svn.apache.org/r1489203
> Log:
> * subversion/libsvn_client/merge.c
> (find_automatic_merge): Error instead of crashing if source branch is not
> related to target branch.
>
> Found by: steveking
>
> Modified:
> subversion/trunk/subversion/libsvn_client/merge.c
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1489203&r1=1489202&r2=1489203&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Mon Jun 3 22:12:41 2013
> @@ -12307,6 +12307,12 @@ find_automatic_merge(svn_client__pathrev
> SVN_ERR(svn_client__get_youngest_common_ancestor(
> &s_t->yca, s_t->source, &s_t->target->loc, s_t->source_ra_session,
> ctx, result_pool, result_pool));
> + if (! s_t->yca)
> + return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> + _("'%s@%ld' must be ancestrally related to "
> + "'%s@%ld'"),
> + s_t->source->url, s_t->source->rev,
> + s_t->target->loc.url, s_t->target->loc.rev);
>
> /* Find the latest revision of A synced to B and the latest
> * revision of B synced to A.
>
>