Updated webrev which resyncs with the gate and cleans up a few things at: http://cr.opensolaris.org/~bpytlik/ips-1929-v2/
Brock Brock Pytlik wrote: > Robert Milkowski wrote: > >> Hello Brock, >> >> >> See comments below. >> >> Tuesday, September 30, 2008, 5:32:02 AM, you wrote: >> >> BP> Webrev: >> BP> http://cr.opensolaris.org/~bpytlik/ips-1929-v1/ >> >> BP> Bug: >> BP> pkg list should give an option to display only current latest package >> BP> http://defect.opensolaris.org/bz/show_bug.cgi?id=1929 >> >> BP> This webrev changes the default behavior of list to display only the >> BP> most recent version of a package, and the version that's installed, if >> BP> one is. It uses the -a option to decide whether to list all packages, or >> BP> only those installed, and the -f flag to decide whether to list all >> BP> versions, or only up to two. >> >> Instead of -f which for most users looks like some kind of "force" >> option maybe --all would be better? >> >> >> > My concern is that I find it strange that -a and --all do different > things. How does --all-versions sound? My guess is that this will be a > rarely used option, so having it be a long option shouldn't be that > painful. Other options could be --versions, -V, --av... > >> BP> I don't feel strongly about changing the default behavior. I don't care >> BP> strongly about which options are used to designate what. I do think we >> BP> need two options, one for the axis of installed vs all, and one for the >> BP> axis of all versions vs up to 2. >> >> The new default (only most recent version and all installed) is a good >> thing. >> >> >> > Thanks > >> BP> Performance: >> BP> From my tests, it seems like the new code has the same speed as the >> BP> old, or is possibly slightly faster, even when directed to a file for >> BP> output, though it may have a slightly longer delay before the first >> BP> result is printed to the screen. With the -f, it's about .016 seconds >> BP> (on a base of 2.5 seconds) slower than the old pkg list, when both are >> BP> directed to files. >> >> When I was playing with it I remember that pkg.get_name() already >> provides you with packages sorted by version with a most recent one >> being returned first. So instead of doing sorting again in >> last_inventory() something like below is sufficient and probably more >> lightweight. >> >> list_inventory() >> last_name = "" >> [...] >> >> if pf != last_name: >> msg(fmt_str % (pf, pkg.get_version(), >> state["state"], ufix)) >> elif state["state"] == "installed": >> msg(fmt_str % (pf, pkg.get_version(), >> state["state"], ufix)) >> >> last_name = pf >> >> >> >> > I don't think image.inventory commits to returning its list in a > particular order. Since sorting the list seems to take about .02 > seconds, I'm inclined to not depend on that feature of inventory, unless > I hear that its ordering is going to remain stable. > > I tried something similar to what you suggested, and it actually ran a > bit slower than the existing code. Honestly, I'm not sure why because I > would've expected your approach to prove to be a bit speedier. > > Thanks for the review, > Brock > >> Best regards, >> Robert Milkowski mailto:[EMAIL PROTECTED] >> http://milek.blogspot.com >> >> >> > > _______________________________________________ > pkg-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
