[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] svn diff -r1:BASE URL asserts

From: Paul Burba <paulb_at_softlanding.com>
Date: 2006-01-13 15:37:32 CET

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

This is an archived mail posted to the Subversion Dev mailing list.