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

Reply via email to