urlutils improvements from hpss

John Arbash Meinel john at arbash-meinel.com
Thu Mar 29 23:51:54 BST 2007


Martin Pool wrote:
> On 3/30/07, John Arbash Meinel <john at arbash-meinel.com> wrote:
>> I don't really understand the need for "joinpath" versus "pathjoin".
> 
> Did you mean "versus join_url_path"?
> 
> Anyhow, I agree, the module name makes it clear enough.
> 

No, I mean we have 2 functions. "osutils.joinpath" and
"osutils.pathjoin". Where "pathjoin" is inherited from os.path.join, but
then "joinpath" seems to be doing the same thing, only disallowing '..',
'', and None in the paths.

The naming is close enough to make it seem like they should be the same
function. And they do almost the same thing, but one has a subtle
difference.

For urlutils I simplified it to urlutils.join().

But now there is urlutils.joinpath(), and from a naive glance, I don't
see a big difference between the two.

What is also weird to me is that urlutils.join() does this in the interior:

            path = '/'.join(path)
            path = joinpath(path, arg)
            path = path.split('/')

so it takes ['foo', 'bar', 'baz']  => 'foo/bar/baz' passes it to
'joinpath' and then splits it back up. At best this is inefficient.

Especially considering that 'joinpath()' is defined first as:

def joinpath(base, *args):
  path = base.split('/')
  ...


So that means the above becomes:


   path = '/'.join(path)
   path = path.split('/')
   # operate on extra argument
   path = '/'.join(path)
   path = path.split('/')



Obviously I missed this when Robert proposed this patch. Though it is
the 'hpss-urlutils' patch. So it is fairly recent.

I'm all for code re-use, but something is seriously borked with this
code path. I sure hope we don't call 'urlutils.join()' with any sort of
frequency. (Though I think we do it for every Transport call, which
means it happens at least 2 times per knit we process).


I suppose I would like to know what was wrong with "urlutils.join()"
that it couldn't be used rather than "urlutils.joinpath()".

John
=:->



More information about the bazaar mailing list