On 03/24/10 05:19 PM, Ethan Quach wrote: > > > On 03/24/10 15:19, sanjay nadkarni (Laptop) wrote: >> On 03/24/10 02:31 PM, Ethan Quach wrote: >>> Hey Sanjay, >>> >>> [path to updated CR lacks your ~userid btw] >>> >>> >>> Couple of questions: >>> >>> 1231 - Is this going to work with hierarchical package names? e.g. >>> >>> pkg://opensolaris.org/system/library at 0.5.11,5.11-0.135:20100312T101036Z >>> >>> >>> 1217-1235 - It seems to me that with this code, the last instance of a >>> package (for the edge and perhaps error case that a package might be >>> specified more than once for whatever reason) would just take the last >>> instance of the package and throw way earlier ones? >> Taken by itself that is true. However, even before this specific code is >> called, the entire list of pkgs are validated and such errors will be >> caught hence it is reasonable to assume that we have a valid list when >> this function is called. >> >> Does that help ? > > Yes, it does. Could we add a comment stating that that is what this > code does then? Its just that its not obvious looking at the code, and > having that would be helpful down the line in case we need to debug > something in this realm. > > (and while I'm not aware of where we actually do sanitize the pkglist for > the AI case before getting here, I agree that this code wouldn't be the > place do that level of validation anyway.) > > > And so given the above, one thing to point out is that a pkglist with > repeated pkgs would have previously been processed by transfer by > successively calling pkg install on each of them. For example, if in the > list we had > > SUNWfoo at 5.2 > SUNWfoo at 6.5 > SUNWfoo at 4.3 > > transfer would have simply tried to install all of them (I'm not even > sure > what the behavior of pkg would be when the latter two are called to get > installed) > > But with our changes here, we're kindof silently ignoring the first two, > and just installing the last. > > If this is a supported list, I'm wondering if we shouldn't just pass all > three of these to 'pkg' in the batch, and have it decide what to do. > Seems that could be achievable if the values in the pkgdict were a > list per each key, instead of a single value. I am not sure if that is valid or not ? Having a list for pkgs is not a bad idea if that is the case.
> > If such a pkglist is not supported, then we need something earlier in > AI to validate against it. If this is the case, I wouldn't expect > that to > be addressed with your changes, but a separate bug should be filed. > This entire code is a workaround for the ordering issue in IPS and hopefully will be deleted soon (see the comment!) Please let me know if you want me to address this. -Sanjay > thanks, > -ethan > > >> >> -Sanjay >> >> >>> >>> >>> thanks, >>> -ethan >>> >>> >>> >>> On 03/24/10 11:37, sanjay nadkarni (Laptop) wrote: >>>> The code review with the change is now available at: >>>> http://cr.opensolaris.org/bugfix_15132 >>>> >>>> -Sanjay >>>> >>>> >>>> On 03/24/10 09:52 AM, sanjay nadkarni wrote: >>>>> On 03/24/10 07:47 AM, Dave Miner wrote: >>>>>> >>>>>>>> 1249-52: I'd think you could turn this into simply >>>>>>>> >>>>>>>> pkgs = " ".join(order.values()) >>>>>>>> >>>>>>> That will not work, since order is a list of keys not a dictionary. >>>>>>> >>>>>> >>>>>> Yeah, missed that. Fine then. >>>>>> >>>>>> Dave >>>>> I have a fix that addresses the primary concern that you expressed >>>>> i.e. will keys work if the pkg is expressed as a complete FMRI and I >>>>> will be posting the webrev soon. >>>>> >>>>> The supported formats can be any of the following. I am using entire >>>>> as an example here. >>>>> entire >>>>> entire at [email protected],5.11-0.134 >>>>> entire at 0.5.11,5.11-0.134:20100302T023003Z >>>>> pkg://pkg.opensolaris.org/entire at 0.5.11,5.11-0.134 >>>>> pkg://pkg.opensolaris.org/entire at 0.5.11,5.11-0.134:20100302T023003Z >>>>> >>>>> -Sanjay >>>>> >>>>> >>>>> >>>>> -Sanjay >>>>> >>>>> _______________________________________________ >>>>> caiman-discuss mailing list >>>>> caiman-discuss at opensolaris.org >>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
