Just to be sure, the way the version testing works is that if an incorporation dependency is specified on [EMAIL PROTECTED] for example [EMAIL PROTECTED] would not work, but [EMAIL PROTECTED] would, correct?
constraint.py Nit 74: space after # 88: Is this just checking whether c.combine(constraint) raises a constraint exception? 90-95: I don't understand why this is right. What if an incorporation dependency on [EMAIL PROTECTED] is already in self.constraints, and the argument to update_constraints is an incorporation dependency on [EMAIL PROTECTED] If I understood combine right, this will pass the check on line 88, but then the constraint stored in self.contratints will be loosened to 1.2. What am I missing? :) 99: I think I understand what's happening here, but a doc string would be very helpful I think. Also, couldn't lines 103-104 be written as: cl.append(constraint) return reduce(constraint_combine, cl) I'd probably prefer that approach, but if you prefer the other I have no objections. 107: Have I got this right? This function takes an fmri, and applies the available constraints If the fmri isn't allowed b/c a constraint has NEVER set, then it catches the ConstraintException and raises one with better information. If the provided fmri doesn't have a high enough minimum version, then a new fmri with the version bumped is returned? 111-117: Comments or more verbose would makes this much more readable I think 174+ check_for_work: Should this function do any verification that the fmri it's passed matches the fmri it is a constraint on? I'm just wondering if a check (or assertion or something) that self.pkg_name == fmri_present.pkg_name would be a safety check. 196-197: I think these mean that the relative ordering of the constants defined on lines 124-127 matters, and the the constants aren't independent of each other. If that's the case, could you add a comment to that effect around 124? image.py Nit 46-64: while this looks nicer, i think it'll be a headache to maintain as we add or remove imports 1657: Does the spinner still work reasonably with this line removed? I didn't see it replaced anyplace similar. Load_optional_dependencies (and this seems to have a similar overhead) took enough time that having the spinner going during it was a win I think. 2091: This feels wrong to me, but maybe I'm the only one. 2151, 2154: Should 2154 be constraint_violations=constraint_violations or should 2151 be removed? t_pkg_api_install.py: 476:I don't understand the changes made here. This should still fail during plan creation unless I've misunderstood something. Nits: depend.py: 196: is there a more descriptive name than parse2 maybe? Can parse be completely replaced by parse2? Seems like it returns a superset of the information that parse does. My inclination is to say that constructing the strings in 248-254 isn't the right place to do that, but I wouldn't hold up the putback for that. api.py: 345: ']' should line up with the p in pkg_list if I've finally got the style for these things down api_errors.py: 126, 131: Extra white space imageplan.py: 275: extra whitespace Thanks, Brock Bart Smaalders wrote: > http://cr.opensolaris.org/~barts/incorp/ > > 2325 optional dependency enforcement can't handle fmris disappearing > from the catalog > 2792 Pkg dependency resolution is broken > 3043 pkg installs a minimum version without telling you why > 3227 Incorporations are broken when the package have dependencies on > incorporated package > 4308 Incorporations don't constraint forward movement > > > This wad make incorporations work, constraining both downward > and upward motion of controlled fmris. This wad also breaks > catalog rename still further, but since no one uses that... > > Examine test case changes to see what now works correctly. > > I will proceed w/ full upgrade testing tomorrow; I think > this is pretty close. > > - Bart > > > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
