On 02/23/12 09:13, Shawn Walker wrote:
[snip]
(also all changes in pkgdep)
I'd appreciate actual changes in functionality not to get rolled into a
bug dealing only with performance improvements.
[snip]
This change was originally necessary because of the calls to
consolidate_attrs. It caused the pkgdep test case to fail, so it was
necessary.
I'm not disputing whether or not the change was necessary. However, I
would expect a changeset only labeled as "performance improvements" not
to contain any actual change in behavior. This clearly does have a
change in the behavior of pkgdepend.
Once a bug has been filed, I'm ok w/ this change in functionality,
though I'm not particularly sure why it's needed. (Perhaps that can be
included in the bug.)
As a side note, this change does actually contradict what we say in our
section 5 manpage, which is that the multiply listed attributes are
treated as unordered lists, not sets. Now, do I think it's both silly
and unlikely for someone to depend on how many times a particular
attribute name/value pair appears? Yes. But these changes do appear to
change our behavior on that issue.
I disagree. The consolidate_attrs thing was there long before I came
along to make this change. And I'm not changing the values into sets,
I'm changing them from sets into lists.
Sorry, let me be clearer hear. Our man page says, "unordered lists" that
suggests that a value can appear in a list more than once, since "list"
contains no notion of uniqueness or eliminating duplicates. By calling
"consolidate_attrs" in new places, there were previously things that
acted like "lists" that now act like "sets".
Either way, because of other changes made during the review I'm now
able to revert the changes to pkgdep.py and *some* of the changes to
t_pkgdep.py.
imageplan.py:
1810-1811: a comment here about why we're doing something that appears
to be more work than is necessary would help. I assume it's because,
usually, oactions is empty and that check is so much faster than finding
the length of oactions ... but then I would've expected that finding the
length of an empty list to be rather fast itself...
[snip]
generic.py:
599-600: I assume you also compared this with using an if/elsif
construction?
The one it was using previously? If so, see my last reply to Ed with
get_new_varcet_keys() comparisons.
Um, no. Previous code:
for ....
if ...
do stuff
if ...
do stuff
Code I was asking for comparison against:
for ...
if ...
do stuff
elsif ...
do stuff
-Shawn
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
