Julian Foad <julianfoad@btopenworld.com> wrote on 01/12/2006 07:39:00 PM:
> >>>>>diff -r1:BASE file:///home/BURBA/REPOS/SIG5
> >>>>>Assertion failed: ! svn_path_is_url (path2), [...]
>
> Paul Burba wrote:
> > Anyway, here is my proposed fix. The "opt_state->start_revision.kind
==
> > svn_opt_revision_base" isn't absolutely necessary since this case is
> > caught in svn_client__get_revision_number(), but it seems more
appropriate
> > to check it here.
>
> My first reaction is: "What is special about BASE?" Why isn't
> handled the same
> as PREV and COMMITTED which are the other two revision keywords that
> require a WC?
Hi Julian,
What's special about it is that svn diff x:BASE asserts and the other
cases are handled, albeit inconsistently. My thought was just to prevent
the assert - that's my rather lame excuse anyway since I *did* include the
svn diff BASE:x case which the current code doesn't assert on. As you
say, it does makes more sense to handle all these invalid diffs in a
consistent manner...
> It would be good to make BASE and PREV and COMMITTED all produce the
same
> error, preferably by all being handled at the same point. At
> present, PREV and
> COMMITTED produce:
>
> svn diff -r1:PREV file:///home/julianfoad/tmp/vcs/sandbox/trunk
> svn: 'file:///home/julianfoad/tmp/vcs/sandbox' is not a working copy
...so to that end here is a new log and patch.
Thanks for checking this out,
Paul B.
[[[
Return consistent error on svn diff of special revisions BASE, COMMITTED,
or PREV when operating on a URL.
The original impetus for this patch was to stop svn diff 1:BASE URL from
asserting and provide a more useful error message.
* subversion/svn/diff-cmd.c
(svn_cl__diff): If the starting or ending revision against a URL is
BASE,
COMMITTED, or PREV return an error.
]]]
Index: subversion/svn/diff-cmd.c
===================================================================
--- subversion/svn/diff-cmd.c (revision 18091)
+++ subversion/svn/diff-cmd.c (working copy)
@@ -155,6 +155,19 @@
return svn_error_createf (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
_("Target lists to diff may not contain
"
"both working copy paths and URLs"));
+
+ /* Diffing special revisions BASE, COMMITTED, or PREV
+ against a URL is invalid. */
+ if (url_present
+ && (opt_state->start_revision.kind == svn_opt_revision_base
+ || opt_state->end_revision.kind == svn_opt_revision_base
+ || opt_state->start_revision.kind ==
svn_opt_revision_committed
+ || opt_state->end_revision.kind ==
svn_opt_revision_committed
+ || opt_state->start_revision.kind ==
svn_opt_revision_previous
+ || opt_state->end_revision.kind ==
svn_opt_revision_previous)
+)
+ return svn_error_create
+ (SVN_ERR_CLIENT_VERSIONED_PATH_REQUIRED, NULL, NULL);
if (opt_state->start_revision.kind == svn_opt_revision_unspecified
&& working_copy_present)
_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jan 13 15:41:34 2006