On 05/01/12 19:26, Brock Pytlik wrote:
On 05/01/12 19:06, Bart Smaalders wrote:
https://cr.opensolaris.org/action/browse/pkg/barts/bug-fixes-2/
7165703 onu cannot uninstall entire
This attacks the root cause for this problem. We need to make sure we
don't expand the possible_set with packages that aren't installed in
this image, so dependency types that reference other images or don't
generate actual new dependencies should not be included when we generate
the dependency closure. I also changed the sense of the dependency type
comparison to improve performance and more easily catch problems if we
add new dependencies in the future.
- Bart
Any reason to add a test case for this change?
My previous change set (needed both for this and for other reasons)
completely masks this bug, making the test case difficult to verify.
I can think about a better (more complex) verification.
The larger question I have is whether __generate_dependencies should be
keeping a list of dependency types which have a certain property (in
this case, whether the dependency can bring another package onto the
system) instead of just asking the action whether it has this property.
To be concrete, I think I'd be happier if __generate_dependencies just
did something like (property name TBD):
if da.imports_packages:
for f in ...
And then over in depend.py have something like
@property
def imports_packages:
if da.attrs["type"] == "require" or...
return True
elif da.attrs["type"] == "incorporate" or ...
return False
else:
raise Exception("Unknown action type")
I also don't need to see this sort of change in this webrev, but would
be happy with a bug/rfe filed to change the code structure in the future.
Shawn and I talked about doing it this way. The problem is that this
code path is pretty hot, and the cost of a method call is painful here.
I did change the sense of the comparison, so a failure to update this
if needed will cause our test cases for new dependencies to fail instead
of causing mysterious problems later on.
Whether or not a particular dependency type should be used here is
pretty solver specific, so this function would have no use elsewhere.
- Bart
--
Bart Smaalders Solaris Kernel Performance
[email protected] http://blogs.oracle.com/barts
"You will contribute more with Mercurial than with Thunderbird."
"Civilization advances by extending the number of important
operations which we can perform without thinking about them."
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
