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.
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.
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
