On 29.11.2015 19:25, Josh Triplett wrote:
> Andreas Cadhalpun wrote:
>> The correct way would be to choose a new 'best hit', if either
>> * there is a target release and it matches the release of the package,
>> * or there is no target release
>> and the version is higher than the last best hit.
>
> Not quite; that would fail if you have "1.1 (stable)" and "1.0 (stable)"
> in that order. You want to choose the package under consideration as
> the new 'best hit' if it matches the target release (or no target
> release exists) *and* if it has a higher version than the last best hit.
That's what I (thought I) described.
> Looking at the code, it looks like you may have implemented that, rather
> than what you describe above. However, your comments don't match that,
> and your test case doesn't test for that:
>
>> @@ -311,7 +309,7 @@ static pkgSrcRecords::Parser *FindSrc(const char *Name,
>> continue;
>>
>> // Newer version or an exact match? Save the hit
>
> This comment needs updating for the new algorithm. Something like:
> "Newer version (in the target release if any)? Save it."
Maybe.
>> - if (Last == 0 || Cache->VS().CmpVersion(Version,Ver) <
>> 0) {
>> + if (CorrectRelTag && (Last == 0 ||
>> Cache->VS().CmpVersion(Version,Ver) < 0)) {
>> Last = Parse;
>> Offset = Parse->Offset();
>> Version = Ver;
>
>> --- a/test/integration/test-apt-get-source
>> +++ b/test/integration/test-apt-get-source
>> @@ -27,6 +27,14 @@ insertsource 'stable' 'foo' 'all' '1.0'
>> insertsource 'wheezy' 'foo' 'all' '0.0.1'
>> insertsource 'wheezy' 'foo' 'all' '0.1'
>>
>> +# the order of these versions is choosen to ensure that
>
> s/choosen/chosen/
Indeed.
>> +# * apt will pick the one in the correct release, despite a higher version
>> coming later and
>> +# * apt will pick the highest version in a release, despite a lower version
>> coming later.
>> +# (bts #746412)
>> +insertsource 'stable' 'baz' 'all' '1.0'
>> +insertsource 'unstable' 'baz' 'all' '2.0'
>> +insertsource 'unstable' 'baz' 'all' '1.5'
>
> This test case doesn't ensure that apt picks the highest version in
> stable, rather than the last-mentioned version in stable.> For more
> robustness, I'd suggest:
>
> insertsource 'stable' 'baz' 'all' '0.1'
> insertsource 'stable' 'baz' 'all' '1.0'
> insertsource 'stable' 'baz' 'all' '0.5'
> insertsource 'unstable' 'baz' 'all' '1.4'
> insertsource 'unstable' 'baz' 'all' '2.0'
> insertsource 'unstable' 'baz' 'all' '1.5'
That'd be a bit redundant, but shouldn't hurt.
Best regards,
Andreas
