On Thu, 2010-12-02 at 13:58 +0530, Noorul Islam K M wrote:
> Julian Foad <julianfoad_at_btopenworld.com> writes:
>
> > On Tue, 2010-11-30 at 18:42 +0530, Noorul Islam K M wrote:
> >
> >> Julian Foad <julian.foad_at_wandisco.com> writes:
> >>
> >> > I tried some potentially invalid inputs to "svn" a week or two ago and
> >> > made notes on what I found. Just posting here in case anyone wants to
> >> > do something about one or more of them.
> >> >
> >> > Noorul, I'm including you in the "To" addresses because you said you
> >> > were looking for more small tasks to do, so feel free to pick one of
> >> > these if you want to.
> >>
> >> Thank you! I will start working on these one by one.
> >
> > Great!
> >
> >
> >> > Where I end with a question mark, it means I'm not sure if we want this
> >> > change, it's just a suggestion.
> >> >
> >> > * "svn checkout ^/ ^/y" -> "A asf/cxf, A asf/cxf/utils, ...". (Don't
> >> > try this without being ready on the Ctrl-C or Ctrl-\!) It seems to
> >> > ignore "^/y" and create ./(basename(^/)); should fail: "'^/y' is not a
> >> > WC path".
> >> >
> >> > * "svn checkout ^/subversion/trunk/build ^/y" -> "Checked out revision
> >> > 1040465. URL 'https://svn.apache.org/repos/asf/y' doesn't exist".
> >> > Bleach - that's just crazy. Should fail: "'^/y' is not a WC path".
> >> >
> >> > * "svn copy a ^/b c" doesn't detect the mixed source types in cl, only
> >> > in lib; should reject them at CLI level?
> >> >
> >> > * "svn info ^/b" -> "Not a valid URL"; should be "path '/b' not found
> >> > in revision REV"?
> >> >
> >> > * "svn mkdir ^/ a" -> "Illegal repository URL 'a'"; should say "can't
> >> > mix URL and local targets"?
> >> >
> >> > * "svn mkdir a ^/" -> "Can't create directory
> >> > 'https:/svn.apache.org/repos/asf'"; should not print the URL as if it's
> >> > a local path.
> >> >
> >> > * "svn mv ^/ ^/" -> "Cannot move path
> >> > 'https:/svn.apache.org/repos/asf' into itself"; should not print the URL
> >> > as if it's a local path.
> >> >
> >> > * "svn update ^/a" -> "Skipped
> >> > 'https://svn.apache.org/repos/asf/a' ..."; should fail: "'^/a' is not a
> >> > WC path"?
> >> >
> >>
> >> svn help update says that the argument should be a PATH. I think it is
> >> right to force the user to enter local PATH.
> >
> > Good. Thanks for finding that. I agree. That change will make some of
> > my recent patch (r1040232) redundant: it will no longer need to remove
> > URL targets from the array of targets, since it will just return an
> > error if it finds any, like you already did in some of the other
> > subcomands.
> >
>
> I further looked into the code, since update accepts multiple targets,
> the program skips URL targets assuming that there might one or more
> local paths as part of targets list. The skipping part is intentional
> and I am not sure whether we should change this behavior.
I think we should change this behaviour and make "svn update" throw an
error if any target is a URL.
- Julian
Received on 2010-12-02 13:25:31 CET