You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Daniel L. Rall" <dl...@finemaltcoding.com> on 2008/03/19 19:12:09 UTC
Re: svn commit: r29962 - trunk/subversion/svn
I would like to re-iterate that I hate this.
On Wed, 19 Mar 2008, kfogel@tigris.org wrote:
> Author: kfogel
> Date: Wed Mar 19 11:33:59 2008
> New Revision: 29962
>
> Log:
> Currently, 'svn merge' always requires a merge source, so enforce that.
>
> * subversion/svn/main.c
> (svn_cl__cmd_table."merge"): Say SOURCE is required, in help string.
>
> * subversion/svn/merge-cmd.c
> (svn_cl__merge): Error if no merge source supplied.
>
> Modified:
> trunk/subversion/svn/main.c
> trunk/subversion/svn/merge-cmd.c
>
> Modified: trunk/subversion/svn/main.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/main.c?pathrev=29962&r1=29961&r2=29962
> ==============================================================================
> --- trunk/subversion/svn/main.c Wed Mar 19 10:48:51 2008 (r29961)
> +++ trunk/subversion/svn/main.c Wed Mar 19 11:33:59 2008 (r29962)
> @@ -586,7 +586,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
> ("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[,N]]... | [-r N:M]...] [SOURCE[@REV] [WCPATH]]\n"
> + " 3. merge [-c M[,N]... | -r N:M...] SOURCE[@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"
> @@ -597,15 +597,14 @@ const svn_opt_subcommand_desc2_t svn_cl_
> " be specified.\n"
> "\n"
> " 3. In the third form, SOURCE can be either a URL or a working copy\n"
> - " path (in which case its corresponding URL is used). If not\n"
> - " specified, SOURCE will be the same as WCPATH. SOURCE in revision\n"
> - " REV is compared as it existed between revisions N and M for each\n"
> - " revision range provided. If REV is not specified, HEAD is\n"
> - " assumed. '-c M' is equivalent to '-r <M-1>:M', and '-c -M' does\n"
> - " the reverse: '-r M:<M-1>'. If no revision ranges are specified,\n"
> - " the default range of 1:HEAD is used. Multiple '-c' and/or '-r'\n"
> - " instances may be specified, and mixing of forward and reverse\n"
> - " ranges is allowed.\n"
> + " path (in which case its corresponding URL is used). SOURCE (in\n"
> + " revision REV) is compared as it existed between revisions N and M\n"
> + " for each revision range provided. If REV is not specified, HEAD\n"
> + " is assumed. '-c M' is equivalent to '-r <M-1>:M', and '-c -M'\n"
> + " does the reverse: '-r M:<M-1>'. If no revision ranges are\n"
> + " specified, the default range of 1:HEAD is used. Multiple '-c'\n"
> + " and/or '-r' instances may be specified, and mixing of forward\n"
> + " and reverse ranges is allowed.\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"
>
> Modified: trunk/subversion/svn/merge-cmd.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/merge-cmd.c?pathrev=29962&r1=29961&r2=29962
> ==============================================================================
> --- trunk/subversion/svn/merge-cmd.c Wed Mar 19 10:48:51 2008 (r29961)
> +++ trunk/subversion/svn/merge-cmd.c Wed Mar 19 11:33:59 2008 (r29962)
> @@ -54,8 +54,57 @@ svn_cl__merge(apr_getopt_t *os,
> opt_state->targets,
> pool));
>
> - /* Parse at least one, and possible two, sources. */
> - if (targets->nelts >= 1)
> + /* For now, we require at least one source. That may change in
> + future versions of Subversion, for example if we have support for
> + negated mergeinfo. See this IRC conversation:
> +
> + <bhuvan> kfogel: yeah, i think you are correct; we should
> + specify the source url
> +
> + <kfogel> bhuvan: I'll change the help output and propose for
> + backport. Thanks.
> +
> + <bhuvan> kfogel: np; while we are at it, 'svn merge' simply
> + returns nothing; i think we should say: """svn: Not
> + enough arguments provided; try 'svn help' for more
> + info"""
> +
> + <kfogel> good idea
> +
> + <kfogel> (in the future, 'svn merge' might actually do
> + something, but that's all the more reason to make
> + sure it errors now)
> +
> + <cmpilato> actually, i'm pretty sure 'svn merge' does something
> +
> + <cmpilato> it says "please merge any unmerged changes from
> + myself to myself."
> +
> + <cmpilato> :-)
> +
> + <kfogel> har har
> +
> + <cmpilato> kfogel: i was serious.
> +
> + <kfogel> cmpilato: urrr, uh. Is that meaningful? Is there
> + ever a reason for a user to run it?
> +
> + <cmpilato> kfogel: not while we don't have support for negated
> + mergeinfo.
> +
> + <kfogel> cmpilato: do you concur that until it does something
> + useful it should error?
> +
> + <cmpilato> kfogel: yup.
> +
> + <kfogel> cool
> + */
> + if (targets->nelts < 1)
> + {
> + return svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, 0,
> + _("merge source required"));
> + }
> + else /* Parse at least one, and possible two, sources. */
> {
> SVN_ERR(svn_opt_parse_path(&peg_revision1, &sourcepath1,
> APR_ARRAY_IDX(targets, 0, const char *),
> @@ -66,8 +115,8 @@ svn_cl__merge(apr_getopt_t *os,
> pool));
> }
>
> - /* If nothing (ie, "."), a single source, or a source URL plus WC path is
> - provided, then we don't have two distinct sources. */
> + /* We could have one or two sources. Deliberately written to stay
> + correct even if we someday permit implied merge source. */
> if (targets->nelts <= 1)
> {
> two_sources_specified = FALSE;
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
--
Daniel Rall
Re: svn commit: r29962 - trunk/subversion/svn
Posted by Karl Fogel <kf...@red-bean.com>.
"Daniel L. Rall" <dl...@finemaltcoding.com> writes:
> I would like to re-iterate that I hate this.
I hate it too. However, I think it's the right call for 1.5. In 1.6,
we can make 'svn merge' work with no explicit arguments if we want --
but then we'll know we're getting it right and that we're not going to
have compatibility problems with 1.5.
-Karl
> On Wed, 19 Mar 2008, kfogel@tigris.org wrote:
>
>> Author: kfogel
>> Date: Wed Mar 19 11:33:59 2008
>> New Revision: 29962
>>
>> Log:
>> Currently, 'svn merge' always requires a merge source, so enforce that.
>>
>> * subversion/svn/main.c
>> (svn_cl__cmd_table."merge"): Say SOURCE is required, in help string.
>>
>> * subversion/svn/merge-cmd.c
>> (svn_cl__merge): Error if no merge source supplied.
>>
>> Modified:
>> trunk/subversion/svn/main.c
>> trunk/subversion/svn/merge-cmd.c
>>
>> Modified: trunk/subversion/svn/main.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/main.c?pathrev=29962&r1=29961&r2=29962
>> ==============================================================================
>> --- trunk/subversion/svn/main.c Wed Mar 19 10:48:51 2008 (r29961)
>> +++ trunk/subversion/svn/main.c Wed Mar 19 11:33:59 2008 (r29962)
>> @@ -586,7 +586,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
>> ("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[,N]]... | [-r N:M]...] [SOURCE[@REV] [WCPATH]]\n"
>> + " 3. merge [-c M[,N]... | -r N:M...] SOURCE[@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"
>> @@ -597,15 +597,14 @@ const svn_opt_subcommand_desc2_t svn_cl_
>> " be specified.\n"
>> "\n"
>> " 3. In the third form, SOURCE can be either a URL or a working copy\n"
>> - " path (in which case its corresponding URL is used). If not\n"
>> - " specified, SOURCE will be the same as WCPATH. SOURCE in revision\n"
>> - " REV is compared as it existed between revisions N and M for each\n"
>> - " revision range provided. If REV is not specified, HEAD is\n"
>> - " assumed. '-c M' is equivalent to '-r <M-1>:M', and '-c -M' does\n"
>> - " the reverse: '-r M:<M-1>'. If no revision ranges are specified,\n"
>> - " the default range of 1:HEAD is used. Multiple '-c' and/or '-r'\n"
>> - " instances may be specified, and mixing of forward and reverse\n"
>> - " ranges is allowed.\n"
>> + " path (in which case its corresponding URL is used). SOURCE (in\n"
>> + " revision REV) is compared as it existed between revisions N and M\n"
>> + " for each revision range provided. If REV is not specified, HEAD\n"
>> + " is assumed. '-c M' is equivalent to '-r <M-1>:M', and '-c -M'\n"
>> + " does the reverse: '-r M:<M-1>'. If no revision ranges are\n"
>> + " specified, the default range of 1:HEAD is used. Multiple '-c'\n"
>> + " and/or '-r' instances may be specified, and mixing of forward\n"
>> + " and reverse ranges is allowed.\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"
>>
>> Modified: trunk/subversion/svn/merge-cmd.c
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/merge-cmd.c?pathrev=29962&r1=29961&r2=29962
>> ==============================================================================
>> --- trunk/subversion/svn/merge-cmd.c Wed Mar 19 10:48:51 2008 (r29961)
>> +++ trunk/subversion/svn/merge-cmd.c Wed Mar 19 11:33:59 2008 (r29962)
>> @@ -54,8 +54,57 @@ svn_cl__merge(apr_getopt_t *os,
>> opt_state->targets,
>> pool));
>>
>> - /* Parse at least one, and possible two, sources. */
>> - if (targets->nelts >= 1)
>> + /* For now, we require at least one source. That may change in
>> + future versions of Subversion, for example if we have support for
>> + negated mergeinfo. See this IRC conversation:
>> +
>> + <bhuvan> kfogel: yeah, i think you are correct; we should
>> + specify the source url
>> +
>> + <kfogel> bhuvan: I'll change the help output and propose for
>> + backport. Thanks.
>> +
>> + <bhuvan> kfogel: np; while we are at it, 'svn merge' simply
>> + returns nothing; i think we should say: """svn: Not
>> + enough arguments provided; try 'svn help' for more
>> + info"""
>> +
>> + <kfogel> good idea
>> +
>> + <kfogel> (in the future, 'svn merge' might actually do
>> + something, but that's all the more reason to make
>> + sure it errors now)
>> +
>> + <cmpilato> actually, i'm pretty sure 'svn merge' does something
>> +
>> + <cmpilato> it says "please merge any unmerged changes from
>> + myself to myself."
>> +
>> + <cmpilato> :-)
>> +
>> + <kfogel> har har
>> +
>> + <cmpilato> kfogel: i was serious.
>> +
>> + <kfogel> cmpilato: urrr, uh. Is that meaningful? Is there
>> + ever a reason for a user to run it?
>> +
>> + <cmpilato> kfogel: not while we don't have support for negated
>> + mergeinfo.
>> +
>> + <kfogel> cmpilato: do you concur that until it does something
>> + useful it should error?
>> +
>> + <cmpilato> kfogel: yup.
>> +
>> + <kfogel> cool
>> + */
>> + if (targets->nelts < 1)
>> + {
>> + return svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, 0,
>> + _("merge source required"));
>> + }
>> + else /* Parse at least one, and possible two, sources. */
>> {
>> SVN_ERR(svn_opt_parse_path(&peg_revision1, &sourcepath1,
>> APR_ARRAY_IDX(targets, 0, const char *),
>> @@ -66,8 +115,8 @@ svn_cl__merge(apr_getopt_t *os,
>> pool));
>> }
>>
>> - /* If nothing (ie, "."), a single source, or a source URL plus WC path is
>> - provided, then we don't have two distinct sources. */
>> + /* We could have one or two sources. Deliberately written to stay
>> + correct even if we someday permit implied merge source. */
>> if (targets->nelts <= 1)
>> {
>> two_sources_specified = FALSE;
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: svn-help@subversion.tigris.org
>
> --
>
> Daniel Rall
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org