Tom Mueller wrote:
Shawn,
Please see inline.
Shawn Walker wrote:
Tom Mueller (pkg-discuss) wrote:
This is a one-line code review question for an issue on Windows with
the new file: URI capability for pkgsend.
webrev: http://cr.opensolaris.org/~tmueller/ips-7535/
bug: http://defect.opensolaris.org/bz/show_bug.cgi?id=7535
Hi Tom,
Instead of special casing this just for pkgsend (and specifically in
FileTransaction), I think we need our own wrapper function for
urlparse put into misc. Then, *anywhere* we call urlparse, we need to
call that function instead.
I looked at all of the other calls to urlparse in pkg(5), and in no
other case would this call to url2pathname be necessary, either because
the call is never made for a file URL or the returned path value is not
used. IMHO, adding a separate method at this time would be adding
additional complexity and performance overhead without sufficient
benefit. While it is true that without this special method one has to
remember to call url2pathname when using urlparse in this context, but
if we had the method, one would have to remember to call the special
method. It's something to remember either way.
I have to disagree somewhat, as pkgsend is not the only client that will
be using the path in the future. In the future, pkg(1) will be using it
as well -- but that will be hidden behind the transport layer.
We need a "big note" somewhere about this particular shortcoming in
urlparse; I am more than annoyed that we need to special case things for
a single platform.
The wrapper should check the scheme of the url, and if it is "file",
it should then make the url2pathname call. The other odd thing is
that it seems like url2pathname could be called on the entire url; not
just the path component.
The documentation for url2pathname
<http://docs.python.org/library/urllib.html> clearly states that it is
to be called on the path value, not the entire url.
The python 2.6 documentation on the web may do that, but the
documentation in python 2.4 for "pydoc urllib.rl2pathname" does not:
urllib.url2pathname = url2pathname(pathname)
OS-specific conversion from a relative URL of the 'file' scheme
to a file system path; not recommended for general use.
...but thanks for the FYI.
I don't like this fix (not because of how you've done or what you've
done), but because of the general suckiness of the python libs :|
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss