[patch] fixes for test case get_url, make_branch, etc

[Martin Pool]

On 27 Jul 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:

> Or you can do:
> 
> if base.startswith('./') or base.startswith('/'):
>   base = urlutils.local_path_to_url(base)
> base += urlutils.escape(relpath)
> 
> *However* you will still run into the problem that this borks on win32.
> 
> Because local paths don't start with '/' there.
> I think the best fix is to make get_url() always return a URL.

I'd like it to always return a url too.  In fact in an earlier attempt
at this I changed it to assert that the server always gives a URL.

The only servers where that is not true are the LocalRelpath and
LocalAbspath.  They only make sense as returning paths rather than urls,
so we'd have to remove them.  (There is afaik no cwd-relative file
url.)

I think we would get basically the same coverage by removing those
servers, then specifically asserting that you can pass a local or
absolute path to get_transport, and get an appropriate LocalTransport.
>From that point on, everything should be done the same way regardless of
how the LocalTransport was created, and testing it repeatedly is a
waste.  Then we can start to make sure the urls really are urls.  How
does that sound?  

> >      def get_transport(self):
> > @@ -1109,20 +1116,18 @@
> >  
> >      def make_bzrdir(self, relpath, format=None):
> >          try:
> > -            url = self.get_url(relpath)
> > -            mutter('relpath %r => url %r', relpath, url)
> > -            segments = url.split('/')
> > +            # might be a relative or absolute path
> > +            maybe_a_url = self.get_url(relpath)
> > +            segments = maybe_a_url.split('/')
> > +            t = get_transport(maybe_a_url)
> >              if segments and segments[-1] not in ('', '.'):
> > -                parent = '/'.join(segments[:-1])
> > -                t = get_transport(parent)
> >                  try:
> > -                    t.mkdir(segments[-1])
> > +                    t.mkdir('.')
> >                  except errors.FileExists:
> >                      pass
> >              if format is None:
> > -                format=bzrlib.bzrdir.BzrDirFormat.get_default_format()
> > -            # FIXME: make this use a single transport someday. RBC 20060418
> > -            return format.initialize_on_transport(get_transport(relpath))
> > +                format = bzrlib.bzrdir.BzrDirFormat.get_default_format()
> > +            return format.initialize_on_transport(t)
> >          except errors.UninitializableFormat:
> >              raise TestSkipped("Format %s is not initializable." % format)
> 
> I think you should get rid of 'segments' because for something like:
> file:/// it does the wrong thing. And for 'http://localhost/' it is also
> broken (it will try to cd into http://, etc)
> 
> Just use "t = t.clone('..')"

Actually it's just a complicated way of checking if it ends in '/' or
'/.' - and we could just do that directly.  Or indeed perhaps we should
try to create the thing even if it ends in such a prefix.

> Or if you just want to create a single directory, you can use:
> base, tip = urlutils.split(url)

No, actually we can't (yet) - because urlutils assumes it's a url.

> So, I don't really like the hacks you put in to handle local paths
> versus URLs, since they will fail on other platforms. (Now, it will only
> fail if 'relpath' has non-standard characters, but still....)

Well, the situation of local paths was already there, I just tried to
make it a bit less dodgy - in particular it does cope better if the
relpath contains nonascii characters.  What failure were you thinking
about?

-- 
Martin