On 08/03/11 16:47, Brock Pytlik wrote:
On 08/03/11 12:59, Shawn Walker wrote:
On 07/28/11 16:23, Shawn Walker wrote:
Greetings,

The following webrev contains fixes for the following issue:

18753 get_pkg_list should return only newest matching version for
LIST_NEWEST

webrev:
https://cr.opensolaris.org/action/browse/pkg/swalker/pkg-18753/webrev/

This bug was missed because the original unit tests already in place
only tested against packages with one or two versions instead of at
least three or more.

Anyone? About three lines of actual change -- the rest is a test case.

-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
I've looked at this and I guess the changes are fine. I don't really
feel like I understand why the changes made fix the problem though.

I think at least part of my problem stems from the omit_* variables.
 From what I can tell, "omit_newest" means "this package instance was
omitted because there was a newer version", but that doesn't match with
"omit_package" which seems to mean "skip this package instance entirely
for any of a number of reasons" and "omit_ver" means "skip this package
instance particularly because it's version is either isn't the latest or
isn't a successor to the pattern described". I'm especially curious why
line 3015 is omit_ver instead of omit_newest. I also wasn't clear how
omit_package could ever be anything but None unless the branch at line
2943 was taken, which seems to me to indicate that 2950-2951 could be
combined with 2956+ for code that was easier to understand and would
reduce the number of conditions we check.

I also think the comment at 3039 should be changed to include an
explanation for why we don't want to do this if we're omitting because
of newest.

I'm looking to make a minimal number of changes here, so I'm going to pass on attempted simplifications.

I've simplified the fix even further to just one line:

  https://cr.opensolaris.org/action/browse/pkg/swalker/pkg-18753-2/webrev/

In short, the bug was that if we skipped a package because a result for it had been returned before, it was mistakenly marked as being omitted because of version.

In the case that it it is omitted because of version, tracking entries are removed from a dictionary. That was causing every other version to be returned instead of only the first matching one.

-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to