Re: apt-get source linux behaves weird

[Josh Triplett <josh@joshtriplett.org>]

Andreas Cadhalpun wrote:
> The relevant testcases are in test/integration/test-apt-get-source.
> There is a test for #731853 that is supposed to "ensure that apt will
> pick the higher version number" of 0.0.1 (stable) and 0.1 (stable).
> However, this works by pure chance, as simply reversing the order
> of the two insertsource lines makes the test fail.
> So #731853 isn't really fixed, yet.
> 
> Actually, that's related to the problem I reported, as the underlying
> issue for both is the same:
> In the FindSrc function apt chooses a new 'best hit', if either
>  * there is a target release and it matches the release of the package,
>  * or the version of the package is higher than the last best hit.
> 
> Consider having 1.0 (stable), 2.0 (unstable) and 1.5 (unstable),
> in this order.
> 
> Looking for the version in stable, apt first selects 1.0, because the
> release matches the target release, but then subsequently selects 2.0,
> because the version is higher.
> 
> Looking for the version in unstable, apt first selects 2.0, because the
> release matches the target release, but then subsequently selects 1.5,
> because the release also matches the target release.
> 
> 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.

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

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

> +# * 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'