Re: apt-get source linux behaves weird
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
Re: apt-get source linux behaves weird
On 29.11.2015 14:41, David Kalnischkies wrote: > On Sun, Nov 29, 2015 at 03:17:47AM +0100, Andreas Cadhalpun wrote: >> One has to do: >> $ cd test/interactive-helper >> $ make aptwebserver > > A simple 'make' in the top-level directory builds this webserver Indeed, but somehow 'debian/rules build-arch' doesn't. >> 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? ;) > > You have never seen apt fail bigtime then. > That generates lots of waves… ;) I'm glad I haven't. ;) > And as a testrun doesn't show regressions… applied & pushed to git. Thanks. > Should be part of the next upload, which might or might not be soon > depending on how many RCs people find now to block our transition after > they had months in experimental to do it. ;) Apparently somebody found something... > Thanks for working on this! Thank you for apt 1.1, I like it. :-) Best regards, Andreas
Re: apt-get source linux behaves weird
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'
Re: apt-get source linux behaves weird
On Sun, Nov 29, 2015 at 03:17:47AM +0100, Andreas Cadhalpun wrote: > >> 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 A simple 'make' in the top-level directory builds this webserver just the same as it builds all the rest of the (apt) world [in fact, it should be the very last thing it does] which is needed in some capacity by the various testcases. That there is an explicit check for the aptwebserver is caused by a) for a while we tried getting real webservers in line, but the simple ones lack HTTP1.1 features and the bigger ones are a giant pain to setup (and do lack features as well) and b) that aptwebserver isn't shipped in a package as Debian doesn't need yet another packaged security-buggy webserver. (aka: going to fix message to reflect reality). > > 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? ;) You have never seen apt fail bigtime then. That generates lots of 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. [you usually do not have this order as Sources tends to be ordered, but you are right that assuming it is wrong.] > 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. And as a testrun doesn't show regressions… applied & pushed to git. Should be part of the next upload, which might or might not be soon depending on how many RCs people find now to block our transition after they had months in experimental to do it. ;) Thanks for working on this! Best regards David Kalnischkies signature.asc Description: PGP signature
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 , >> // 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
Re: apt-get source linux behaves weird
Control: tag -1 patch On 15.08.2015 02:13, Russ Allbery wrote: I believe the explanation is that selecting the distribution doesn't work the way that you think it does. It just changes the prioritization used for selecting packages to install, which is then ignored by the source command. (I could have the details wrong; this is vague memory from previous discussions.) Actually the explanation is that it's a bug (or two) in apt. The code responsible for this behavior is [1]: while ((Parse = SrcRecs.Find(Src.c_str(), MatchSrcOnly)) != 0) { const string Ver = Parse-Version(); // See if we need to look for a specific release tag if (RelTag != UserRequestedVerTag == ) { const string Rel = GetReleaseForSourceRecord(SrcList, Parse); if (Rel == RelTag) /// -- Here it compares the '-t' argument with the release, e.g. stable/unstable. { Last = Parse; Offset = Parse-Offset(); Version = Ver; /// -- Here it can have the correct version. :-) } } // Ignore all versions which doesn't fit if (VerTag.empty() == false Cache-VS().CmpVersion(VerTag, Ver) != 0) // exact match continue; // Newer version or an exact match? Save the hit if (Last == 0 || Cache-VS().CmpVersion(Version,Ver) 0) { Last = Parse; Offset = Parse-Offset(); Version = Ver; /// -- But here it overwrites the found version with any newer one. :-( } // was the version check above an exact match? // If so, we don't need to look further if (VerTag.empty() == false (VerTag == Ver)) break; } To fix this problem, one can add a 'break;' at the point, where apt got the correct version. Then 'apt-get -t unstable source source-pkg' works as expected, but 'apt-get -t sid source source-pkg' still doesn't work, because apt doesn't compare with the distribution codename, e.g. jessie/sid. That's not hard to fix either. It would be great if this could be fixed at some point, since it's really surprising UI behavior. Attached patch fixes both problems, so hopefully the next apt release gets this right. Best regards, Andreas 1: https://anonscm.debian.org/cgit/apt/apt.git/tree/cmdline/apt-get.cc?id=990af3c952676eaa51ccd614ab2d4234693da397#n383 --- a/cmdline/apt-get.cc +++ b/cmdline/apt-get.cc @@ -161,7 +161,7 @@ static std::string MetaIndexFileNameOnDisk(metaIndex *metaindex) // - /* */ static std::string GetReleaseForSourceRecord(pkgSourceList *SrcList, - pkgSrcRecords::Parser *Parse) + pkgSrcRecords::Parser *Parse, std::string Dist) { // try to find release const pkgIndexFile CurrentIndexFile = Parse-Index(); @@ -184,6 +184,7 @@ static std::string GetReleaseForSourceRecord(pkgSourceList *SrcList, { indexRecords records; records.Load(path); + Dist = records.GetDist(); return records.GetSuite(); } } @@ -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) { Last = Parse; Offset = Parse-Offset(); Version = Ver; + break; } }
Re: apt-get source linux behaves weird
On Sat, Aug 15, 2015 at 2:13 AM, Russ Allbery wrote: The workaround, as you discovered, is to figure out what version you want with apt-cache show and then specify it with the = syntax. Another workaround is to specify binary package names instead of source package names. Sometimes this is more useful than your workaround but probably not in this case. -- bye, pabs https://wiki.debian.org/PaulWise
Re: apt-get source linux behaves weird
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. 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. 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. If you wanna work on this further feel free to find me at DebConf (if you are here). I am the guy accompanied by super cow. Otherwise hit me on IRC (DonKult) in #debian-apt or anywhere else. That extends to all the things apt of course like bugs tagged newcomer… *hint hint* Best regards David Kalnischkies signature.asc Description: Digital signature
Re: apt-get source linux behaves weird
Hi Daniel, On 14.08.2015 08:10, Daniel Reichelt wrote: when I do 'apt-get source linux' with jessie+sid enabled in sources.list, there's no way to select jessie's ksrc version by target release. Neither of these work: - apt-get source linux - apt-get -t jessie source linux - apt-get source linux/jessie Everytime the result is: ---8--- $ apt-get source linux/jessie Reading package lists... Done Building dependency tree Reading state information... Done Selected version '4.1.3-1' (jessie) for linux Note how this line is clearly wrong, as jessie doesn't have linux 4.1.3-1. [...] Am I missing something or is this a bug in apt? Any hints are greatly appreciated. This looks very much like apt bug #746412 [1], which I reported in April last year (and which was mistakenly closed today). Best regards, Andreas 1: https://bugs.debian.org/746412
Re: apt-get source linux behaves weird
Daniel Reichelt deb...@nachtgeist.net writes: when I do 'apt-get source linux' with jessie+sid enabled in sources.list, there's no way to select jessie's ksrc version by target release. Neither of these work: - apt-get source linux - apt-get -t jessie source linux - apt-get source linux/jessie [...] Doing an 'apt-get source linux=3.16.7-ckt11-1+deb8u3' works as expected. Yeah, it's been like this forever. I've reported this before. I believe the explanation is that selecting the distribution doesn't work the way that you think it does. It just changes the prioritization used for selecting packages to install, which is then ignored by the source command. (I could have the details wrong; this is vague memory from previous discussions.) It would be great if this could be fixed at some point, since it's really surprising UI behavior. The workaround, as you discovered, is to figure out what version you want with apt-cache show and then specify it with the = syntax. -- Russ Allbery (r...@debian.org) http://www.eyrie.org/~eagle/