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

Re: [PATCH] - svn blame doesn't use the working revision (Issue 1777)

From: John Szakmeister <john_at_szakmeister.net>
Date: 2004-04-16 23:57:21 CEST

On Friday 16 April 2004 11:23, kfogel@collab.net wrote:
> John Szakmeister <john@szakmeister.net> writes:
> > > Log Message:
> > > Fix Issue #1777: 'svn blame' should default to the working revision,
> > > when no revision is specified.
> > >
> > > * subversion/clients/cmdline/blame-cmd.c
> > > (svn_cl__blame) : Set opt_state->end_revision.kind
> > > to svn_opt_revision_base,to ensure 'svn blame' defaults to base
> > > revision.
> >
> > Small problem here. This breaks using a URL without specifying a
> > revision. With your patch I get the following when trying to use a URL
> > instead of a
> >
> > working copy path:
> > :: svn blame file://`pwd`/repo/A
> >
> > subversion/libsvn_wc/lock.c:373: (apr_err=155007)
> > svn: 'file:///home/jszakmeister/projects/tmp/repo' is not a working copy
> >
> > I think the best solution here would be to let svn_client_blame()
> > resolve the start and end revision if it's unspecified. This
> > changes the API a little, so the earliest it could make it into a
> > release would be 1.1 (which is where the issue put it anyways).
> > Would you mind taking another stab at this but fixing
> > svn_client_blame() instead?
>
> That would mean creating svn_client_blame2(), at least until 2.0.
> (See the subsection "Deprecation" in HACKING for details.)
>
> Instead, why not take Ramaswamy's patch a bit further:
>
> svn_cl__blame should set to svn_opt_revision_base if the target is a
> local path, or to svn_opt_revision_head if target is a URL. That way,
> no modifications to svn_client_blame() are necessary right now.

I actually thought of that this morning and had a paragraph in the email about
it. I think the best answer to this problem is to fix svn_client_blame()
which is why I proposed that as a solution (every other user of
svn_client_blame() is going to have to implement the same logic otherwise).
But, I also though that this could fit into 1.1. I have to admit I'm still a
little confused about the API compatibility/deprecation stuff. I read the
following paragraph, and told myself that we aren't changing the signature at
all and old clients would still function with the new restrictions (actually,
it's less restrictive :-).

   2) Upgrading to a new minor release in the same major line may
      cause new APIs to appear, but not remove any APIs. Any code
      written to the old minor number will work with any later minor
      number in that line. However, downgrading afterwards may not
      work, if new code has been written that takes advantage of the
      new APIs.

If changing svn_client_blame() doesn't get us into 1.1, then I'd say you and
C. Mike are right, we just fix the inputs in the command line client and wait
until 2.0 to fix the API.

> We can, if we want, file a 2.0 issue to change that interface promise
> in svn_client_blame(), have it behave as you imply, and adjust
> svn_cl__blame() accordingly. But, I don't think we need to bother
> with the svn_client_blame2() intermediate step right now, when we can
> solve it in the caller pretty easily.

Yeah, that's too much for such a minor change. We should just file an issue
for a 2.0 change to the API, and take care of it then. I can do that if no
one objects to it.

S.Ramaswamy, did you want to take a stab at fixing this in the command-line
client? Basically you need to choose the end revision to be HEAD if the
target is a URL, and BASE if it's a working copy path. You can use
svn_path_is_url() to make that determination.

-John

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Apr 17 00:01:18 2004

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.