On 11/29/10 03:32 PM, Tim Foster wrote:
Hi Shawn,

On Mon, 2010-11-29 at 13:17 -0800, Shawn Walker wrote:
webrev:
    http://cr.opensolaris.org/~swalker/pkg-15864/

Logic is:
<snip>

That logic seems sane.

src/client.py, line 3738, lines 1312, 3545, 3572, 3622 (&  elsewhere)

There's comments saying various items should be accessible through the
API, but then we still call img.xxx(). Given as img isn't in these
methods (but declared a global before we get here) would it be more
polite to just remove the global, and call api_inst.img.xxx() instead?
   Later, once those items are accessible through the API, we could
revisit the code to have it call the new API methods, hiding
api_inst.img.

I'd rather not change them in that way to make it a bit more obvious that they're wrong. There's already bugs filed for all of the XXX bits.

I think the end result really will be the same.

src/pkgdep.py, line 279

Could the shell-escaping code be shared between pkgdep and client,
perhaps in pkg.misc?

It's not meant to be a generic shell escaping routine which is why I didn't put it in a central place to be shared. If we do actually end up having more than one consumer that needs to do this, then I think it might be worth it.

At the moment, I'm attempting to discourage use of this.

src/tests/pkg5unittest.py

Why don't we need changes to use -R in pkgdepend_resolve(), or have its
callers pass a -R flag in their arguments?  That said, I also can't find
what changed in pkgdepend to cause us to need to file 7002405, so I
could well be mistaken here.

Ah; thanks for spotting that. So the reason we don't have to is because pkg5nuittest.py sets PKG_IMAGE in the environment to the image rooted at self.get_img_path() every time a unit test is setup.

However, I really don't want that behaviour, so I think I'll have it set to PKG_IMAGE to the test root instead (which is not an image) to force any tests that don't explicitly specify the image root to fail.

I'll work on this and get back to you,
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to