Re: apt-get source linux behaves weird
Control: tag -1 patch
Hi David,
On 15.08.2015 13:40, David Kalnischkies wrote:
> Control: tag -1 - patch
>
>> @@ -387,13 +388,15 @@ static pkgSrcRecords::Parser *FindSrc(const char *Name,pkgRecords &Recs,
>> // See if we need to look for a specific release tag
>> if (RelTag != "" && UserRequestedVerTag == "")
>> {
>> - const string Rel = GetReleaseForSourceRecord(SrcList, Parse);
>> + string Dist;
>> + const string Rel = GetReleaseForSourceRecord(SrcList, Parse, Dist);
>>
>> - if (Rel == RelTag)
>> + if (Rel == RelTag || Dist == RelTag)
>> {
>
> In the experimental git branch this changed completely as Release files
> have risen from the underground to be proper first class citizens (while
> Sources are still second class at best) where this was fixed by
> "accident" already in a seemingly "unrelated" commit (5ad0096a4) by me.
Since that has finally reached sid, I had another look at this bug.
>> Last = Parse;
>> Offset = Parse->Offset();
>> Version = Ver;
>> + break;
>> }
>> }
>>
>
> That 'fixes' this problem while reopening #731853 among others. Running
> the autopkgtest tests would have shown it. You can do this without
> building and installing apt packages via ./test/integration/run-tests,
> which will use the apt buildtree it is in. You need to install a bunch
> of additional test-dependencies through.
Thanks for pointing to the autopkgtests. However, some tests fail with:
"You have to build aptwerbserver or install a webserver"
But there is no aptwe*r*bserver...
One has to do:
$ cd test/interactive-helper
$ make aptwebserver
> Adding a testcase here (they are simple shell scripts with an insane
> amount of shell functions building a testing 'framework' to setup
> packages, repositories and webservers among others) wouldn't hurt the
> acceptance of a patch either.
How sane can a framework be if it has to generate packages that are "used
only by testcases and surf [...]", even though there are no waves? ;)
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.
Attached is a patch fixing this and another one adding above two
testcases.
Best regards,
Andreas
--- a/apt-private/private-source.cc
+++ b/apt-private/private-source.cc
@@ -288,6 +288,7 @@ static pkgSrcRecords::Parser *FindSrc(const char *Name,
while ((Parse = SrcRecs.Find(Src.c_str(), MatchSrcOnly)) != 0)
{
const std::string Ver = Parse->Version();
+ bool CorrectRelTag = false;
// See if we need to look for a specific release tag
if (RelTag != "" && UserRequestedVerTag == "")
@@ -297,13 +298,10 @@ static pkgSrcRecords::Parser *FindSrc(const char *Name,
{
if ((Rls->Archive != 0 && RelTag == Rls.Archive()) ||
(Rls->Codename != 0 && RelTag == Rls.Codename()))
- {
- Last = Parse;
- Offset = Parse->Offset();
- Version = Ver;
- }
+ CorrectRelTag = true;
}
- }
+ } else
+ CorrectRelTag = true;
// Ignore all versions which doesn't fit
if (VerTag.empty() == false &&
@@ -311,7 +309,7 @@ static pkgSrcRecords::Parser *FindSrc(const char *Name,
continue;
// Newer version or an exact match? Save the hit
- 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
+# * 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'
+
insertsource 'stable' 'bar' 'any' '1.1' 'Vcs-Browser: https://anonscm.debian.org/cgit/bar/bar.git
Vcs-Git: git://anonscm.debian.org/bar/bar.git -b debian/experimental'
@@ -43,6 +51,12 @@ DOWNLOAD1="Need to get 0 B/25 B of source archives.
DOWNLOAD2="Need to get 0 B/25 B of source archives.
'file://${APTARCHIVE}/foo_2.0.dsc' foo_2.0.dsc 11 SHA256:0fcb803ffbeef26db884625aaf06e75f3eda5c994634980e7c20fd37ed1fc104
'file://${APTARCHIVE}/foo_2.0.tar.gz' foo_2.0.tar.gz 14 SHA256:ca9b0b828ca22372502af2b80f61f0bd9063910ece9fc34eeaf9d9e31aa8195a"
+DOWNLOAD3="Need to get 0 B/25 B of source archives.
+'file://${APTARCHIVE}/baz_1.0.dsc' baz_1.0.dsc 11 SHA256:322245f56092b466801dda62d79c8687bba9724af6d16d450d655d29e41d3d7b
+'file://${APTARCHIVE}/baz_1.0.tar.gz' baz_1.0.tar.gz 14 SHA256:0870bc73164ff5ba1f52153fdcb48e140137f9c7c122d57592cea136a57f73c0"
+DOWNLOAD4="Need to get 0 B/25 B of source archives.
+'file://${APTARCHIVE}/baz_2.0.dsc' baz_2.0.dsc 11 SHA256:47d062d29070b3f592d1c8aed8c1e7913804bbb67ca1d64877c8219dac5e0420
+'file://${APTARCHIVE}/baz_2.0.tar.gz' baz_2.0.tar.gz 14 SHA256:11c1b202c94a64ab6433d9f0ed5515fce1dc7b20e6bcf51cec9ef8b9455f5a41"
testsuccessequal "$HEADER
$DOWNLOAD2" aptget source -q --print-uris foo
testsuccessequal "$HEADER
@@ -61,6 +75,9 @@ $DOWNLOAD1" aptget source -q --print-uris foo -t stable
testsuccessequal "$HEADER
Selected version '2.0' (unstable) for foo
$DOWNLOAD2" aptget source -q --print-uris foo -t unstable
+testsuccessequal "$HEADER
+Selected version '1.0' (stable) for baz
+$DOWNLOAD3" aptget source -q --print-uris baz -t stable
# select by release: codename
testsuccessequal "$HEADER
@@ -69,6 +86,9 @@ $DOWNLOAD2" aptget source -q --print-uris foo/sid
testsuccessequal "$HEADER
Selected version '2.0' (sid) for foo
$DOWNLOAD2" aptget source -q --print-uris foo -t sid
+testsuccessequal "$HEADER
+Selected version '2.0' (sid) for baz
+$DOWNLOAD4" aptget source -q --print-uris baz -t sid
# select by version
testsuccessequal "$HEADER
Reply to: