From b5595da902e62af7c295f1603ae5b43ba4cef281 Mon Sep 17 00:00:00 2001 From: "bubulle@debian.org" <> Date: Wed, 10 Apr 2013 11:28:11 +0200 Subject: Fix English spelling error in a message ('A error'). Unfuzzy translations. Closes: #705087 --- apt-pkg/acquire-item.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index d12733747..7b800e42f 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1515,7 +1515,7 @@ void pkgAcqMetaIndex::Failed(string Message,pkgAcquire::MethodConfig *Cnf) VerifiedSigFile.append(".gpg"); Rename(LastGoodSigFile, VerifiedSigFile); Status = StatTransientNetworkError; - _error->Warning(_("A error occurred during the signature " + _error->Warning(_("An error occurred during the signature " "verification. The repository is not updated " "and the previous index files will be used. " "GPG error: %s: %s\n"), -- cgit v1.2.3-70-g09d2 From 1dea08eb2e1115b8da14cc3da02d53f8e069ba14 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 8 May 2013 17:45:17 +0200 Subject: properly handle if-modfied-since with libcurl/https (closes: #705648) --- apt-pkg/acquire-worker.cc | 13 ++++++++++--- debian/changelog | 8 ++++++++ debian/control | 2 +- methods/https.cc | 7 ++++++- 4 files changed, 25 insertions(+), 5 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-worker.cc b/apt-pkg/acquire-worker.cc index 9d90b08bc..44a84216a 100644 --- a/apt-pkg/acquire-worker.cc +++ b/apt-pkg/acquire-worker.cc @@ -305,7 +305,15 @@ bool pkgAcquire::Worker::RunMessages() OwnerQ->ItemDone(Itm); unsigned long long const ServerSize = strtoull(LookupTag(Message,"Size","0").c_str(), NULL, 10); - if (TotalSize != 0 && ServerSize != TotalSize) + bool isHit = StringToBool(LookupTag(Message,"IMS-Hit"),false) || + StringToBool(LookupTag(Message,"Alt-IMS-Hit"),false); + // Using the https method the server might return 200, but the + // If-Modified-Since condition is not satsified, libcurl will + // discard the download. In this case, however, TotalSize will be + // set to the actual size of the file, while ServerSize will be set + // to 0. Therefore, if the item is marked as a hit and the + // downloaded size (ServerSize) is 0, we ignore TotalSize. + if (TotalSize != 0 && (!isHit || ServerSize != 0) && ServerSize != TotalSize) _error->Warning("Size of file %s is not what the server reported %s %llu", Owner->DestFile.c_str(), LookupTag(Message,"Size","0").c_str(),TotalSize); @@ -332,8 +340,7 @@ bool pkgAcquire::Worker::RunMessages() // Log that we are done if (Log != 0) { - if (StringToBool(LookupTag(Message,"IMS-Hit"),false) == true || - StringToBool(LookupTag(Message,"Alt-IMS-Hit"),false) == true) + if (isHit) { /* Hide 'hits' for local only sources - we also manage to hide gets */ diff --git a/debian/changelog b/debian/changelog index 9ed9b4d61..db7a8ffda 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,11 @@ +apt (0.9.7.9) UNRELEASED; urgency=low + + [ Ludovico Cavedon ] + * properly handle if-modfied-since with libcurl/https + (closes: #705648) + + -- Michael Vogt Wed, 08 May 2013 16:20:13 +0200 + apt (0.9.7.8) unstable; urgency=criticial * SECURITY UPDATE: InRelease verification bypass diff --git a/debian/control b/debian/control index 50b3599fc..4a73239f7 100644 --- a/debian/control +++ b/debian/control @@ -7,7 +7,7 @@ Uploaders: Michael Vogt , Otavio Salvador , Julian Andres Klode Standards-Version: 3.9.3 Build-Depends: dpkg-dev (>= 1.15.8), debhelper (>= 8.1.3~), libdb-dev, - gettext (>= 0.12), libcurl4-gnutls-dev (>= 7.19.0), + gettext (>= 0.12), libcurl4-gnutls-dev (>= 7.19.4~), zlib1g-dev, libbz2-dev, xsltproc, docbook-xsl, docbook-xml, po4a (>= 0.34-2), autotools-dev, autoconf, automake Build-Depends-Indep: doxygen, debiandoc-sgml diff --git a/methods/https.cc b/methods/https.cc index c1a49ba60..d85415b2f 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -285,6 +285,11 @@ bool HttpsMethod::Fetch(FetchItem *Itm) long curl_servdate; curl_easy_getinfo(curl, CURLINFO_FILETIME, &curl_servdate); + // If the server returns 200 OK but the If-Modified-Since condition is not + // met, CURLINFO_CONDITION_UNMET will be set to 1 + long curl_condition_unmet = 0; + curl_easy_getinfo(curl, CURLINFO_CONDITION_UNMET, &curl_condition_unmet); + File->Close(); // cleanup @@ -312,7 +317,7 @@ bool HttpsMethod::Fetch(FetchItem *Itm) Res.Filename = File->Name(); Res.LastModified = Buf.st_mtime; Res.IMSHit = false; - if (curl_responsecode == 304) + if (curl_responsecode == 304 || curl_condition_unmet) { unlink(File->Name().c_str()); Res.IMSHit = true; -- cgit v1.2.3-70-g09d2 From dec5b117052b77e4366efd8234e0cec09989b700 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 8 May 2013 17:46:31 +0200 Subject: * apt-pkg/algorithms.cc: - Do not propagate negative scores from rdepends. Propagating the absolute value of a negative score may boost obsolete packages and keep them installed instead of installing their successors. (Closes: #699759) --- apt-pkg/algorithms.cc | 5 ++++- debian/changelog | 8 +++++++- 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/algorithms.cc b/apt-pkg/algorithms.cc index 8cd9d4c6e..991d425e3 100644 --- a/apt-pkg/algorithms.cc +++ b/apt-pkg/algorithms.cc @@ -645,7 +645,10 @@ void pkgProblemResolver::MakeScores() D->Type != pkgCache::Dep::Recommends)) continue; - Scores[I->ID] += abs(OldScores[D.ParentPkg()->ID]); + // Do not propagate negative scores otherwise + // an extra (-2) package might score better than an optional (-1) + if (OldScores[D.ParentPkg()->ID] > 0) + Scores[I->ID] += OldScores[D.ParentPkg()->ID]; } } diff --git a/debian/changelog b/debian/changelog index db7a8ffda..7775c1b79 100644 --- a/debian/changelog +++ b/debian/changelog @@ -4,7 +4,13 @@ apt (0.9.7.9) UNRELEASED; urgency=low * properly handle if-modfied-since with libcurl/https (closes: #705648) - -- Michael Vogt Wed, 08 May 2013 16:20:13 +0200 + [ Andreas Beckman ] + * apt-pkg/algorithms.cc: + - Do not propagate negative scores from rdepends. Propagating the absolute + value of a negative score may boost obsolete packages and keep them + installed instead of installing their successors. (Closes: #699759) + + -- Michael Vogt Wed, 08 May 2013 17:46:10 +0200 apt (0.9.7.8) unstable; urgency=criticial -- cgit v1.2.3-70-g09d2 From 5b63d2a9a2e088bb7df7c703e9452af7efc88210 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 8 May 2013 17:50:15 +0200 Subject: merged patch from Daniel Hartwig to fix URI and proxy releated issues --- apt-pkg/contrib/strutl.cc | 9 +++++---- debian/changelog | 17 +++++++++++++++++ methods/http.cc | 14 +++++++------- methods/https.cc | 20 +++++++++++++++++++- .../test-bug-595691-empty-and-broken-archive-files | 14 +++++++------- test/integration/test-releasefile-verification | 4 ++-- test/libapt/uri_test.cc | 8 ++++++++ 7 files changed, 65 insertions(+), 21 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index 03b98e93e..f4dd3407d 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -1483,9 +1483,12 @@ URI::operator string() if (User.empty() == false) { - Res += User; + // FIXME: Technically userinfo is permitted even less + // characters than these, but this is not conveniently + // expressed with a blacklist. + Res += QuoteString(User, ":/?#[]@"); if (Password.empty() == false) - Res += ":" + Password; + Res += ":" + QuoteString(Password, ":/?#[]@"); Res += "@"; } @@ -1524,7 +1527,6 @@ string URI::SiteOnly(const string &URI) U.User.clear(); U.Password.clear(); U.Path.clear(); - U.Port = 0; return U; } /*}}}*/ @@ -1536,7 +1538,6 @@ string URI::NoUserPassword(const string &URI) ::URI U(URI); U.User.clear(); U.Password.clear(); - U.Port = 0; return U; } /*}}}*/ diff --git a/debian/changelog b/debian/changelog index 182596b62..d5ae8448b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -50,6 +50,23 @@ apt (0.9.8) UNRELEASED; urgency=low [ Manpages translations ] * French translation completed (Christian Perrier) + [ Daniel Hartwig ] + * apt-pkg/contrib/strutl.cc: + - include port in shortened URIs (e.g. with apt-cache policy, progress + display) thanks to James McCoy (Closes: #154868, #322074) + - percent-encode username and password when writing URIs + * methods/http.cc: + - properly escape IP-literals (e.g. IPv6 address) when building + Host headers and URIs (Closes: #620344) + * methods/https.cc: + - use https_proxy environment variable if present, falling back to + http_proxy otherwise + - use authentication credentials from proxy URI + (Closes: #651640, LP: #1087512) + - environment variables do not override an explicit no proxy + directive ("DIRECT") in apt.conf + - disregard all_proxy environment variable, like other methods + -- Michael Vogt Mon, 08 Apr 2013 08:43:21 +0200 apt (0.9.7.9~exp2) experimental; urgency=low diff --git a/methods/http.cc b/methods/http.cc index fddf8a78e..db1085a2d 100644 --- a/methods/http.cc +++ b/methods/http.cc @@ -667,7 +667,12 @@ void HttpMethod::SendReq(FetchItem *Itm,CircleBuf &Out) // The HTTP server expects a hostname with a trailing :port char Buf[1000]; - string ProperHost = Uri.Host; + string ProperHost; + + if (Uri.Host.find(':') != string::npos) + ProperHost = '[' + Uri.Host + ']'; + else + ProperHost = Uri.Host; if (Uri.Port != 0) { sprintf(Buf,":%u",Uri.Port); @@ -975,12 +980,7 @@ HttpMethod::DealWithHeaders(FetchResult &Res,ServerState *Srv) { URI Uri = Queue->Uri; if (Uri.Host.empty() == false) - { - if (Uri.Port != 0) - strprintf(NextURI, "http://%s:%u", Uri.Host.c_str(), Uri.Port); - else - NextURI = "http://" + Uri.Host; - } + NextURI = URI::SiteOnly(Uri); else NextURI.clear(); NextURI.append(DeQuoteString(Srv->Location)); diff --git a/methods/https.cc b/methods/https.cc index b44642ab2..84ce2d68f 100644 --- a/methods/https.cc +++ b/methods/https.cc @@ -63,6 +63,12 @@ void HttpsMethod::SetupProxy() /*{{{*/ { URI ServerName = Queue->Uri; + // Curl should never read proxy settings from the environment, as + // we determine which proxy to use. Do this for consistency among + // methods and prevent an environment variable overriding a + // no-proxy ("DIRECT") setting in apt.conf. + curl_easy_setopt(curl, CURLOPT_PROXY, ""); + // Determine the proxy setting - try https first, fallback to http and use env at last string UseProxy = _config->Find("Acquire::https::Proxy::" + ServerName.Host, _config->Find("Acquire::http::Proxy::" + ServerName.Host).c_str()); @@ -81,7 +87,14 @@ void HttpsMethod::SetupProxy() /*{{{*/ if (getenv("no_proxy") != 0 && CheckDomainList(ServerName.Host,getenv("no_proxy")) == true) return; } else { - const char* result = getenv("http_proxy"); + const char* result = getenv("https_proxy"); + // FIXME: Fall back to http_proxy is to remain compatible with + // existing setups and behaviour of apt.conf. This should be + // deprecated in the future (including apt.conf). Most other + // programs do not fall back to http proxy settings and neither + // should Apt. + if (result == NULL) + result = getenv("http_proxy"); UseProxy = result == NULL ? "" : result; } @@ -92,6 +105,11 @@ void HttpsMethod::SetupProxy() /*{{{*/ if (Proxy.Port != 1) curl_easy_setopt(curl, CURLOPT_PROXYPORT, Proxy.Port); curl_easy_setopt(curl, CURLOPT_PROXY, Proxy.Host.c_str()); + if (Proxy.User.empty() == false || Proxy.Password.empty() == false) + { + curl_easy_setopt(curl, CURLOPT_PROXYUSERNAME, Proxy.User.c_str()); + curl_easy_setopt(curl, CURLOPT_PROXYPASSWORD, Proxy.Password.c_str()); + } } } /*}}}*/ // HttpsMethod::Fetch - Fetch an item /*{{{*/ diff --git a/test/integration/test-bug-595691-empty-and-broken-archive-files b/test/integration/test-bug-595691-empty-and-broken-archive-files index 4611b8b8e..a05ed5fa6 100755 --- a/test/integration/test-bug-595691-empty-and-broken-archive-files +++ b/test/integration/test-bug-595691-empty-and-broken-archive-files @@ -103,23 +103,23 @@ testoverhttp() { setupcompressor "$1" createemptyfile 'en' - testaptgetupdate "Get: http://localhost Packages [] -Get: http://localhost Translation-en + testaptgetupdate "Get: http://localhost:8080 Packages [] +Get: http://localhost:8080 Translation-en Reading package lists..." "empty file en.$COMPRESS over http" createemptyarchive 'en' - testaptgetupdate "Get: http://localhost Packages [] -Get: http://localhost Translation-en [] + testaptgetupdate "Get: http://localhost:8080 Packages [] +Get: http://localhost:8080 Translation-en [] Reading package lists..." "empty archive en.$COMPRESS over http" createemptyarchive 'Packages' - testaptgetupdate "Get: http://localhost Packages [] + testaptgetupdate "Get: http://localhost:8080 Packages [] Reading package lists..." "empty archive Packages.$COMPRESS over http" createemptyfile 'Packages' #FIXME: we should response with a good error message instead - testaptgetupdate "Get: http://localhost Packages -Err http://localhost Packages + testaptgetupdate "Get: http://localhost:8080 Packages +Err http://localhost:8080 Packages Empty files can't be valid archives W: Failed to fetch ${COMPRESSOR}:$(readlink -f rootdir/var/lib/apt/lists/partial/localhost:8080_Packages) Empty files can't be valid archives diff --git a/test/integration/test-releasefile-verification b/test/integration/test-releasefile-verification index 01fb2e529..fba7ab290 100755 --- a/test/integration/test-releasefile-verification +++ b/test/integration/test-releasefile-verification @@ -37,7 +37,7 @@ The following NEW packages will be installed: apt 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. After this operation, 5370 kB of additional disk space will be used. -Get:1 http://localhost/ apt 0.7.25.3 +Get:1 http://localhost:8080/ apt 0.7.25.3 Download complete and in download only mode' aptget install apt -dy } @@ -50,7 +50,7 @@ The following NEW packages will be installed: apt 0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. After this operation, 5808 kB of additional disk space will be used. -Get:1 http://localhost/ apt 0.8.0~pre1 +Get:1 http://localhost:8080/ apt 0.8.0~pre1 Download complete and in download only mode' aptget install apt -dy } diff --git a/test/libapt/uri_test.cc b/test/libapt/uri_test.cc index 99bb3067e..16fde503f 100644 --- a/test/libapt/uri_test.cc +++ b/test/libapt/uri_test.cc @@ -108,5 +108,13 @@ int main() { equals("/debian/", U.Path); } + // Percent-encoding. + { + URI U("ftp://foo:b%40r@example.org"); + equals("foo", U.User); + equals("b@r", U.Password); + equals("ftp://foo:b%40r@example.org", (std::string) U); + } + return 0; } -- cgit v1.2.3-70-g09d2 From 27cc55ee824913aa3687cc3adf91d96e89bd27d0 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 11 May 2013 10:10:07 +0200 Subject: non-inline RunGPGV methods to restore ABI compatibility with previous versions to fix partial upgrades (Closes: #707771) The rename in 0.9.7.9~exp2 moved the method body to the class definition which means it became inline, which isn't ABI compatibile. The reverse of moving inline to non-inline is safe though. --- apt-pkg/indexcopy.cc | 12 ++++++++++++ apt-pkg/indexcopy.h | 10 ++-------- debian/changelog | 9 +++++++++ 3 files changed, 23 insertions(+), 8 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/indexcopy.cc b/apt-pkg/indexcopy.cc index a262ef789..0e36b3ded 100644 --- a/apt-pkg/indexcopy.cc +++ b/apt-pkg/indexcopy.cc @@ -642,6 +642,18 @@ bool SigVerify::CopyAndVerify(string CDROM,string Name,vector &SigList, return true; } /*}}}*/ +// SigVerify::RunGPGV - deprecated wrapper calling ExecGPGV /*{{{*/ +bool SigVerify::RunGPGV(std::string const &File, std::string const &FileOut, + int const &statusfd, int fd[2]) { + ExecGPGV(File, FileOut, statusfd, fd); + return false; +}; +bool SigVerify::RunGPGV(std::string const &File, std::string const &FileOut, + int const &statusfd) { + ExecGPGV(File, FileOut, statusfd); + return false; +}; + /*}}}*/ bool TranslationsCopy::CopyTranslations(string CDROM,string Name, /*{{{*/ vector &List, pkgCdromStatus *log) { diff --git a/apt-pkg/indexcopy.h b/apt-pkg/indexcopy.h index aa221158e..e6a07a887 100644 --- a/apt-pkg/indexcopy.h +++ b/apt-pkg/indexcopy.h @@ -100,15 +100,9 @@ class SigVerify /*{{{*/ std::vector PkgList,std::vector SrcList); __deprecated static bool RunGPGV(std::string const &File, std::string const &FileOut, - int const &statusfd, int fd[2]) { - ExecGPGV(File, FileOut, statusfd, fd); - return false; - }; + int const &statusfd, int fd[2]); __deprecated static bool RunGPGV(std::string const &File, std::string const &FileOut, - int const &statusfd = -1) { - ExecGPGV(File, FileOut, statusfd); - return false; - }; + int const &statusfd = -1); }; /*}}}*/ diff --git a/debian/changelog b/debian/changelog index d5ae8448b..6ccbd37c4 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +apt (0.9.8.1) UNRELEASED; urgency=low + + [ David Kalnischkies ] + * apt-pkg/indexcopy.cc: + - non-inline RunGPGV methods to restore ABI compatibility with previous + versions to fix partial upgrades (Closes: #707771) + + -- David Kalnischkies Sat, 11 May 2013 09:53:59 +0200 + apt (0.9.8) UNRELEASED; urgency=low [ Ludovico Cavedon ] -- cgit v1.2.3-70-g09d2 From e3b402f40373365e169f30d276e467f708074ffc Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 6 Jun 2013 18:17:14 +0200 Subject: fix double free (closes: #711045) --- apt-pkg/contrib/fileutl.cc | 7 +++++-- debian/changelog | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index f18e17005..46661887a 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1424,8 +1424,11 @@ bool FileFd::Seek(unsigned long long To) return _error->Error("Reopen is only implemented for read-only files!"); } #ifdef HAVE_BZ2 - if (d->bz2 != NULL) - BZ2_bzclose(d->bz2); + if (d->bz2 != NULL) + { + BZ2_bzclose(d->bz2); + d->bz2 = NULL; + } #endif if (iFd != -1) close(iFd); diff --git a/debian/changelog b/debian/changelog index f5be97bd3..f0eb0421d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -9,6 +9,9 @@ apt (0.9.8.2) UNRELEASED; urgency=low [ Michael Vogt ] * buildlib/apti18n.h.in: - fix build failure when building without NLS (closes: #671587) + + [ Gregoire Menuel ] + * fix double free (closes: #711045) -- Christian Perrier Thu, 16 May 2013 22:28:22 +0200 -- cgit v1.2.3-70-g09d2 From 2de7157775551185beb3d7cb56e9f9f353e57ab4 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 21 May 2013 00:05:14 +0200 Subject: rewrite pkgOrderList::DepRemove to stop incorrect immediate setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some squeeze → wheezy upgrades indicate that DepRemove runs amok in complicated setups as it wasn't correctly working with or-groups. Completely rewritten the check is now moving from or-group to or-group instead. The behavior should be the same as the code before, but (hopefully) with less bugs and more comments. Closes: 645713 --- apt-pkg/orderlist.cc | 219 ++++++++++++++++++++++++--------------------------- debian/changelog | 2 + 2 files changed, 105 insertions(+), 116 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/orderlist.cc b/apt-pkg/orderlist.cc index 80d8fd490..86d2a9478 100644 --- a/apt-pkg/orderlist.cc +++ b/apt-pkg/orderlist.cc @@ -893,149 +893,136 @@ bool pkgOrderList::DepConfigure(DepIterator D) /*}}}*/ // OrderList::DepRemove - Removal ordering /*{{{*/ // --------------------------------------------------------------------- -/* Removal visits all reverse depends. It considers if the dependency - of the Now state version to see if it is okay with removing this - package. This check should always fail, but is provided for symetery - with the other critical handlers. - - Loops are preprocessed and logged. Removal loops can also be - detected in the critical handler. They are characterized by an - old version of A depending on B but the new version of A conflicting - with B, thus either A or B must break to install. */ -bool pkgOrderList::DepRemove(DepIterator D) +/* Checks all given dependencies if they are broken by the remove of a + package and if so fix it by visiting another provider or or-group + member to ensure that the dependee keeps working which is especially + important for Immediate packages like e.g. those depending on an + awk implementation. If the dependency can't be fixed with another + package this means an upgrade of the package will solve the problem. */ +bool pkgOrderList::DepRemove(DepIterator Broken) { - if (D.Reverse() == false) + if (Broken.Reverse() == false) return true; - for (; D.end() == false; ++D) - if (D->Type == pkgCache::Dep::Depends || D->Type == pkgCache::Dep::PreDepends) + + for (; Broken.end() == false; ++Broken) + { + if (Broken->Type != pkgCache::Dep::Depends && + Broken->Type != pkgCache::Dep::PreDepends) + continue; + + PkgIterator BrokenPkg = Broken.ParentPkg(); + // uninstalled packages can't break via a remove + if (BrokenPkg->CurrentVer == 0) + continue; + + // if its already added, we can't do anything useful + if (IsFlag(BrokenPkg, AddPending) == true || IsFlag(BrokenPkg, Added) == true) + continue; + + // if the dependee is going to be removed, visit it now + if (Cache[BrokenPkg].Delete() == true) + return VisitNode(BrokenPkg, "Remove-Dependee"); + + // The package stays around, so find out how this is possible + for (DepIterator D = BrokenPkg.CurrentVer().DependsList(); D.end() == false;) { - // Duplication elimination, consider the current version only - if (D.ParentPkg().CurrentVer() != D.ParentVer()) + // only important or-groups need fixing + if (D->Type != pkgCache::Dep::Depends && + D->Type != pkgCache::Dep::PreDepends) + { + ++D; continue; + } - /* We wish to see if the dep on the parent package is okay - in the removed (install) state of the target pkg. */ - bool tryFixDeps = false; - if (CheckDep(D) == true) + // Start is the beginning of the or-group, D is the first one after or + DepIterator Start = D; + bool foundBroken = false; + for (bool LastOR = true; D.end() == false && LastOR == true; ++D) { - // We want to catch loops with the code below. - if (IsFlag(D.ParentPkg(),AddPending) == false) - continue; + LastOR = (D->CompareOp & pkgCache::Dep::Or) == pkgCache::Dep::Or; + if (D == Broken) + foundBroken = true; } - else - tryFixDeps = true; - // This is the loop detection - if (IsFlag(D.ParentPkg(),Added) == true || - IsFlag(D.ParentPkg(),AddPending) == true) - { - if (IsFlag(D.ParentPkg(),AddPending) == true) - AddLoop(D); + // this or-group isn't the broken one: keep searching + if (foundBroken == false) continue; + + // iterate over all members of the or-group searching for a ready replacement + bool readyReplacement = false; + for (DepIterator OrMember = Start; OrMember != D && readyReplacement == false; ++OrMember) + { + Version ** Replacements = OrMember.AllTargets(); + for (Version **R = Replacements; *R != 0; ++R) + { + VerIterator Ver(Cache,*R); + // only currently installed packages can be a replacement + PkgIterator RPkg = Ver.ParentPkg(); + if (RPkg.CurrentVer() != Ver) + continue; + + // packages going to be removed can't be a replacement + if (Cache[RPkg].Delete() == true) + continue; + + readyReplacement = true; + break; + } + delete[] Replacements; } - if (tryFixDeps == true) + // something else is ready to take over, do nothing + if (readyReplacement == true) + continue; + + // see if we can visit a replacement + bool visitReplacement = false; + for (DepIterator OrMember = Start; OrMember != D && visitReplacement == false; ++OrMember) { - for (pkgCache::DepIterator F = D.ParentPkg().CurrentVer().DependsList(); - F.end() == false; ++F) + Version ** Replacements = OrMember.AllTargets(); + for (Version **R = Replacements; *R != 0; ++R) { - if (F->Type != pkgCache::Dep::Depends && F->Type != pkgCache::Dep::PreDepends) + VerIterator Ver(Cache,*R); + // consider only versions we plan to install + PkgIterator RPkg = Ver.ParentPkg(); + if (Cache[RPkg].Install() == false || Cache[RPkg].InstallVer != Ver) continue; - // Check the Providers - if (F.TargetPkg()->ProvidesList != 0) - { - pkgCache::PrvIterator Prov = F.TargetPkg().ProvidesList(); - for (; Prov.end() == false; ++Prov) - { - pkgCache::PkgIterator const P = Prov.OwnerPkg(); - if (IsFlag(P, InList) == true && - IsFlag(P, AddPending) == true && - IsFlag(P, Added) == false && - Cache[P].InstallVer == 0) - break; - } - if (Prov.end() == false) - for (pkgCache::PrvIterator Prv = F.TargetPkg().ProvidesList(); - Prv.end() == false; ++Prv) - { - pkgCache::PkgIterator const P = Prv.OwnerPkg(); - if (IsFlag(P, InList) == true && - IsFlag(P, AddPending) == false && - Cache[P].InstallVer != 0 && - VisitNode(P, "Remove-P") == true) - { - Flag(P, Immediate); - tryFixDeps = false; - break; - } - } - if (tryFixDeps == false) - break; - } - // Check for Or groups - if ((F->CompareOp & pkgCache::Dep::Or) != pkgCache::Dep::Or) + // loops are not going to help us, so don't create them + if (IsFlag(RPkg, AddPending) == true) continue; - // Lets see if the package is part of the Or group - pkgCache::DepIterator S = F; - for (; S.end() == false; ++S) + + if (IsMissing(RPkg) == true) + continue; + + visitReplacement = true; + if (IsFlag(BrokenPkg, Immediate) == false) { - if (S.TargetPkg() == D.TargetPkg()) + if (VisitNode(RPkg, "Remove-Rep") == true) break; - if ((S->CompareOp & pkgCache::Dep::Or) != pkgCache::Dep::Or || - CheckDep(S)) // Or group is satisfied by another package - for (;S.end() == false; ++S); } - if (S.end() == true) - continue; - // skip to the end of the or group - for (;S.end() == false && (S->CompareOp & pkgCache::Dep::Or) == pkgCache::Dep::Or; ++S); - ++S; - // The soon to be removed is part of the Or group - // start again in the or group and find something which will serve as replacement - for (; F.end() == false && F != S; ++F) + else { - if (IsFlag(F.TargetPkg(), InList) == true && - IsFlag(F.TargetPkg(), AddPending) == false && - Cache[F.TargetPkg()].InstallVer != 0 && - VisitNode(F.TargetPkg(), "Remove-Target") == true) - { - Flag(F.TargetPkg(), Immediate); - tryFixDeps = false; + Flag(RPkg, Immediate); + if (VisitNode(RPkg, "Remove-ImmRep") == true) break; - } - else if (F.TargetPkg()->ProvidesList != 0) - { - pkgCache::PrvIterator Prv = F.TargetPkg().ProvidesList(); - for (; Prv.end() == false; ++Prv) - { - if (IsFlag(Prv.OwnerPkg(), InList) == true && - IsFlag(Prv.OwnerPkg(), AddPending) == false && - Cache[Prv.OwnerPkg()].InstallVer != 0 && - VisitNode(Prv.OwnerPkg(), "Remove-Owner") == true) - { - Flag(Prv.OwnerPkg(), Immediate); - tryFixDeps = false; - break; - } - } - if (Prv.end() == false) - break; - } } - if (tryFixDeps == false) - break; + visitReplacement = false; } + delete[] Replacements; } - - // Skip over missing files - if (IsMissing(D.ParentPkg()) == true) + if (visitReplacement == true) continue; - - if (VisitNode(D.ParentPkg(), "Remove-Parent") == false) + + // the broken package in current version can't be fixed, so install new version + if (IsMissing(BrokenPkg) == true) + break; + + if (VisitNode(BrokenPkg, "Remove-Upgrade") == false) return false; } - + } + return true; } /*}}}*/ diff --git a/debian/changelog b/debian/changelog index 662289fba..d7c3494ce 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,6 +3,8 @@ apt (0.9.8.3) UNRELEASED; urgency=low [ David Kalnischkies ] * build the en manpages in subdirectory doc/en * remove -ldl from cdrom and -lutil from apt-get linkage + * rewrite pkgOrderList::DepRemove to stop incorrect immediate setting + (Closes: 645713) -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 -- cgit v1.2.3-70-g09d2 From 978844db25deb7cd88b053bc2f4685caf2c61a75 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 21 May 2013 17:10:21 +0200 Subject: prefer Essentials over Removals in ordering score Doing Removes early is good to have them out of the way, so they don't break 'Inst' or 'Conf' chains, but scoring them above Essentials means that we end up upgrading (many) less important packages before we handle big stuff like libc6 or debconf which not only fails if those less important packages have unannounced (strict) dependencies, but also leads to having these packages unconfigured for a long time triggering bugs in maintainer scripts for no good reason (#708831). So this commits sets the default value for remove scores to 100, which is below the one for essentials (200) and a lot lower than the previous default value (500). --- apt-pkg/orderlist.cc | 5 ++--- debian/changelog | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/orderlist.cc b/apt-pkg/orderlist.cc index 86d2a9478..984ae1d10 100644 --- a/apt-pkg/orderlist.cc +++ b/apt-pkg/orderlist.cc @@ -301,9 +301,8 @@ bool pkgOrderList::OrderConfigure() /* Higher scores order earlier */ int pkgOrderList::Score(PkgIterator Pkg) { - static int const ScoreDelete = _config->FindI("OrderList::Score::Delete", 500); - - // Removal is always done first + // Removals should be done after we dealt with essentials + static int const ScoreDelete = _config->FindI("OrderList::Score::Delete", 100); if (Cache[Pkg].Delete() == true) return ScoreDelete; diff --git a/debian/changelog b/debian/changelog index d7c3494ce..f57a8334f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -5,6 +5,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * remove -ldl from cdrom and -lutil from apt-get linkage * rewrite pkgOrderList::DepRemove to stop incorrect immediate setting (Closes: 645713) + * prefer Essentials over Removals in ordering score -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 -- cgit v1.2.3-70-g09d2 From 69335858269845904635c592268cf9519e75c1a9 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 21 May 2013 18:06:17 +0200 Subject: fix priority sorting by prefering higher in MarkInstall Used to work until a certain (here unnamed) person came along and used the wrong operator causing low-priority packages to be sorted above high-priority packages while choosing a provider in commit 2b5c35c7bb915dbd46fefd7c79f05364ba22f93b from Nov 2011 --- apt-pkg/depcache.cc | 2 +- debian/changelog | 1 + .../test-prefer-higher-priority-providers | 36 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100755 test/integration/test-prefer-higher-priority-providers (limited to 'apt-pkg') diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc index 6a3e9bfc4..5ca0c2ea5 100644 --- a/apt-pkg/depcache.cc +++ b/apt-pkg/depcache.cc @@ -1004,7 +1004,7 @@ struct CompareProviders { } // higher priority seems like a good idea if (AV->Priority != BV->Priority) - return AV->Priority < BV->Priority; + return AV->Priority > BV->Priority; // prefer native architecture if (strcmp(A.Arch(), B.Arch()) != 0) { diff --git a/debian/changelog b/debian/changelog index f57a8334f..af606453d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -6,6 +6,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * rewrite pkgOrderList::DepRemove to stop incorrect immediate setting (Closes: 645713) * prefer Essentials over Removals in ordering score + * fix priority sorting by prefering higher in MarkInstall -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 diff --git a/test/integration/test-prefer-higher-priority-providers b/test/integration/test-prefer-higher-priority-providers new file mode 100755 index 000000000..66458bee0 --- /dev/null +++ b/test/integration/test-prefer-higher-priority-providers @@ -0,0 +1,36 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'native' + +insertpackage 'unstable' 'foo' 'all' '1' 'Provides: stuff' 'important' +insertpackage 'unstable' 'bar' 'all' '1' 'Provides: stuff' 'optional' +insertpackage 'unstable' 'baz' 'all' '1' 'Provides: stuff' 'extra' +insertpackage 'unstable' 'awesome' 'all' '1' 'Depends: stuff' + +setupaptarchive + +testequal 'Reading package lists... +Building dependency tree... +The following extra packages will be installed: + foo +The following NEW packages will be installed: + awesome foo +0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. +Inst foo (1 unstable [all]) +Inst awesome (1 unstable [all]) +Conf foo (1 unstable [all]) +Conf awesome (1 unstable [all])' aptget install awesome -s + +testequal 'Reading package lists... +Building dependency tree... +The following NEW packages will be installed: + awesome foo +0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. +Inst foo (1 unstable [all]) +Inst awesome (1 unstable [all]) +Conf foo (1 unstable [all]) +Conf awesome (1 unstable [all])' aptget install awesome foo -s -- cgit v1.2.3-70-g09d2 From 66706285737a895d0baf64c2387c58d5211be4f9 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 21 May 2013 21:50:30 +0200 Subject: try all providers in order if uninstallable in MarkInstall --- apt-pkg/depcache.cc | 18 ++++-- debian/changelog | 1 + .../test-prefer-higher-priority-providers | 70 ++++++++++++++++++++++ 3 files changed, 84 insertions(+), 5 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc index 5ca0c2ea5..5bed10d95 100644 --- a/apt-pkg/depcache.cc +++ b/apt-pkg/depcache.cc @@ -1200,16 +1200,23 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg,bool AutoInst, verlist.insert(Cand); } CompareProviders comp(Start); - APT::VersionList::iterator InstVer = std::max_element(verlist.begin(), verlist.end(), comp); - if (InstVer != verlist.end()) - { + do { + APT::VersionList::iterator InstVer = std::max_element(verlist.begin(), verlist.end(), comp); + + if (InstVer == verlist.end()) + break; + pkgCache::PkgIterator InstPkg = InstVer.ParentPkg(); if(DebugAutoInstall == true) std::clog << OutputInDepth(Depth) << "Installing " << InstPkg.Name() << " as " << Start.DepType() << " of " << Pkg.Name() << std::endl; - MarkInstall(InstPkg, true, Depth + 1, false, ForceImportantDeps); + if (MarkInstall(InstPkg, true, Depth + 1, false, ForceImportantDeps) == false) + { + verlist.erase(InstVer); + continue; + } // now check if we should consider it a automatic dependency or not if(InstPkg->CurrentVer == 0 && Pkg->Section != 0 && ConfigValueInSubTree("APT::Never-MarkAuto-Sections", Pkg.Section())) { @@ -1218,7 +1225,8 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg,bool AutoInst, << Start.DepType() << " of pkg in APT::Never-MarkAuto-Sections)" << std::endl; MarkAuto(InstPkg, false); } - } + break; + } while(true); continue; } /* Negative dependencies have no or-group diff --git a/debian/changelog b/debian/changelog index af606453d..b64a57bef 100644 --- a/debian/changelog +++ b/debian/changelog @@ -7,6 +7,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low (Closes: 645713) * prefer Essentials over Removals in ordering score * fix priority sorting by prefering higher in MarkInstall + * try all providers in order if uninstallable in MarkInstall -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 diff --git a/test/integration/test-prefer-higher-priority-providers b/test/integration/test-prefer-higher-priority-providers index 66458bee0..64b901dd0 100755 --- a/test/integration/test-prefer-higher-priority-providers +++ b/test/integration/test-prefer-higher-priority-providers @@ -34,3 +34,73 @@ Inst foo (1 unstable [all]) Inst awesome (1 unstable [all]) Conf foo (1 unstable [all]) Conf awesome (1 unstable [all])' aptget install awesome foo -s + +testequal "Reading package lists... +Building dependency tree... +Package 'bar' is not installed, so not removed +Package 'baz' is not installed, so not removed +The following extra packages will be installed: + foo +The following NEW packages will be installed: + awesome foo +0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. +Inst foo (1 unstable [all]) +Inst awesome (1 unstable [all]) +Conf foo (1 unstable [all]) +Conf awesome (1 unstable [all])" aptget install awesome bar- baz- -s + +testequal "Reading package lists... +Building dependency tree... +Package 'foo' is not installed, so not removed +The following extra packages will be installed: + bar +The following NEW packages will be installed: + awesome bar +0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. +Inst bar (1 unstable [all]) +Inst awesome (1 unstable [all]) +Conf bar (1 unstable [all]) +Conf awesome (1 unstable [all])" aptget install awesome foo- -s + +testequal "Reading package lists... +Building dependency tree... +Package 'foo' is not installed, so not removed +Package 'baz' is not installed, so not removed +The following extra packages will be installed: + bar +The following NEW packages will be installed: + awesome bar +0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. +Inst bar (1 unstable [all]) +Inst awesome (1 unstable [all]) +Conf bar (1 unstable [all]) +Conf awesome (1 unstable [all])" aptget install awesome foo- baz- -s + +testequal "Reading package lists... +Building dependency tree... +Package 'foo' is not installed, so not removed +Package 'bar' is not installed, so not removed +The following extra packages will be installed: + baz +The following NEW packages will be installed: + awesome baz +0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. +Inst baz (1 unstable [all]) +Inst awesome (1 unstable [all]) +Conf baz (1 unstable [all]) +Conf awesome (1 unstable [all])" aptget install awesome foo- bar- -s + +testequal "Reading package lists... +Building dependency tree... +Package 'foo' is not installed, so not removed +Package 'bar' is not installed, so not removed +Package 'baz' is not installed, so not removed +Some packages could not be installed. This may mean that you have +requested an impossible situation or if you are using the unstable +distribution that some required packages have not yet been created +or been moved out of Incoming. +The following information may help to resolve the situation: + +The following packages have unmet dependencies: + awesome : Depends: stuff +E: Unable to correct problems, you have held broken packages." aptget install awesome foo- bar- baz- -s -- cgit v1.2.3-70-g09d2 From 42d51f333e8ef522fed02cdfc48663488d56c3a3 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 23 May 2013 12:14:56 +0200 Subject: do unpacks before configures in SmartConfigure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Splits the big loop over dependencies in SmartConfigure which unpacks and configures dependencies into two loops and reverse their order, so that all dependencies which need to be unpacked are handled first and only after that configures are issued for dependencies. This is needed as otherwise the unpack of a (new) dependency will be issued in between a configure call for two (or more) packages which form a loop, which means the configure calls aren't part of the same dpkg call and therefore dpkg bails out. Such tight loops should really be avoided as they are usually wrong – and in reality the dependencies in libreoffice were greatly simplified thanks to Rene Engelhard so the problem is gone for the benefit of all. Closes: 707578 --- apt-pkg/packagemanager.cc | 113 +++++++++++++-------- debian/changelog | 1 + ...ight-loop-configure-with-unpacking-new-packages | 46 +++++++++ 3 files changed, 115 insertions(+), 45 deletions(-) create mode 100755 test/integration/test-very-tight-loop-configure-with-unpacking-new-packages (limited to 'apt-pkg') diff --git a/apt-pkg/packagemanager.cc b/apt-pkg/packagemanager.cc index e2d7dbf2a..b8932753d 100644 --- a/apt-pkg/packagemanager.cc +++ b/apt-pkg/packagemanager.cc @@ -340,6 +340,7 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth) bool Bad = false, Changed = false; const unsigned int max_loops = _config->FindI("APT::pkgPackageManager::MaxLoopCount", 500); unsigned int i=0; + std::list needConfigure; do { Changed = false; @@ -353,7 +354,7 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth) continue; Bad = true; - // Search for dependencies which are unpacked but aren't configured yet (maybe loops) + // Check for dependencies that have not been unpacked, probably due to loops. for (DepIterator Cur = Start; true; ++Cur) { SPtrArray VList = Cur.AllTargets(); @@ -373,51 +374,63 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth) } // Check if the version that is going to be installed will satisfy the dependency - if (Cache[DepPkg].InstallVer != *I) + if (Cache[DepPkg].InstallVer != *I || List->IsNow(DepPkg) == false) continue; - if (List->IsFlag(DepPkg,pkgOrderList::UnPacked)) + if (PkgLoop == true) { - if (List->IsFlag(DepPkg,pkgOrderList::Loop) && PkgLoop) - { - // This dependency has already been dealt with by another SmartConfigure on Pkg - Bad = false; - break; - } - /* Check for a loop to prevent one forming - If A depends on B and B depends on A, SmartConfigure will - just hop between them if this is not checked. Dont remove the - loop flag after finishing however as loop is already set. - This means that there is another SmartConfigure call for this - package and it will remove the loop flag */ + if (Debug) + std::clog << OutputInDepth(Depth) << "Package " << Pkg << " loops in SmartConfigure" << std::endl; + Bad = false; + break; + } + else + { + if (Debug) + clog << OutputInDepth(Depth) << "Unpacking " << DepPkg.FullName() << " to avoid loop " << Cur << endl; if (PkgLoop == false) List->Flag(Pkg,pkgOrderList::Loop); - if (SmartConfigure(DepPkg, Depth + 1) == true) + if (SmartUnPack(DepPkg, true, Depth + 1) == true) { Bad = false; if (List->IsFlag(DepPkg,pkgOrderList::Loop) == false) - Changed = true; + Changed = true; } if (PkgLoop == false) - List->RmFlag(Pkg,pkgOrderList::Loop); - // If SmartConfigure was succesfull, Bad is false, so break + List->RmFlag(Pkg,pkgOrderList::Loop); if (Bad == false) break; } - else if (List->IsFlag(DepPkg,pkgOrderList::Configured)) - { - Bad = false; - break; - } } - if (Cur == End) + + if (Cur == End || Bad == false) break; - } + } if (Bad == false) continue; - // Check for dependencies that have not been unpacked, probably due to loops. + needConfigure.push_back(Start); + } + if (i++ > max_loops) + return _error->Error("Internal error: MaxLoopCount reached in SmartUnPack (1) for %s, aborting", Pkg.FullName().c_str()); + } while (Changed == true); + + Bad = false, Changed = false, i = 0; + do + { + Changed = false; + for (std::list::iterator D = needConfigure.begin(); D != needConfigure.end(); ++D) + { + // Compute a single dependency element (glob or) + pkgCache::DepIterator Start, End; + D->GlobOr(Start,End); + + if (End->Type != pkgCache::Dep::Depends) + continue; + Bad = true; + + // Search for dependencies which are unpacked but aren't configured yet (maybe loops) for (DepIterator Cur = Start; true; ++Cur) { SPtrArray VList = Cur.AllTargets(); @@ -428,44 +441,54 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth) PkgIterator DepPkg = Ver.ParentPkg(); // Check if the version that is going to be installed will satisfy the dependency - if (Cache[DepPkg].InstallVer != *I || List->IsNow(DepPkg) == false) + if (Cache[DepPkg].InstallVer != *I) continue; - if (PkgLoop == true) - { - if (Debug) - std::clog << OutputInDepth(Depth) << "Package " << Pkg << " loops in SmartConfigure" << std::endl; - Bad = false; - break; - } - else + if (List->IsFlag(DepPkg,pkgOrderList::UnPacked)) { - if (Debug) - clog << OutputInDepth(Depth) << "Unpacking " << DepPkg.FullName() << " to avoid loop " << Cur << endl; + if (List->IsFlag(DepPkg,pkgOrderList::Loop) && PkgLoop) + { + // This dependency has already been dealt with by another SmartConfigure on Pkg + Bad = false; + break; + } + /* Check for a loop to prevent one forming + If A depends on B and B depends on A, SmartConfigure will + just hop between them if this is not checked. Dont remove the + loop flag after finishing however as loop is already set. + This means that there is another SmartConfigure call for this + package and it will remove the loop flag */ if (PkgLoop == false) List->Flag(Pkg,pkgOrderList::Loop); - if (SmartUnPack(DepPkg, true, Depth + 1) == true) + if (SmartConfigure(DepPkg, Depth + 1) == true) { Bad = false; if (List->IsFlag(DepPkg,pkgOrderList::Loop) == false) - Changed = true; + Changed = true; } if (PkgLoop == false) - List->RmFlag(Pkg,pkgOrderList::Loop); + List->RmFlag(Pkg,pkgOrderList::Loop); + // If SmartConfigure was succesfull, Bad is false, so break if (Bad == false) break; } + else if (List->IsFlag(DepPkg,pkgOrderList::Configured)) + { + Bad = false; + break; + } } - - if (Cur == End) + if (Cur == End || Bad == false) break; - } + } + + if (Bad == true && Changed == false && Debug == true) std::clog << OutputInDepth(Depth) << "Could not satisfy " << Start << std::endl; } if (i++ > max_loops) - return _error->Error("Internal error: MaxLoopCount reached in SmartUnPack for %s, aborting", Pkg.FullName().c_str()); + return _error->Error("Internal error: MaxLoopCount reached in SmartUnPack (2) for %s, aborting", Pkg.FullName().c_str()); } while (Changed == true); if (Bad) { diff --git a/debian/changelog b/debian/changelog index b64a57bef..8a1194b1b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -8,6 +8,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * prefer Essentials over Removals in ordering score * fix priority sorting by prefering higher in MarkInstall * try all providers in order if uninstallable in MarkInstall + * do unpacks before configures in SmartConfigure (Closes: #707578) -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 diff --git a/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages b/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages new file mode 100755 index 000000000..7f3b05e59 --- /dev/null +++ b/test/integration/test-very-tight-loop-configure-with-unpacking-new-packages @@ -0,0 +1,46 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' + +# the difference between version 3 and 4 is the new package 'ure' which +# we have to unpack before we start configuring parts of the loop +insertinstalledpackage 'libreoffice' 'amd64' '3' 'Depends: libreoffice-core (= 3)' +insertinstalledpackage 'libreoffice-core' 'amd64' '3' 'Depends: libreoffice-common (>= 3)' +insertinstalledpackage 'libreoffice-common' 'all' '3' 'Depends: libreoffice-style +Breaks: libreoffice-core (>= 3+), libreoffice-core (<= 3~), libreoffice-style-galaxy (>= 3+), libreoffice-style-galaxy (<= 3~)' +insertinstalledpackage 'libreoffice-style-galaxy' 'amd64' '3' 'Depends: libreoffice-core +Provides: libreoffice-style' + +buildsimplenativepackage 'libreoffice' 'amd64' '4' 'sid' 'Depends: libreoffice-core (= 4)' +buildsimplenativepackage 'libreoffice-core' 'amd64' '4' 'sid' 'Depends: libreoffice-common (>= 4) +Breaks: libreoffice-common (<< 4), libreoffice-style-galaxy (<< 4)' +buildsimplenativepackage 'libreoffice-common' 'all' '4' 'sid' 'Depends: libreoffice-style, ure +Breaks: libreoffice-core (>= 4+), libreoffice-core (<= 4~), libreoffice-style-galaxy (>= 4+), libreoffice-style-galaxy (<= 4~)' +buildsimplenativepackage 'libreoffice-style-galaxy' 'amd64' '4' 'sid' 'Depends: libreoffice-core +Provides: libreoffice-style' + +buildsimplenativepackage 'ure' 'amd64' '4' 'sid' + +setupaptarchive + +testequal 'Reading package lists... +Building dependency tree... +The following NEW packages will be installed: + ure +The following packages will be upgraded: + libreoffice libreoffice-common libreoffice-core libreoffice-style-galaxy +4 upgraded, 1 newly installed, 0 to remove and 0 not upgraded. +Inst libreoffice [3] (4 sid [amd64]) [] +Inst libreoffice-style-galaxy [3] (4 sid [amd64]) [libreoffice-common:amd64 on libreoffice-style-galaxy:amd64] [libreoffice-common:amd64 ] +Inst libreoffice-core [3] (4 sid [amd64]) [libreoffice-core:amd64 on libreoffice-common:amd64] [libreoffice-common:amd64 on libreoffice-core:amd64] [libreoffice-common:amd64 on libreoffice-style-galaxy:amd64] [libreoffice-common:amd64 ] +Inst libreoffice-common [3] (4 sid [all]) [] +Inst ure (4 sid [amd64]) +Conf ure (4 sid [amd64]) +Conf libreoffice-style-galaxy (4 sid [amd64]) +Conf libreoffice-common (4 sid [all]) +Conf libreoffice-core (4 sid [amd64]) +Conf libreoffice (4 sid [amd64])' aptget dist-upgrade -s -- cgit v1.2.3-70-g09d2 From f3c736f9b6fdef1d8045846c465d675858eb1471 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 24 May 2013 10:20:38 +0200 Subject: deprecate InstallProtect as a cpu-eating no-op In the past packages were flagged "Protected" so that install/ remove markings where issued before the ProblemResolver. Nowadays, the marking methods check if they are allowed to modify the marking of a package instead, so that markings set by FromUser calls are not overwritten anymore by automatic calls which elimates the need for InstallProtect which just eats CPU now. The method itself is left untouched for now in case frontend needs it still for some wierd usecase, but they should be eliminated. --- apt-pkg/algorithms.cc | 6 ++++-- apt-pkg/algorithms.h | 7 ++++--- cmdline/apt-get.cc | 4 +--- 3 files changed, 9 insertions(+), 8 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/algorithms.cc b/apt-pkg/algorithms.cc index 6cde4d6cc..d19783983 100644 --- a/apt-pkg/algorithms.cc +++ b/apt-pkg/algorithms.cc @@ -1440,9 +1440,11 @@ bool pkgProblemResolver::ResolveByKeepInternal() return true; } /*}}}*/ -// ProblemResolver::InstallProtect - Install all protected packages /*{{{*/ +// ProblemResolver::InstallProtect - deprecated cpu-eating no-op /*{{{*/ // --------------------------------------------------------------------- -/* This is used to make sure protected packages are installed */ +/* Actions issued with FromUser bit set are protected from further + modification (expect by other calls with FromUser set) nowadays , so we + don't need to reissue actions here, they are already set in stone. */ void pkgProblemResolver::InstallProtect() { pkgDepCache::ActionGroup group(Cache); diff --git a/apt-pkg/algorithms.h b/apt-pkg/algorithms.h index aff8a68f2..7f58c8eed 100644 --- a/apt-pkg/algorithms.h +++ b/apt-pkg/algorithms.h @@ -36,6 +36,8 @@ #include +#include + #ifndef APT_8_CLEANER_HEADERS #include using std::ostream; @@ -132,9 +134,8 @@ class pkgProblemResolver /*{{{*/ // Try to resolve problems only by using keep bool ResolveByKeep(); - // Install all protected packages - void InstallProtect(); - + __deprecated void InstallProtect(); + pkgProblemResolver(pkgDepCache *Cache); ~pkgProblemResolver(); }; diff --git a/cmdline/apt-get.cc b/cmdline/apt-get.cc index 999f2a6a7..85ed80a95 100644 --- a/cmdline/apt-get.cc +++ b/cmdline/apt-get.cc @@ -1962,7 +1962,6 @@ bool DoInstall(CommandLine &CmdL) if (Fix != NULL) { // Call the scored problem resolver - Fix->InstallProtect(); Fix->Resolve(true); delete Fix; } @@ -3123,8 +3122,7 @@ bool DoBuildDep(CommandLine &CmdL) } } } - - Fix.InstallProtect(); + if (Fix.Resolve(true) == false) _error->Discard(); -- cgit v1.2.3-70-g09d2 From c7873955a3b9419c5de49ee4665a6006b3bf6563 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 25 May 2013 20:18:15 +0200 Subject: make the vprintf like _error->Insert public Git-Dch: Ignore --- apt-pkg/contrib/error.h | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/error.h b/apt-pkg/contrib/error.h index 21c51c1be..7d09b2d4a 100644 --- a/apt-pkg/contrib/error.h +++ b/apt-pkg/contrib/error.h @@ -123,6 +123,25 @@ public: /*{{{*/ bool InsertErrno(MsgType const &type, const char* Function, const char* Description,...) __like_printf(4) __cold; + /** \brief adds an errno message with the given type + * + * args needs to be initialized with va_start and terminated + * with va_end by the caller. msgSize is also an out-parameter + * in case the msgSize was not enough to store the complete message. + * + * \param type of the error message + * \param Function which failed + * \param Description is the format string for args + * \param args list from a printf-like function + * \param errsv is the errno the error is for + * \param msgSize is the size of the char[] used to store message + * \return true if the message was added, false if not - the caller + * should call this method again in that case + */ + bool InsertErrno(MsgType type, const char* Function, + const char* Description, va_list &args, + int const errsv, size_t &msgSize); + /** \brief add an fatal error message to the list * * Most of the stuff we consider as "error" is also "fatal" for @@ -185,6 +204,22 @@ public: /*{{{*/ */ bool Insert(MsgType const &type, const char* Description,...) __like_printf(3) __cold; + /** \brief adds an error message with the given type + * + * args needs to be initialized with va_start and terminated + * with va_end by the caller. msgSize is also an out-parameter + * in case the msgSize was not enough to store the complete message. + * + * \param type of the error message + * \param Description is the format string for args + * \param args list from a printf-like function + * \param msgSize is the size of the char[] used to store message + * \return true if the message was added, false if not - the caller + * should call this method again in that case + */ + bool Insert(MsgType type, const char* Description, + va_list &args, size_t &msgSize) __cold; + /** \brief is an error in the list? * * \return \b true if an error is included in the list, \b false otherwise @@ -305,12 +340,6 @@ private: /*{{{*/ }; std::list Stacks; - - bool InsertErrno(MsgType type, const char* Function, - const char* Description, va_list &args, - int const errsv, size_t &msgSize); - bool Insert(MsgType type, const char* Description, - va_list &args, size_t &msgSize); /*}}}*/ }; /*}}}*/ -- cgit v1.2.3-70-g09d2 From ae635e3cf7559f3455b88a2499e7521d2094c416 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 25 May 2013 20:22:06 +0200 Subject: set Fail flag in FileFd on all errors consistently Previously some errors would set the Fail flag while some didn't without a clear reason as all errors leave a bad FileFd behind, so we use a helper now to ensure that all errors set the flag. --- apt-pkg/contrib/fileutl.cc | 167 +++++++++++++++++++-------------------------- apt-pkg/contrib/fileutl.h | 4 ++ debian/changelog | 1 + 3 files changed, 76 insertions(+), 96 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 46661887a..4a7299e99 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -861,7 +861,7 @@ bool FileFd::Open(string FileName,unsigned int const Mode,CompressMode Compress, return Open(FileName, ReadOnly, Gzip, Perms); if (Compress == Auto && (Mode & WriteOnly) == WriteOnly) - return _error->Error("Autodetection on %s only works in ReadOnly openmode!", FileName.c_str()); + return FileFdError("Autodetection on %s only works in ReadOnly openmode!", FileName.c_str()); std::vector const compressors = APT::Configuration::getCompressors(); std::vector::const_iterator compressor = compressors.begin(); @@ -914,17 +914,17 @@ bool FileFd::Open(string FileName,unsigned int const Mode,CompressMode Compress, case Auto: case Extension: // Unreachable - return _error->Error("Opening File %s in None, Auto or Extension should be already handled?!?", FileName.c_str()); + return FileFdError("Opening File %s in None, Auto or Extension should be already handled?!?", FileName.c_str()); } for (; compressor != compressors.end(); ++compressor) if (compressor->Name == name) break; if (compressor == compressors.end()) - return _error->Error("Can't find a configured compressor %s for file %s", name.c_str(), FileName.c_str()); + return FileFdError("Can't find a configured compressor %s for file %s", name.c_str(), FileName.c_str()); } if (compressor == compressors.end()) - return _error->Error("Can't find a match for specified compressor mode for file %s", FileName.c_str()); + return FileFdError("Can't find a match for specified compressor mode for file %s", FileName.c_str()); return Open(FileName, Mode, *compressor, Perms); } bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Compressor const &compressor, unsigned long const Perms) @@ -933,9 +933,9 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co Flags = AutoClose; if ((Mode & WriteOnly) != WriteOnly && (Mode & (Atomic | Create | Empty | Exclusive)) != 0) - return _error->Error("ReadOnly mode for %s doesn't accept additional flags!", FileName.c_str()); + return FileFdError("ReadOnly mode for %s doesn't accept additional flags!", FileName.c_str()); if ((Mode & ReadWrite) == 0) - return _error->Error("No openmode provided in FileFd::Open for %s", FileName.c_str()); + return FileFdError("No openmode provided in FileFd::Open for %s", FileName.c_str()); if ((Mode & Atomic) == Atomic) { @@ -981,7 +981,7 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co close (iFd); iFd = -1; } - return _error->Errno("open",_("Could not open file %s"), FileName.c_str()); + return FileFdErrno("open",_("Could not open file %s"), FileName.c_str()); } SetCloseExec(iFd,true); @@ -1010,13 +1010,13 @@ bool FileFd::OpenDescriptor(int Fd, unsigned int const Mode, CompressMode Compre case Xz: name = "xz"; break; case Auto: case Extension: - return _error->Error("Opening Fd %d in Auto or Extension compression mode is not supported", Fd); + return FileFdError("Opening Fd %d in Auto or Extension compression mode is not supported", Fd); } for (; compressor != compressors.end(); ++compressor) if (compressor->Name == name) break; if (compressor == compressors.end()) - return _error->Error("Can't find a configured compressor %s for file %s", name.c_str(), FileName.c_str()); + return FileFdError("Can't find a configured compressor %s for file %s", name.c_str(), FileName.c_str()); return OpenDescriptor(Fd, Mode, *compressor, AutoClose); } @@ -1043,7 +1043,7 @@ bool FileFd::OpenDescriptor(int Fd, unsigned int const Mode, APT::Configuration: { if (AutoClose) close (iFd); - return _error->Errno("gzdopen",_("Could not open file descriptor %d"), Fd); + return FileFdErrno("gzdopen",_("Could not open file descriptor %d"), Fd); } return true; } @@ -1105,10 +1105,7 @@ bool FileFd::OpenInternDescriptor(unsigned int const Mode, APT::Configuration::C ExecWait(d->compressor_pid, "FileFdCompressor", true); if ((Mode & ReadWrite) == ReadWrite) - { - Flags |= Fail; - return _error->Error("ReadWrite mode is not supported for file %s", FileName.c_str()); - } + return FileFdError("ReadWrite mode is not supported for file %s", FileName.c_str()); bool const Comp = (Mode & WriteOnly) == WriteOnly; if (Comp == false) @@ -1131,10 +1128,7 @@ bool FileFd::OpenInternDescriptor(unsigned int const Mode, APT::Configuration::C // Create a data pipe int Pipe[2] = {-1,-1}; if (pipe(Pipe) != 0) - { - Flags |= Fail; - return _error->Errno("pipe",_("Failed to create subprocess IPC")); - } + return FileFdErrno("pipe",_("Failed to create subprocess IPC")); for (int J = 0; J != 2; J++) SetCloseExec(Pipe[J],true); @@ -1244,14 +1238,13 @@ bool FileFd::Read(void *To,unsigned long long Size,unsigned long long *Actual) { if (errno == EINTR) continue; - Flags |= Fail; #ifdef HAVE_ZLIB if (d != NULL && d->gz != NULL) { int err; char const * const errmsg = gzerror(d->gz, &err); if (err != Z_ERRNO) - return _error->Error("gzread: %s (%d: %s)", _("Read error"), err, errmsg); + return FileFdError("gzread: %s (%d: %s)", _("Read error"), err, errmsg); } #endif #ifdef HAVE_BZ2 @@ -1260,10 +1253,10 @@ bool FileFd::Read(void *To,unsigned long long Size,unsigned long long *Actual) int err; char const * const errmsg = BZ2_bzerror(d->bz2, &err); if (err != BZ_IO_ERROR) - return _error->Error("BZ2_bzread: %s (%d: %s)", _("Read error"), err, errmsg); + return FileFdError("BZ2_bzread: %s (%d: %s)", _("Read error"), err, errmsg); } #endif - return _error->Errno("read",_("Read error")); + return FileFdErrno("read",_("Read error")); } To = (char *)To + Res; @@ -1284,9 +1277,8 @@ bool FileFd::Read(void *To,unsigned long long Size,unsigned long long *Actual) Flags |= HitEof; return true; } - - Flags |= Fail; - return _error->Error(_("read, still have %llu to read but none left"), Size); + + return FileFdError(_("read, still have %llu to read but none left"), Size); } /*}}}*/ // FileFd::ReadLine - Read a complete line from the file /*{{{*/ @@ -1342,14 +1334,13 @@ bool FileFd::Write(const void *From,unsigned long long Size) continue; if (Res < 0) { - Flags |= Fail; #ifdef HAVE_ZLIB if (d != NULL && d->gz != NULL) { int err; char const * const errmsg = gzerror(d->gz, &err); if (err != Z_ERRNO) - return _error->Error("gzwrite: %s (%d: %s)", _("Write error"), err, errmsg); + return FileFdError("gzwrite: %s (%d: %s)", _("Write error"), err, errmsg); } #endif #ifdef HAVE_BZ2 @@ -1358,10 +1349,10 @@ bool FileFd::Write(const void *From,unsigned long long Size) int err; char const * const errmsg = BZ2_bzerror(d->bz2, &err); if (err != BZ_IO_ERROR) - return _error->Error("BZ2_bzwrite: %s (%d: %s)", _("Write error"), err, errmsg); + return FileFdError("BZ2_bzwrite: %s (%d: %s)", _("Write error"), err, errmsg); } #endif - return _error->Errno("write",_("Write error")); + return FileFdErrno("write",_("Write error")); } From = (char *)From + Res; @@ -1373,9 +1364,8 @@ bool FileFd::Write(const void *From,unsigned long long Size) if (Size == 0) return true; - - Flags |= Fail; - return _error->Error(_("write, still have %llu to write but couldn't"), Size); + + return FileFdError(_("write, still have %llu to write but couldn't"), Size); } bool FileFd::Write(int Fd, const void *From, unsigned long long Size) { @@ -1419,10 +1409,7 @@ bool FileFd::Seek(unsigned long long To) return Skip(To - seekpos); if ((d->openmode & ReadOnly) != ReadOnly) - { - Flags |= Fail; - return _error->Error("Reopen is only implemented for read-only files!"); - } + return FileFdError("Reopen is only implemented for read-only files!"); #ifdef HAVE_BZ2 if (d->bz2 != NULL) { @@ -1443,17 +1430,11 @@ bool FileFd::Seek(unsigned long long To) if (lseek(d->compressed_fd, 0, SEEK_SET) != 0) iFd = d->compressed_fd; if (iFd < 0) - { - Flags |= Fail; - return _error->Error("Reopen is not implemented for pipes opened with FileFd::OpenDescriptor()!"); - } + return FileFdError("Reopen is not implemented for pipes opened with FileFd::OpenDescriptor()!"); } if (OpenInternDescriptor(d->openmode, d->compressor) == false) - { - Flags |= Fail; - return _error->Error("Seek on file %s because it couldn't be reopened", FileName.c_str()); - } + return FileFdError("Seek on file %s because it couldn't be reopened", FileName.c_str()); if (To != 0) return Skip(To); @@ -1469,10 +1450,7 @@ bool FileFd::Seek(unsigned long long To) #endif res = lseek(iFd,To,SEEK_SET); if (res != (signed)To) - { - Flags |= Fail; - return _error->Error("Unable to seek to %llu", To); - } + return FileFdError("Unable to seek to %llu", To); if (d != NULL) d->seekpos = To; @@ -1496,10 +1474,7 @@ bool FileFd::Skip(unsigned long long Over) { unsigned long long toread = std::min((unsigned long long) sizeof(buffer), Over); if (Read(buffer, toread) == false) - { - Flags |= Fail; - return _error->Error("Unable to seek ahead %llu",Over); - } + return FileFdError("Unable to seek ahead %llu",Over); Over -= toread; } return true; @@ -1513,10 +1488,7 @@ bool FileFd::Skip(unsigned long long Over) #endif res = lseek(iFd,Over,SEEK_CUR); if (res < 0) - { - Flags |= Fail; - return _error->Error("Unable to seek ahead %llu",Over); - } + return FileFdError("Unable to seek ahead %llu",Over); if (d != NULL) d->seekpos = res; @@ -1530,17 +1502,11 @@ bool FileFd::Truncate(unsigned long long To) { #if defined HAVE_ZLIB || defined HAVE_BZ2 if (d != NULL && (d->gz != NULL || d->bz2 != NULL)) - { - Flags |= Fail; - return _error->Error("Truncating compressed files is not implemented (%s)", FileName.c_str()); - } + return FileFdError("Truncating compressed files is not implemented (%s)", FileName.c_str()); #endif if (ftruncate(iFd,To) != 0) - { - Flags |= Fail; - return _error->Error("Unable to truncate to %llu",To); - } - + return FileFdError("Unable to truncate to %llu",To); + return true; } /*}}}*/ @@ -1568,10 +1534,7 @@ unsigned long long FileFd::Tell() #endif Res = lseek(iFd,0,SEEK_CUR); if (Res == (off_t)-1) - { - Flags |= Fail; - _error->Errno("lseek","Failed to determine the current file position"); - } + FileFdErrno("lseek","Failed to determine the current file position"); if (d != NULL) d->seekpos = Res; return Res; @@ -1584,10 +1547,7 @@ unsigned long long FileFd::FileSize() { struct stat Buf; if ((d == NULL || d->pipe == false) && fstat(iFd,&Buf) != 0) - { - Flags |= Fail; - return _error->Errno("fstat","Unable to determine the file size"); - } + return FileFdErrno("fstat","Unable to determine the file size"); // for compressor pipes st_size is undefined and at 'best' zero if ((d != NULL && d->pipe == true) || S_ISFIFO(Buf.st_mode)) @@ -1597,10 +1557,7 @@ unsigned long long FileFd::FileSize() if (d != NULL) d->pipe = true; if (stat(FileName.c_str(), &Buf) != 0) - { - Flags |= Fail; - return _error->Errno("stat","Unable to determine the file size"); - } + return FileFdErrno("stat","Unable to determine the file size"); } return Buf.st_size; @@ -1642,16 +1599,10 @@ unsigned long long FileFd::Size() * bits of the file */ // FIXME: Size for gz-files is limited by 32bit… no largefile support if (lseek(iFd, -4, SEEK_END) < 0) - { - Flags |= Fail; - return _error->Errno("lseek","Unable to seek to end of gzipped file"); - } + return FileFdErrno("lseek","Unable to seek to end of gzipped file"); size = 0L; if (read(iFd, &size, 4) != 4) - { - Flags |= Fail; - return _error->Errno("read","Unable to read original size of gzipped file"); - } + return FileFdErrno("read","Unable to read original size of gzipped file"); #ifdef WORDS_BIGENDIAN uint32_t tmp_size = size; @@ -1661,10 +1612,7 @@ unsigned long long FileFd::Size() #endif if (lseek(iFd, oldPos, SEEK_SET) < 0) - { - Flags |= Fail; - return _error->Errno("lseek","Unable to seek in gzipped file"); - } + return FileFdErrno("lseek","Unable to seek in gzipped file"); return size; } @@ -1681,8 +1629,7 @@ time_t FileFd::ModificationTime() struct stat Buf; if ((d == NULL || d->pipe == false) && fstat(iFd,&Buf) != 0) { - Flags |= Fail; - _error->Errno("fstat","Unable to determine the modification time of file %s", FileName.c_str()); + FileFdErrno("fstat","Unable to determine the modification time of file %s", FileName.c_str()); return 0; } @@ -1695,8 +1642,7 @@ time_t FileFd::ModificationTime() d->pipe = true; if (stat(FileName.c_str(), &Buf) != 0) { - Flags |= Fail; - _error->Errno("fstat","Unable to determine the modification time of file %s", FileName.c_str()); + FileFdErrno("fstat","Unable to determine the modification time of file %s", FileName.c_str()); return 0; } } @@ -1752,11 +1698,40 @@ bool FileFd::Close() bool FileFd::Sync() { if (fsync(iFd) != 0) + return FileFdErrno("sync",_("Problem syncing the file")); + return true; +} + /*}}}*/ +// FileFd::FileFdErrno - set Fail and call _error->Errno *{{{*/ +bool FileFd::FileFdErrno(const char *Function, const char *Description,...) +{ + Flags |= Fail; + va_list args; + size_t msgSize = 400; + int const errsv = errno; + while (true) { - Flags |= Fail; - return _error->Errno("sync",_("Problem syncing the file")); + va_start(args,Description); + if (_error->InsertErrno(GlobalError::ERROR, Function, Description, args, errsv, msgSize) == false) + break; + va_end(args); } - return true; + return false; +} + /*}}}*/ +// FileFd::FileFdError - set Fail and call _error->Error *{{{*/ +bool FileFd::FileFdError(const char *Description,...) { + Flags |= Fail; + va_list args; + size_t msgSize = 400; + while (true) + { + va_start(args,Description); + if (_error->Insert(GlobalError::ERROR, Description, args, msgSize) == false) + break; + va_end(args); + } + return false; } /*}}}*/ diff --git a/apt-pkg/contrib/fileutl.h b/apt-pkg/contrib/fileutl.h index 426664d3a..3ec01dd9a 100644 --- a/apt-pkg/contrib/fileutl.h +++ b/apt-pkg/contrib/fileutl.h @@ -149,6 +149,10 @@ class FileFd private: FileFdPrivate* d; bool OpenInternDescriptor(unsigned int const Mode, APT::Configuration::Compressor const &compressor); + + // private helpers to set Fail flag and call _error->Error + bool FileFdErrno(const char* Function, const char* Description,...) __like_printf(3) __cold; + bool FileFdError(const char* Description,...) __like_printf(2) __cold; }; bool RunScripts(const char *Cnf); diff --git a/debian/changelog b/debian/changelog index 9f35441f9..056025509 100644 --- a/debian/changelog +++ b/debian/changelog @@ -10,6 +10,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * try all providers in order if uninstallable in MarkInstall * do unpacks before configures in SmartConfigure (Closes: #707578) * fix support for multiple patterns in apt-cache search (Closes: #691453) + * set Fail flag in FileFd on all errors consistently -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 -- cgit v1.2.3-70-g09d2 From f97bb5237489134cb971ce38b93c5d6220341ea8 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 25 May 2013 20:33:15 +0200 Subject: OpenDescriptor should autoclose fd always on error OpenInternDescriptor failures would cause additional errors to be generated by double-closing an fd. Other errors (although these are generated if the method is used incorrectly, so unlikely) didn't close the fd aswell. Closes: 704608 --- apt-pkg/contrib/fileutl.cc | 23 +++++++++++++++++++---- debian/changelog | 1 + 2 files changed, 20 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 4a7299e99..7c3a302e2 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1010,14 +1010,19 @@ bool FileFd::OpenDescriptor(int Fd, unsigned int const Mode, CompressMode Compre case Xz: name = "xz"; break; case Auto: case Extension: + if (AutoClose == true && Fd != -1) + close(Fd); return FileFdError("Opening Fd %d in Auto or Extension compression mode is not supported", Fd); } for (; compressor != compressors.end(); ++compressor) if (compressor->Name == name) break; if (compressor == compressors.end()) + { + if (AutoClose == true && Fd != -1) + close(Fd); return FileFdError("Can't find a configured compressor %s for file %s", name.c_str(), FileName.c_str()); - + } return OpenDescriptor(Fd, Mode, *compressor, AutoClose); } bool FileFd::OpenDescriptor(int Fd, unsigned int const Mode, APT::Configuration::Compressor const &compressor, bool AutoClose) @@ -1039,11 +1044,21 @@ bool FileFd::OpenDescriptor(int Fd, unsigned int const Mode, APT::Configuration: else iFd = Fd; this->FileName = ""; - if (OpenInternDescriptor(Mode, compressor) == false) + if (Fd == -1 || OpenInternDescriptor(Mode, compressor) == false) { - if (AutoClose) + if (iFd != -1 && ( +#ifdef HAVE_ZLIB + compressor.Name == "gzip" || +#endif +#ifdef HAVE_BZ2 + compressor.Name == "bzip2" || +#endif + AutoClose == true)) + { close (iFd); - return FileFdErrno("gzdopen",_("Could not open file descriptor %d"), Fd); + iFd = -1; + } + return FileFdError(_("Could not open file descriptor %d"), Fd); } return true; } diff --git a/debian/changelog b/debian/changelog index a80bac3d7..42a8e540e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -12,6 +12,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * fix support for multiple patterns in apt-cache search (Closes: #691453) * set Fail flag in FileFd on all errors consistently * don't explicitly init ExtractTar InFd with invalid fd + * OpenDescriptor should autoclose fd always on error (Closes: #704608) -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 -- cgit v1.2.3-70-g09d2 From 2128d3fce44da34ecb1f0d784b703807f66b20f9 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 9 Jun 2013 18:33:48 +0200 Subject: fail in CopyFile if the FileFds have error flag set Testing for global PendingErrors in users of CopyFile is incorrect in so far as unrelated errors will prevent us from copying perfectly fine files and checking for the validity of the files is just better in CopyFiles as it already checks if files are at least opened. Add also a higher-level error message to the error stack if it fails. --- apt-pkg/contrib/fileutl.cc | 3 ++- apt-pkg/indexcopy.cc | 6 ++---- debian/changelog | 1 + 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 7c3a302e2..0f88923cf 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -184,7 +184,8 @@ bool RunScripts(const char *Cnf) /* The caller is expected to set things so that failure causes erasure */ bool CopyFile(FileFd &From,FileFd &To) { - if (From.IsOpen() == false || To.IsOpen() == false) + if (From.IsOpen() == false || To.IsOpen() == false || + From.Failed() == true || To.Failed() == true) return false; // Buffered copy between fds diff --git a/apt-pkg/indexcopy.cc b/apt-pkg/indexcopy.cc index 0e36b3ded..1d61b974d 100644 --- a/apt-pkg/indexcopy.cc +++ b/apt-pkg/indexcopy.cc @@ -544,11 +544,9 @@ bool SigVerify::CopyMetaIndex(string CDROM, string CDName, /*{{{*/ FileFd Rel; Target.Open(TargetF,FileFd::WriteAtomic); Rel.Open(prefix + file,FileFd::ReadOnly); - if (_error->PendingError() == true) - return false; if (CopyFile(Rel,Target) == false) - return false; - + return _error->Error("Copying of '%s' for '%s' from '%s' failed", file.c_str(), CDName.c_str(), prefix.c_str()); + return true; } /*}}}*/ diff --git a/debian/changelog b/debian/changelog index 42a8e540e..cde0aba2f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -13,6 +13,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * set Fail flag in FileFd on all errors consistently * don't explicitly init ExtractTar InFd with invalid fd * OpenDescriptor should autoclose fd always on error (Closes: #704608) + * fail in CopyFile if the FileFds have error flag set -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 -- cgit v1.2.3-70-g09d2 From b2ea1a47531266377abe4f12c6f21417ea96eea0 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 9 Jun 2013 18:58:34 +0200 Subject: ensure state-dir exists before coyping cdrom files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We do the same in the acquire system which handles the 'normal' downloads, so do it here as well even though its unlikely anyone will ever notice (beside testcases of course …) --- apt-pkg/cdrom.cc | 8 ++++++++ debian/changelog | 1 + test/integration/framework | 8 +++++--- test/integration/test-apt-cdrom | 2 +- 4 files changed, 15 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/cdrom.cc b/apt-pkg/cdrom.cc index 9a9a854bf..a5668a50a 100644 --- a/apt-pkg/cdrom.cc +++ b/apt-pkg/cdrom.cc @@ -829,6 +829,14 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ log->Update(msg.str()); log->Update(_("Copying package lists..."), STEP_COPY); } + + // check for existence and possibly create state directory for copying + string const listDir = _config->FindDir("Dir::State::lists"); + string const partialListDir = listDir + "partial/"; + if (CreateAPTDirectoryIfNeeded(_config->FindDir("Dir::State"), partialListDir) == false && + CreateAPTDirectoryIfNeeded(listDir, partialListDir) == false) + return _error->Errno("cdrom", _("List directory %spartial is missing."), listDir.c_str()); + // take care of the signatures and copy them if they are ok // (we do this before PackageCopy as it modifies "List" and "SourceList") SigVerify SignVerify; diff --git a/debian/changelog b/debian/changelog index cde0aba2f..7582b4f35 100644 --- a/debian/changelog +++ b/debian/changelog @@ -14,6 +14,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * don't explicitly init ExtractTar InFd with invalid fd * OpenDescriptor should autoclose fd always on error (Closes: #704608) * fail in CopyFile if the FileFds have error flag set + * ensure state-dir exists before coyping cdrom files -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 diff --git a/test/integration/framework b/test/integration/framework index 7c2aed592..5c50498a2 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -692,9 +692,11 @@ setupaptarchive() { setupflataptarchive fi signreleasefiles - msgninfo "\tSync APT's cache with the archive… " - aptget update -qq - msgdone "info" + if [ "$1" != '--no-update' ]; then + msgninfo "\tSync APT's cache with the archive… " + aptget update -qq + msgdone "info" + fi } signreleasefiles() { diff --git a/test/integration/test-apt-cdrom b/test/integration/test-apt-cdrom index 3394aa505..6e3533152 100755 --- a/test/integration/test-apt-cdrom +++ b/test/integration/test-apt-cdrom @@ -7,7 +7,7 @@ setupenvironment configarchitecture 'amd64' 'i386' buildsimplenativepackage 'testing' 'amd64,i386' '0.8.15' 'stable' -setupaptarchive +setupaptarchive --no-update changetocdrom 'Debian APT Testdisk 0.8.15' -- cgit v1.2.3-70-g09d2 From 99359751efb1ad84e877219639030feb47fb28f7 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 16 Jun 2013 15:42:31 +0200 Subject: handle missing "Description" in apt-cache show do not blindly assume that all packages stanzas have a "Description:" field in 'apt-cache show' as well as in the cache creation itself. We instead assume now that if the stanza has a Description, it will not be the first field as we look out for "\nDescription" to take care of MD5sum as well as (maybe ignored) translated Descriptions embedded in the package stanza. Closes: #712435 --- apt-pkg/deb/deblistparser.cc | 6 +- apt-pkg/pkgcachegen.cc | 4 +- cmdline/apt-cache.cc | 47 ++++++++---- debian/changelog | 1 + .../test-bug-712435-missing-descriptions | 89 ++++++++++++++++++++++ 5 files changed, 130 insertions(+), 17 deletions(-) create mode 100755 test/integration/test-bug-712435-missing-descriptions (limited to 'apt-pkg') diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc index db86bd698..28857176b 100644 --- a/apt-pkg/deb/deblistparser.cc +++ b/apt-pkg/deb/deblistparser.cc @@ -219,8 +219,12 @@ MD5SumValue debListParser::Description_md5() string const value = Section.FindS("Description-md5"); if (value.empty() == true) { + std::string const desc = Description() + "\n"; + if (desc == "\n") + return MD5SumValue(); + MD5Summation md5; - md5.Add((Description() + "\n").c_str()); + md5.Add(desc.c_str()); return md5.Result(); } else if (likely(value.size() == 32)) diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index 3f10b6c40..7ce7aba7b 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -296,6 +296,8 @@ bool pkgCacheGenerator::MergeListPackage(ListParser &List, pkgCache::PkgIterator // Find the right version to write the description MD5SumValue CurMd5 = List.Description_md5(); + if (CurMd5.Value().empty() == true || List.Description().empty() == true) + return true; std::string CurLang = List.DescriptionLanguage(); for (Ver = Pkg.VersionList(); Ver.end() == false; ++Ver) @@ -480,7 +482,7 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator /* Record the Description (it is not translated) */ MD5SumValue CurMd5 = List.Description_md5(); - if (CurMd5.Value().empty() == true) + if (CurMd5.Value().empty() == true || List.Description().empty() == true) return true; std::string CurLang = List.DescriptionLanguage(); diff --git a/cmdline/apt-cache.cc b/cmdline/apt-cache.cc index bda09a5a1..fb4467c2c 100644 --- a/cmdline/apt-cache.cc +++ b/cmdline/apt-cache.cc @@ -1161,7 +1161,11 @@ bool DisplayRecord(pkgCacheFile &CacheFile, pkgCache::VerIterator V) } // Get a pointer to start of Description field - const unsigned char *DescP = (unsigned char*)strstr((char*)Buffer, "Description:"); + const unsigned char *DescP = (unsigned char*)strstr((char*)Buffer, "\nDescription"); + if (DescP != NULL) + ++DescP; + else + DescP = Buffer + V.FileList()->Size; // Write all but Description if (fwrite(Buffer,1,DescP - Buffer,stdout) < (size_t)(DescP - Buffer)) @@ -1173,25 +1177,38 @@ bool DisplayRecord(pkgCacheFile &CacheFile, pkgCache::VerIterator V) // Show the right description pkgRecords Recs(*Cache); pkgCache::DescIterator Desc = V.TranslatedDescription(); - pkgRecords::Parser &P = Recs.Lookup(Desc.FileList()); - cout << "Description" << ( (strcmp(Desc.LanguageCode(),"") != 0) ? "-" : "" ) << Desc.LanguageCode() << ": " << P.LongDesc(); - - // Find the first field after the description (if there is any) - for(DescP++;DescP != &Buffer[V.FileList()->Size];DescP++) + if (Desc.end() == false) { - if(*DescP == '\n' && *(DescP+1) != ' ') + pkgRecords::Parser &P = Recs.Lookup(Desc.FileList()); + cout << "Description" << ( (strcmp(Desc.LanguageCode(),"") != 0) ? "-" : "" ) << Desc.LanguageCode() << ": " << P.LongDesc(); + cout << std::endl << "Description-md5: " << Desc.md5() << std::endl; + + // Find the first field after the description (if there is any) + while ((DescP = (unsigned char*)strchr((char*)DescP, '\n')) != NULL) { - // write the rest of the buffer - const unsigned char *end=&Buffer[V.FileList()->Size]; - if (fwrite(DescP,1,end-DescP,stdout) < (size_t)(end-DescP)) - { - delete [] Buffer; - return false; - } + if (DescP[1] == ' ') + DescP += 2; + else if (strncmp((char*)DescP, "\nDescription", strlen("\nDescription")) == 0) + DescP += strlen("\nDescription"); + else + break; + } + if (DescP != NULL) + ++DescP; + } + // if we have no translation, we found a lonely Description-md5, so don't skip it - break; + if (DescP != NULL) + { + // write the rest of the buffer + const unsigned char *end=&Buffer[V.FileList()->Size]; + if (fwrite(DescP,1,end-DescP,stdout) < (size_t)(end-DescP)) + { + delete [] Buffer; + return false; } } + // write a final newline (after the description) cout< Sun, 09 Jun 2013 15:06:24 +0200 diff --git a/test/integration/test-bug-712435-missing-descriptions b/test/integration/test-bug-712435-missing-descriptions new file mode 100755 index 000000000..9b3c2ee50 --- /dev/null +++ b/test/integration/test-bug-712435-missing-descriptions @@ -0,0 +1,89 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' + +PACKAGESTANZA='Version: 0.9.7.8 +Installed-Size: 3270 +Maintainer: APT Development Team +Architecture: amd64 +Filename: pool/main/a/apt/apt_0.9.7.8_amd64.deb +MD5sum: 3a622acda41620df50aa22a9fac6f32e' + +DESCRIPTION='Description: commandline package manager + This APT has Super Cow Powers.' + +TRANSDESCRIPTION='Description-en: commandline package manager + This APT has translated Super Cow Powers.' + +echo "Package: apt-normal +$PACKAGESTANZA +$DESCRIPTION +Description-md5: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + +Package: apt-both-below +$PACKAGESTANZA +$DESCRIPTION +$TRANSDESCRIPTION +Description-md5: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + +Package: apt-both-middle +$PACKAGESTANZA +$DESCRIPTION +Description-md5: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +$TRANSDESCRIPTION + +Package: apt-both-top +$PACKAGESTANZA +Description-md5: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +$DESCRIPTION +$TRANSDESCRIPTION + +Package: apt-trans +$PACKAGESTANZA +$TRANSDESCRIPTION +Description-md5: cccccccccccccccccccccccccccccccc + +Package: apt-md5 +$PACKAGESTANZA +Description-md5: dddddddddddddddddddddddddddddddd + +Package: apt-none +$PACKAGESTANZA" > aptarchive/Packages + +setupaptarchive + +testequal "Package: apt-normal +$PACKAGESTANZA +$DESCRIPTION +Description-md5: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +" aptcache show apt-normal + +# displaying the translated Description would be equally valid, +# but we assume only one description is in a Packages file and +# so we prefer "Description" over "Description-*" currently. +for variant in 'below' 'middle' 'top'; do + testequal "Package: apt-both-$variant +$PACKAGESTANZA +$DESCRIPTION +Description-md5: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +" aptcache show apt-both-$variant +done + +testequal "Package: apt-trans +$PACKAGESTANZA +$TRANSDESCRIPTION +Description-md5: cccccccccccccccccccccccccccccccc +" aptcache show apt-trans + +testequal "Package: apt-md5 +$PACKAGESTANZA +Description-md5: dddddddddddddddddddddddddddddddd +" aptcache show apt-md5 + +testequal "Package: apt-none +$PACKAGESTANZA +" aptcache show apt-none -- cgit v1.2.3-70-g09d2 From 709038652854b71895e06caed2b028b389acefd6 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 15 Jun 2013 10:35:04 +0200 Subject: support \n and \r\n line endings in ReadMessages --- apt-pkg/contrib/strutl.cc | 5 +++-- debian/changelog | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index f4dd3407d..d0e74d8c5 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -758,7 +758,8 @@ bool ReadMessages(int Fd, vector &List) // Look for the end of the message for (char *I = Buffer; I + 1 < End; I++) { - if (I[0] != '\n' || I[1] != '\n') + if (I[1] != '\n' || + (I[0] != '\n' && strncmp(I, "\r\n\r\n", 4) != 0)) continue; // Pull the message out @@ -766,7 +767,7 @@ bool ReadMessages(int Fd, vector &List) PartialMessage += Message; // Fix up the buffer - for (; I < End && *I == '\n'; I++); + for (; I < End && (*I == '\n' || *I == '\r'); ++I); End -= I-Buffer; memmove(Buffer,I,End-Buffer); I = Buffer; diff --git a/debian/changelog b/debian/changelog index 8fef47d14..dfff872a7 100644 --- a/debian/changelog +++ b/debian/changelog @@ -18,6 +18,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * fix file location for configure-index.gz in apt.conf(5) (Closes: #711921) * handle missing "Description" in apt-cache show (Closes: #712435) * try defaults if auto-detection failed in apt-cdrom (Closes: #712433) + * support \n and \r\n line endings in ReadMessages -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 -- cgit v1.2.3-70-g09d2 From 0aec7d5c2f6fd55301bd1e1ed35ab7a23fd2357e Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 15 Jun 2013 23:47:07 +0200 Subject: do not redownload unchanged InRelease files Before we download the 'new' InRelease file the old file will be moved out of the way with the name 'foobar_InRelease.reverify', so if no partial file for the 'new' file exists take the modification time from this reverify file, so that if we get an IMS hit for the InRelease file we can move back the reverify file as new file rather than downloading the 'new' file even though we already have it. We do the same for Release files and this happened to work until the reverify renaming was corrected for InRelease files. --- apt-pkg/acquire-item.cc | 13 ++++++++++++- debian/changelog | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 7b800e42f..c48443eff 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1289,7 +1289,14 @@ void pkgAcqMetaIndex::RetrievalDone(string Message) /*{{{*/ string FinalFile = _config->FindDir("Dir::State::lists"); FinalFile += URItoFileName(RealURI); if (SigFile == DestFile) + { SigFile = FinalFile; + // constructor of pkgAcqMetaClearSig moved it out of the way, + // now move it back in on IMS hit for the 'old' file + string const OldClearSig = DestFile + ".reverify"; + if (RealFileExists(OldClearSig) == true) + Rename(OldClearSig, FinalFile); + } DestFile = FinalFile; } Complete = true; @@ -1595,7 +1602,11 @@ string pkgAcqMetaClearSig::Custom600Headers() struct stat Buf; if (stat(Final.c_str(),&Buf) != 0) - return "\nIndex-File: true\nFail-Ignore: true\n"; + { + Final = DestFile + ".reverify"; + if (stat(Final.c_str(),&Buf) != 0) + return "\nIndex-File: true\nFail-Ignore: true\n"; + } return "\nIndex-File: true\nFail-Ignore: true\nLast-Modified: " + TimeRFC1123(Buf.st_mtime); } diff --git a/debian/changelog b/debian/changelog index dfff872a7..91d4ae536 100644 --- a/debian/changelog +++ b/debian/changelog @@ -19,6 +19,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * handle missing "Description" in apt-cache show (Closes: #712435) * try defaults if auto-detection failed in apt-cdrom (Closes: #712433) * support \n and \r\n line endings in ReadMessages + * do not redownload unchanged InRelease files -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 -- cgit v1.2.3-70-g09d2 From ae99ce2e3cadb07c80b89ab2afc804875b1026c5 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 17 Jun 2013 11:23:13 +0200 Subject: trigger NODATA error for invalid InRelease files With the selfgrown splitting we got the problem of not recovering from networks which just reply with invalid data like those sending us login pages to authenticate with the network (e.g. hotels) back. The good thing about the InRelease file is that we know that it must be clearsigned (a Release file might or might not have a detached sig) so if we get a file but are unable to split it something is seriously wrong, so there is not much point in trying further. The Acquire system already looks out for a NODATA error from gpgv, so this adds a new error message sent to the acquire system in case the splitting we do now ourselves failed including this magic word. Closes: #712486 --- apt-pkg/contrib/gpgv.cc | 2 +- apt-pkg/contrib/gpgv.h | 15 ++++- debian/changelog | 1 + methods/gpgv.cc | 16 +++--- test/integration/framework | 3 + .../test-ubuntu-bug-346386-apt-get-update-paywall | 64 ++++++++++++++++++++++ test/interactive-helper/aptwebserver.cc | 26 +++++++++ 7 files changed, 114 insertions(+), 13 deletions(-) create mode 100755 test/integration/test-ubuntu-bug-346386-apt-get-update-paywall (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc index 31db7d5fe..f47e7ea48 100644 --- a/apt-pkg/contrib/gpgv.cc +++ b/apt-pkg/contrib/gpgv.cc @@ -154,7 +154,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG, if (sigFd != -1) unlink(data); ioprintf(std::cerr, "Splitting up %s into data and signature failed", File.c_str()); - exit(EINTERNAL); + exit(112); } Args.push_back(sig); Args.push_back(data); diff --git a/apt-pkg/contrib/gpgv.h b/apt-pkg/contrib/gpgv.h index 08b10a97a..45f069058 100644 --- a/apt-pkg/contrib/gpgv.h +++ b/apt-pkg/contrib/gpgv.h @@ -23,9 +23,18 @@ /** \brief generates and run the command to verify a file with gpgv * * If File and FileSig specify the same file it is assumed that we - * deal with a clear-signed message. In that case the file will be - * rewritten to be in a good-known format without uneeded whitespaces - * and additional messages (unsigned or signed). + * deal with a clear-signed message. Note that the method will accept + * and validate files which include additional (unsigned) messages + * without complaining. Do NOT open files accepted by this method + * for reading. Use #OpenMaybeClearSignedFile to access the message + * instead to ensure you are only reading signed data. + * + * The method does not return, but has some noteable exit-codes: + * 111 signals an internal error like the inability to execute gpgv, + * 112 indicates a clear-signed file which doesn't include a message, + * which can happen if APT is run while on a network requiring + * authentication before usage (e.g. in hotels) + * All other exit-codes are passed-through from gpgv. * * @param File is the message (unsigned or clear-signed) * @param FileSig is the signature (detached or clear-signed) diff --git a/debian/changelog b/debian/changelog index 91d4ae536..bd25df1e2 100644 --- a/debian/changelog +++ b/debian/changelog @@ -20,6 +20,7 @@ apt (0.9.8.3) UNRELEASED; urgency=low * try defaults if auto-detection failed in apt-cdrom (Closes: #712433) * support \n and \r\n line endings in ReadMessages * do not redownload unchanged InRelease files + * trigger NODATA error for invalid InRelease files (Closes: #712486) -- David Kalnischkies Sun, 09 Jun 2013 15:06:24 +0200 diff --git a/methods/gpgv.cc b/methods/gpgv.cc index 3f814b9f0..fe8bac6c9 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -55,9 +55,6 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, vector &NoPubKeySigners) { bool const Debug = _config->FindB("Debug::Acquire::gpgv", false); - // setup a (empty) stringstream for formating the return value - std::stringstream ret; - ret.str(""); if (Debug == true) std::clog << "inside VerifyGetSigners" << std::endl; @@ -170,18 +167,19 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, return ""; } else if (WEXITSTATUS(status) == 1) - { return _("At least one invalid signature was encountered."); - } else if (WEXITSTATUS(status) == 111) + return _("Could not execute 'gpgv' to verify signature (is gpgv installed?)"); + else if (WEXITSTATUS(status) == 112) { - ioprintf(ret, _("Could not execute 'gpgv' to verify signature (is gpgv installed?)")); - return ret.str(); + // acquire system checks for "NODATA" to generate GPG errors (the others are only warnings) + std::string errmsg; + //TRANSLATORS: %s is a single techy word like 'NODATA' + strprintf(errmsg, _("Clearsigned file isn't valid, got '%s' (does the network require authentication?)"), "NODATA"); + return errmsg; } else - { return _("Unknown error executing gpgv"); - } } bool GPGVMethod::Fetch(FetchItem *Itm) diff --git a/test/integration/framework b/test/integration/framework index 3f11ac23b..3a02cfb76 100644 --- a/test/integration/framework +++ b/test/integration/framework @@ -118,6 +118,9 @@ gdb() { echo "gdb: run »$*«" APT_CONFIG=aptconfig.conf LD_LIBRARY_PATH=${BUILDDIRECTORY} $(which gdb) ${BUILDDIRECTORY}/$1 } +http() { + LD_LIBRARY_PATH=${BUILDDIRECTORY} ${BUILDDIRECTORY}/methods/http +} exitwithstatus() { # error if we about to overflow, but ... diff --git a/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall b/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall new file mode 100755 index 000000000..1576c396c --- /dev/null +++ b/test/integration/test-ubuntu-bug-346386-apt-get-update-paywall @@ -0,0 +1,64 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'native' + +insertpackage 'unstable' 'unrelated' 'all' '1.0' 'stable' +insertsource 'unstable' 'unrelated' 'all' '1.0' 'stable' + +echo 'ni ni ni' > aptarchive/knights + +setupaptarchive +changetowebserver -o 'aptwebserver::overwrite::.*::filename=/knights' + +msgtest 'Acquire test file from the webserver to check' 'overwrite' +echo '601 Configuration +Config-Item: Acquire::http::DependOnSTDIN=0 + +600 Acquire URI +URI: http://localhost:8080/holygrail +Filename: knights-talking +' | http >/dev/null 2>&1 && msgpass || msgfail +testfileequal knights-talking 'ni ni ni' + +ensure_n_canary_strings_in_dir() { + local DIR="$1" + local CANARY_STRING="$2" + local EXPECTED_N="$3" + + msgtest "Testing in $DIR for $EXPECTED_N canary" "$CANARY_STRING" + local N=$(grep "$CANARY_STRING" $DIR/* 2>/dev/null |wc -l ) + test "$N" = "$EXPECTED_N" && msgpass || msgfail "Expected $EXPECTED_N canaries, got $N" +} + +LISTS='rootdir/var/lib/apt/lists' +rm -rf rootdir/var/lib/apt/lists +msgtest 'Got expected NODATA failure in' 'apt-get update' +aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail + +ensure_n_canary_strings_in_dir $LISTS 'ni ni ni' 0 +testequal 'partial' ls $LISTS + +# and again with pre-existing files with "valid data" which should remain +for f in Release Release.gpg main_binary-amd64_Packages main_source_Sources; do + echo 'peng neee-wom' > $LISTS/localhost:8080_dists_stable_${f} +done + +msgtest 'Got expected NODATA failure in' 'apt-get update' +aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail + +ensure_n_canary_strings_in_dir $LISTS 'peng neee-wom' 4 +ensure_n_canary_strings_in_dir $LISTS 'ni ni ni' 0 + +# and now with a pre-existing InRelease file +echo 'peng neee-wom' > $LISTS/localhost:8080_dists_stable_InRelease +rm -f $LISTS/localhost:8080_dists_stable_Release $LISTS/localhost:8080_dists_stable_Release.gpg +msgtest 'excpected failure of' 'apt-get update' +aptget update -qq 2>&1 | grep -q 'E: GPG error.*NODATA' && msgpass || msgfail + +ensure_n_canary_strings_in_dir $LISTS 'peng neee-wom' 3 +ensure_n_canary_strings_in_dir $LISTS 'ni ni ni' 0 diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc index de235fa05..05b875673 100644 --- a/test/interactive-helper/aptwebserver.cc +++ b/test/interactive-helper/aptwebserver.cc @@ -435,6 +435,32 @@ int main(int const argc, const char * argv[]) } } + ::Configuration::Item const *Overwrite = _config->Tree("aptwebserver::overwrite"); + if (Overwrite != NULL) + { + for (::Configuration::Item *I = Overwrite->Child; I != NULL; I = I->Next) + { + regex_t *pattern = new regex_t; + int const res = regcomp(pattern, I->Tag.c_str(), REG_EXTENDED | REG_ICASE | REG_NOSUB); + if (res != 0) + { + char error[300]; + regerror(res, pattern, error, sizeof(error)); + sendError(client, 500, *m, sendContent, error); + continue; + } + if (regexec(pattern, filename.c_str(), 0, 0, 0) == 0) + { + filename = _config->Find("aptwebserver::overwrite::" + I->Tag + "::filename", filename); + if (filename[0] == '/') + filename.erase(0,1); + regfree(pattern); + break; + } + regfree(pattern); + } + } + // deal with the request if (RealFileExists(filename) == true) { -- cgit v1.2.3-70-g09d2 From 6420c00e30cd9512e045a5370bf391321691ca78 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 24 Jun 2013 12:29:48 +0200 Subject: do not modify DepIterator as we might check again fixup for 42d51f333e8ef522fed02cdfc48663488d56c3a3 The for-loop iterating over the DepIterators which need configuration can (and will be in 'complicated' situations) run multiple times, so we can't just GlobOr on the DepIterator as it modifies it, so that the next iteration over the list ends up checking another dependency leading us into a 'Internal error, packages left unconfigured. foopkg' maybe or we are 'lucky' and calculate a solution which might break down the line Git-Dch: Ignore --- apt-pkg/packagemanager.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/packagemanager.cc b/apt-pkg/packagemanager.cc index b8932753d..310934c42 100644 --- a/apt-pkg/packagemanager.cc +++ b/apt-pkg/packagemanager.cc @@ -420,11 +420,14 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth) do { Changed = false; - for (std::list::iterator D = needConfigure.begin(); D != needConfigure.end(); ++D) + for (std::list::const_iterator D = needConfigure.begin(); D != needConfigure.end(); ++D) { - // Compute a single dependency element (glob or) + // Compute a single dependency element (glob or) without modifying D pkgCache::DepIterator Start, End; - D->GlobOr(Start,End); + { + pkgCache::DepIterator Discard = *D; + Discard.GlobOr(Start,End); + } if (End->Type != pkgCache::Dep::Depends) continue; @@ -483,9 +486,8 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth) } - if (Bad == true && Changed == false && Debug == true) - std::clog << OutputInDepth(Depth) << "Could not satisfy " << Start << std::endl; + std::clog << OutputInDepth(Depth) << "Could not satisfy " << *D << std::endl; } if (i++ > max_loops) return _error->Error("Internal error: MaxLoopCount reached in SmartUnPack (2) for %s, aborting", Pkg.FullName().c_str()); -- cgit v1.2.3-70-g09d2 From f58a9890e345faa04b5fcb2a01cae39f986a42db Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 24 Jun 2013 12:51:29 +0200 Subject: fix SHA2* cleanups to zero-out the complete context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GCC 4.8 is now clever enough to warn about: contrib/sha2_internal.cc: In function ‘char* SHA256_End(SHA256_CTX*, char*)’: contrib/sha2_internal.cc:656:31: warning: argument to ‘sizeof’ in ‘void* memset(void*, int, size_t)’ call is the same expression as the destination; did you mean to dereference it? [-Wsizeof-pointer-memaccess] MEMSET_BZERO(context, sizeof(context)); So fix it as suggested. Its interesting though that the SHA2* calculation as far as we need it works even without zeroing out. Git-Dch: Ignore --- apt-pkg/contrib/sha2_internal.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/sha2_internal.cc b/apt-pkg/contrib/sha2_internal.cc index 83b5a98d3..f84fb761c 100644 --- a/apt-pkg/contrib/sha2_internal.cc +++ b/apt-pkg/contrib/sha2_internal.cc @@ -632,7 +632,7 @@ void SHA256_Final(sha2_byte digest[], SHA256_CTX* context) { } /* Clean up state data: */ - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); usedspace = 0; } @@ -653,7 +653,7 @@ char *SHA256_End(SHA256_CTX* context, char buffer[]) { } *buffer = (char)0; } else { - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } MEMSET_BZERO(digest, SHA256_DIGEST_LENGTH); return buffer; @@ -969,7 +969,7 @@ void SHA512_Final(sha2_byte digest[], SHA512_CTX* context) { } /* Zero out state data */ - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } char *SHA512_End(SHA512_CTX* context, char buffer[]) { @@ -989,7 +989,7 @@ char *SHA512_End(SHA512_CTX* context, char buffer[]) { } *buffer = (char)0; } else { - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } MEMSET_BZERO(digest, SHA512_DIGEST_LENGTH); return buffer; @@ -1044,7 +1044,7 @@ void SHA384_Final(sha2_byte digest[], SHA384_CTX* context) { } /* Zero out state data */ - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } char *SHA384_End(SHA384_CTX* context, char buffer[]) { @@ -1064,7 +1064,7 @@ char *SHA384_End(SHA384_CTX* context, char buffer[]) { } *buffer = (char)0; } else { - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } MEMSET_BZERO(digest, SHA384_DIGEST_LENGTH); return buffer; -- cgit v1.2.3-70-g09d2 From 32b5dd08ac22b0ad153b145d1500c66fe4c78f83 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 27 Jun 2013 07:25:53 +0200 Subject: show broken count when starting the resolver --- apt-pkg/algorithms.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/algorithms.cc b/apt-pkg/algorithms.cc index 6cde4d6cc..b08b8dcfc 100644 --- a/apt-pkg/algorithms.cc +++ b/apt-pkg/algorithms.cc @@ -845,8 +845,10 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix) } while (Again == true); - if (Debug == true) + if (Debug == true) { clog << "Starting" << endl; + clog << " Broken count: " << Cache.BrokenCount() << endl; + } MakeScores(); @@ -874,8 +876,10 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix) } } - if (Debug == true) + if (Debug == true) { clog << "Starting 2" << endl; + clog << " Broken count: " << Cache.BrokenCount() << endl; + } /* Now consider all broken packages. For each broken package we either remove the package or fix it's problem. We do this once, it should -- cgit v1.2.3-70-g09d2 From 06f881174113eaa667006663c6f4da3e3f4409b4 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 27 Jun 2013 07:26:39 +0200 Subject: when doing MarkInstall() packages may also get removed, so show them in the debug output of Debug::pkgDepCache::AutoInstall=true --- apt-pkg/depcache.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc index 6a3e9bfc4..f9bfa1f67 100644 --- a/apt-pkg/depcache.cc +++ b/apt-pkg/depcache.cc @@ -1243,9 +1243,16 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg,bool AutoInst, PkgState[Pkg->ID].CandidateVer != *I && MarkInstall(Pkg,true,Depth + 1, false, ForceImportantDeps) == true) continue; - else if ((Start->Type == pkgCache::Dep::Conflicts || Start->Type == pkgCache::Dep::DpkgBreaks) && - MarkDelete(Pkg,false,Depth + 1, false) == false) - break; + else if (Start->Type == pkgCache::Dep::Conflicts || + Start->Type == pkgCache::Dep::DpkgBreaks) + { + if(DebugAutoInstall == true) + std::clog << OutputInDepth(Depth) + << " Removing: " << Pkg.Name() + << std::endl; + if (MarkDelete(Pkg,false,Depth + 1, false) == false) + break; + } } continue; } -- cgit v1.2.3-70-g09d2 From dcab2a78e1585903ff144949f40c514788638f92 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 27 Jun 2013 10:27:29 +0200 Subject: use just one line for the debug output (thanks to donkult for the review) --- apt-pkg/algorithms.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/algorithms.cc b/apt-pkg/algorithms.cc index b08b8dcfc..c2c38e728 100644 --- a/apt-pkg/algorithms.cc +++ b/apt-pkg/algorithms.cc @@ -846,8 +846,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix) while (Again == true); if (Debug == true) { - clog << "Starting" << endl; - clog << " Broken count: " << Cache.BrokenCount() << endl; + clog << "Starting, broken count: " << Cache.BrokenCount() << endl; } MakeScores(); @@ -877,8 +876,7 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix) } if (Debug == true) { - clog << "Starting 2" << endl; - clog << " Broken count: " << Cache.BrokenCount() << endl; + clog << "Starting 2, broken count: " << Cache.BrokenCount() << endl; } /* Now consider all broken packages. For each broken package we either -- cgit v1.2.3-70-g09d2 From 49b49018ff08e337c506d5ccb3476bc2faa5d1b5 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 28 Jun 2013 07:37:55 +0200 Subject: make starting debug output of pkgProblemResolver proper english --- apt-pkg/algorithms.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/algorithms.cc b/apt-pkg/algorithms.cc index c2c38e728..df38e6109 100644 --- a/apt-pkg/algorithms.cc +++ b/apt-pkg/algorithms.cc @@ -846,7 +846,8 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix) while (Again == true); if (Debug == true) { - clog << "Starting, broken count: " << Cache.BrokenCount() << endl; + clog << "Starting pkgProblemResolver with broken count: " + << Cache.BrokenCount() << endl; } MakeScores(); @@ -876,7 +877,8 @@ bool pkgProblemResolver::ResolveInternal(bool const BrokenFix) } if (Debug == true) { - clog << "Starting 2, broken count: " << Cache.BrokenCount() << endl; + clog << "Starting 2 pkgProblemResolver with broken count: " + << Cache.BrokenCount() << endl; } /* Now consider all broken packages. For each broken package we either -- cgit v1.2.3-70-g09d2 From 9c5104cf35c6bca91247a8a31c1748be80ab7d68 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 10 Jul 2013 16:41:14 +0200 Subject: apt-pkg/packagemanager.cc: * apt-pkg/packagemanager.cc: - increate APT::pkgPackageManager::MaxLoopCount to 5000 --- apt-pkg/packagemanager.cc | 4 ++-- debian/changelog | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/packagemanager.cc b/apt-pkg/packagemanager.cc index 310934c42..8c0d2e855 100644 --- a/apt-pkg/packagemanager.cc +++ b/apt-pkg/packagemanager.cc @@ -338,7 +338,7 @@ bool pkgPackageManager::SmartConfigure(PkgIterator Pkg, int const Depth) however if there is a loop (A depends on B, B depends on A) this will not be the case, so check for dependencies before configuring. */ bool Bad = false, Changed = false; - const unsigned int max_loops = _config->FindI("APT::pkgPackageManager::MaxLoopCount", 500); + const unsigned int max_loops = _config->FindI("APT::pkgPackageManager::MaxLoopCount", 5000); unsigned int i=0; std::list needConfigure; do @@ -628,7 +628,7 @@ bool pkgPackageManager::SmartUnPack(PkgIterator Pkg, bool const Immediate, int c This will be either dealt with if the package is configured as a dependency of Pkg (if and when Pkg is configured), or by the ConfigureAll call at the end of the for loop in OrderInstall. */ bool Changed = false; - const unsigned int max_loops = _config->FindI("APT::pkgPackageManager::MaxLoopCount", 500); + const unsigned int max_loops = _config->FindI("APT::pkgPackageManager::MaxLoopCount", 5000); unsigned int i = 0; do { diff --git a/debian/changelog b/debian/changelog index 295142165..cb47dd456 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,7 +1,10 @@ apt (0.9.9.1) UNRELEASED; urgency=low + [ Michael Vogt ] * debian/rules: - call dh_clean in clean (closes: #714980) + * apt-pkg/packagemanager.cc: + - increate APT::pkgPackageManager::MaxLoopCount to 5000 -- Michael Vogt Fri, 05 Jul 2013 16:39:34 +0200 -- cgit v1.2.3-70-g09d2 From 7a948ec719ecc020c2337fe3f41c5fc42699e2c1 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 26 Jun 2013 17:37:57 +0200 Subject: Version 3 for DPkg::Pre-Install-Pkgs with MultiArch info Adds on top of Version 2 to all displayed version numbers the architecture as well as the MultiArch flag for consumption by the hooks. Most of the time the architecture will be the same for both versions displayed, but packages might change from "all" to "any" (or back) between versions so we can't display the architecture for packages. Pseudo-Format for Version 3: Examples: stuff - - none < 1 amd64 none **CONFIGURE** libsame 1 i386 same < 2 i386 same **CONFIGURE** stuff 2 i386 none > 1 i386 none **CONFIGURE** libsame 2 i386 same > - - none **REMOVE** toolkit 1 all foreign > - - none **REMOVE** Closes: #712116 --- apt-pkg/cacheiterators.h | 1 + apt-pkg/deb/dpkgpm.cc | 71 +++++++++++----- apt-pkg/deb/dpkgpm.h | 4 +- apt-pkg/pkgcache.cc | 12 +++ doc/apt.conf.5.xml | 14 +++- ...bug-712116-dpkg-pre-install-pkgs-hook-multiarch | 95 ++++++++++++++++++++++ 6 files changed, 171 insertions(+), 26 deletions(-) create mode 100755 test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch (limited to 'apt-pkg') diff --git a/apt-pkg/cacheiterators.h b/apt-pkg/cacheiterators.h index 179a0e963..886d84838 100644 --- a/apt-pkg/cacheiterators.h +++ b/apt-pkg/cacheiterators.h @@ -220,6 +220,7 @@ class pkgCache::VerIterator : public Iterator { inline VerFileIterator FileList() const; bool Downloadable() const; inline const char *PriorityType() const {return Owner->Priority(S->Priority);}; + const char *MultiArchType() const; std::string RelStr() const; bool Automatic() const; diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index 3bc31dc37..fb0473535 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -238,15 +238,23 @@ bool pkgDPkgPM::Remove(PkgIterator Pkg,bool Purge) return true; } /*}}}*/ -// DPkgPM::SendV2Pkgs - Send version 2 package info /*{{{*/ +// DPkgPM::SendPkgInfo - Send info for install-pkgs hook /*{{{*/ // --------------------------------------------------------------------- /* This is part of the helper script communication interface, it sends very complete information down to the other end of the pipe.*/ bool pkgDPkgPM::SendV2Pkgs(FILE *F) { - fprintf(F,"VERSION 2\n"); - - /* Write out all of the configuration directives by walking the + return SendPkgsInfo(F, 2); +} +bool pkgDPkgPM::SendPkgsInfo(FILE * const F, unsigned int const &Version) +{ + // This version of APT supports only v3, so don't sent higher versions + if (Version <= 3) + fprintf(F,"VERSION %u\n", Version); + else + fprintf(F,"VERSION 3\n"); + + /* Write out all of the configuration directives by walking the configuration tree */ const Configuration::Item *Top = _config->Tree(0); for (; Top != 0;) @@ -280,30 +288,51 @@ bool pkgDPkgPM::SendV2Pkgs(FILE *F) pkgDepCache::StateCache &S = Cache[I->Pkg]; fprintf(F,"%s ",I->Pkg.Name()); - // Current version - if (I->Pkg->CurrentVer == 0) - fprintf(F,"- "); + + // Current version which we are going to replace + pkgCache::VerIterator CurVer = I->Pkg.CurrentVer(); + if (CurVer.end() == true && (I->Op == Item::Remove || I->Op == Item::Purge)) + CurVer = FindNowVersion(I->Pkg); + + else if (CurVer.end() == true) + { + if (Version <= 2) + fprintf(F, "- "); + else + fprintf(F, "- - none "); + } else - fprintf(F,"%s ",I->Pkg.CurrentVer().VerStr()); - - // Show the compare operator - // Target version + { + fprintf(F, "%s ", CurVer.VerStr()); + if (Version >= 3) + fprintf(F, "%s %s ", CurVer.Arch(), CurVer.MultiArchType()); + } + + // Show the compare operator between current and install version if (S.InstallVer != 0) { + pkgCache::VerIterator const InstVer = S.InstVerIter(Cache); int Comp = 2; - if (I->Pkg->CurrentVer != 0) - Comp = S.InstVerIter(Cache).CompareVer(I->Pkg.CurrentVer()); + if (CurVer.end() == false) + Comp = InstVer.CompareVer(CurVer); if (Comp < 0) fprintf(F,"> "); - if (Comp == 0) + else if (Comp == 0) fprintf(F,"= "); - if (Comp > 0) + else if (Comp > 0) fprintf(F,"< "); - fprintf(F,"%s ",S.InstVerIter(Cache).VerStr()); + fprintf(F, "%s ", InstVer.VerStr()); + if (Version >= 3) + fprintf(F, "%s %s ", InstVer.Arch(), InstVer.MultiArchType()); } else - fprintf(F,"> - "); - + { + if (Version <= 2) + fprintf(F, "> - "); + else + fprintf(F, "> - - none "); + } + // Show the filename/operation if (I->Op == Item::Install) { @@ -313,9 +342,9 @@ bool pkgDPkgPM::SendV2Pkgs(FILE *F) else fprintf(F,"%s\n",I->File.c_str()); } - if (I->Op == Item::Configure) + else if (I->Op == Item::Configure) fprintf(F,"**CONFIGURE**\n"); - if (I->Op == Item::Remove || + else if (I->Op == Item::Remove || I->Op == Item::Purge) fprintf(F,"**REMOVE**\n"); @@ -404,7 +433,7 @@ bool pkgDPkgPM::RunScriptsWithPkgs(const char *Cnf) } } else - SendV2Pkgs(F); + SendPkgsInfo(F, Version); fclose(F); diff --git a/apt-pkg/deb/dpkgpm.h b/apt-pkg/deb/dpkgpm.h index aab39f633..c31d56f8e 100644 --- a/apt-pkg/deb/dpkgpm.h +++ b/apt-pkg/deb/dpkgpm.h @@ -14,6 +14,7 @@ #include #include #include +#include #ifndef APT_8_CLEANER_HEADERS using std::vector; @@ -79,7 +80,8 @@ class pkgDPkgPM : public pkgPackageManager // Helpers bool RunScriptsWithPkgs(const char *Cnf); - bool SendV2Pkgs(FILE *F); + __deprecated bool SendV2Pkgs(FILE *F); + bool SendPkgsInfo(FILE * const F, unsigned int const &Version); void WriteHistoryTag(std::string const &tag, std::string value); // apport integration diff --git a/apt-pkg/pkgcache.cc b/apt-pkg/pkgcache.cc index d7725563b..52e814c0b 100644 --- a/apt-pkg/pkgcache.cc +++ b/apt-pkg/pkgcache.cc @@ -924,6 +924,18 @@ string pkgCache::VerIterator::RelStr() const return Res; } /*}}}*/ +// VerIterator::MultiArchType - string representing MultiArch flag /*{{{*/ +const char * pkgCache::VerIterator::MultiArchType() const +{ + if ((S->MultiArch & pkgCache::Version::Same) == pkgCache::Version::Same) + return "same"; + else if ((S->MultiArch & pkgCache::Version::Foreign) == pkgCache::Version::Foreign) + return "foreign"; + else if ((S->MultiArch & pkgCache::Version::Allowed) == pkgCache::Version::Allowed) + return "allowed"; + return "none"; +} + /*}}}*/ // PkgFileIterator::IsOk - Checks if the cache is in sync with the file /*{{{*/ // --------------------------------------------------------------------- /* This stats the file and compares its stats with the ones that were diff --git a/doc/apt.conf.5.xml b/doc/apt.conf.5.xml index 3cf3136d3..9973fe42b 100644 --- a/doc/apt.conf.5.xml +++ b/doc/apt.conf.5.xml @@ -688,11 +688,17 @@ DPkg::Pre-Install-Pkgs {"/usr/sbin/dpkg-preconfigure --apt";}; will abort. APT will pass the filenames of all .deb files it is going to install to the commands, one per line on standard input. - Version 2 of this protocol dumps more information, including the + Version 2 of this protocol dumps more information, including the protocol version, the APT configuration space and the packages, files - and versions being changed. Version 2 is enabled by setting - DPkg::Tools::options::cmd::Version to 2. cmd is a - command given to Pre-Install-Pkgs. + and versions being changed. Version 3 adds the architecture and MultiArch + flag to each version being dumped. + + The version of the protocol to be used for the command + cmd can be chosen by setting + DPkg::Tools::options::cmd::Version + accordingly, the default being version 1. If APT isn't supporting the requested + version it will send the information in the highest version it has support for instead. + diff --git a/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch b/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch new file mode 100755 index 000000000..aee44f76b --- /dev/null +++ b/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch @@ -0,0 +1,95 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' 'i386' + +buildsimplenativepackage 'toolkit' 'all' '1' 'stable' 'Multi-Arch: foreign' +buildsimplenativepackage 'toolkit' 'amd64' '2' 'unstable' 'Multi-Arch: foreign' +buildsimplenativepackage 'libsame' 'i386,amd64' '1' 'stable' 'Multi-Arch: same' +buildsimplenativepackage 'libsame' 'i386,amd64' '2' 'unstable' 'Multi-Arch: same' +buildsimplenativepackage 'stuff' 'i386,amd64' '1' 'stable' 'Depends: libsame (= 1), toolkit (= 1)' +buildsimplenativepackage 'stuff' 'i386,amd64' '2' 'unstable' 'Depends: libsame (= 2), toolkit (= 2)' + +setupaptarchive + +hook='pre-install-pkgs' + +enablehookversion() { + echo "#!/bin/sh +while read line; do + if echo \"\$line\" | grep -Fq '**'; then + echo \"\$line\" + fi +done > ${hook}-v${1}.list" > ${hook}-v${1}.sh + chmod +x ${hook}-v${1}.sh + echo "dpkg::${hook}:: \"./${hook}-v${1}.sh --foo -bar\"; +DPkg::Tools::options::\"./${hook}-v${1}.sh\"::Version \"$1\";" > rootdir/etc/apt/apt.conf.d/hook-v$1 +} + +enablehookversion 2 +enablehookversion 3 + +observehook() { + rm -f ${hook}-v2.list ${hook}-v3.list + msgtest 'Observe hooks while' "$*" + aptget "$@" -y --force-yes >/dev/null 2>&1 && msgpass || msgfail +} + +observehook install stuff -t stable +testfileequal "${hook}-v2.list" 'libsame - < 1 **CONFIGURE** +toolkit - < 1 **CONFIGURE** +stuff - < 1 **CONFIGURE**' +testfileequal "${hook}-v3.list" 'libsame - - none < 1 amd64 same **CONFIGURE** +toolkit - - none < 1 all foreign **CONFIGURE** +stuff - - none < 1 amd64 none **CONFIGURE**' + +observehook install stuff -t unstable +testfileequal "${hook}-v2.list" 'libsame 1 < 2 **CONFIGURE** +toolkit 1 < 2 **CONFIGURE** +stuff 1 < 2 **CONFIGURE**' +testfileequal "${hook}-v3.list" 'libsame 1 amd64 same < 2 amd64 same **CONFIGURE** +toolkit 1 all foreign < 2 amd64 foreign **CONFIGURE** +stuff 1 amd64 none < 2 amd64 none **CONFIGURE**' + +observehook install stuff:i386 -t unstable +testfileequal "${hook}-v2.list" 'stuff 2 > - **REMOVE** +libsame - < 2 **CONFIGURE** +stuff - < 2 **CONFIGURE**' +testfileequal "${hook}-v3.list" 'stuff 2 amd64 none > - - none **REMOVE** +libsame - - none < 2 i386 same **CONFIGURE** +stuff - - none < 2 i386 none **CONFIGURE**' + +observehook remove libsame +testfileequal "${hook}-v2.list" 'libsame 2 > - **REMOVE**' +testfileequal "${hook}-v3.list" 'libsame 2 amd64 same > - - none **REMOVE**' + +observehook install stuff:i386/stable libsame:i386/stable toolkit/stable +testfileequal "${hook}-v2.list" 'libsame 2 > 1 **CONFIGURE** +toolkit 2 > 1 **CONFIGURE** +stuff 2 > 1 **CONFIGURE**' +testfileequal "${hook}-v3.list" 'libsame 2 i386 same > 1 i386 same **CONFIGURE** +toolkit 2 amd64 foreign > 1 all foreign **CONFIGURE** +stuff 2 i386 none > 1 i386 none **CONFIGURE**' + +observehook install 'libsame:*' +testfileequal "${hook}-v2.list" 'libsame 1 < 2 **CONFIGURE** +libsame - < 2 **CONFIGURE** +toolkit 1 < 2 **CONFIGURE** +stuff 1 < 2 **CONFIGURE**' +testfileequal "${hook}-v3.list" 'libsame 1 i386 same < 2 i386 same **CONFIGURE** +libsame - - none < 2 amd64 same **CONFIGURE** +toolkit 1 all foreign < 2 amd64 foreign **CONFIGURE** +stuff 1 i386 none < 2 i386 none **CONFIGURE**' + +observehook purge stuff:i386 'libsame:*' toolkit +testfileequal "${hook}-v2.list" 'libsame 2 > - **REMOVE** +stuff 2 > - **REMOVE** +libsame 2 > - **REMOVE** +toolkit 2 > - **REMOVE**' +testfileequal "${hook}-v3.list" 'libsame 2 amd64 same > - - none **REMOVE** +stuff 2 i386 none > - - none **REMOVE** +libsame 2 i386 same > - - none **REMOVE** +toolkit 2 amd64 foreign > - - none **REMOVE**' -- cgit v1.2.3-70-g09d2 From 3d1be93dc4242df2b93de632715a8aa7dd34f96f Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 28 Jun 2013 20:42:18 +0200 Subject: implement arch+= and arch-= for sources.list Default is to acquire all architectures from APT::Architectures which can be changed by arch=, but this isn't very flexible if you want "mostly" the default as you have to hardcode the architectures then, so arch-= and arch+= can be used to add/remove architectures from the default set. On a machine with 'amd64' and 'i386' configured the lines: deb [arch+=armel] http://example.org/debian wheezy rocks deb [arch-=amd64] http://example.org/debian jessie rocks will result in the download of: wheezy Packages for 'amd64', 'i386' and 'armel' jessie Packages for 'i386' --- apt-pkg/deb/debmetaindex.cc | 23 +++++- doc/sources.list.5.xml | 6 +- .../test-sourceslist-arch-plusminus-options | 85 ++++++++++++++++++++++ 3 files changed, 111 insertions(+), 3 deletions(-) create mode 100755 test/integration/test-sourceslist-arch-plusminus-options (limited to 'apt-pkg') diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc index 7a88d71e3..7dd5ab2bf 100644 --- a/apt-pkg/deb/debmetaindex.cc +++ b/apt-pkg/deb/debmetaindex.cc @@ -373,10 +373,29 @@ class debSLTypeDebian : public pkgSourceList::Type string const &Dist, string const &Section, bool const &IsSrc, map const &Options) const { - map::const_iterator const arch = Options.find("arch"); - vector const Archs = + // parse arch=, arch+= and arch-= settings + map::const_iterator arch = Options.find("arch"); + vector Archs = (arch != Options.end()) ? VectorizeString(arch->second, ',') : APT::Configuration::getArchitectures(); + if ((arch = Options.find("arch+")) != Options.end()) + { + std::vector const plusArch = VectorizeString(arch->second, ','); + for (std::vector::const_iterator plus = plusArch.begin(); plus != plusArch.end(); ++plus) + if (std::find(Archs.begin(), Archs.end(), *plus) == Archs.end()) + Archs.push_back(*plus); + } + if ((arch = Options.find("arch-")) != Options.end()) + { + std::vector const minusArch = VectorizeString(arch->second, ','); + for (std::vector::const_iterator minus = minusArch.begin(); minus != minusArch.end(); ++minus) + { + std::vector::iterator kill = std::find(Archs.begin(), Archs.end(), *minus); + if (kill != Archs.end()) + Archs.erase(kill); + } + } + map::const_iterator const trusted = Options.find("trusted"); for (vector::const_iterator I = List.begin(); diff --git a/doc/sources.list.5.xml b/doc/sources.list.5.xml index 5c539798a..fa32297c2 100644 --- a/doc/sources.list.5.xml +++ b/doc/sources.list.5.xml @@ -114,10 +114,14 @@ setting=value. Multiple settings are separated by spaces. The following settings are supported by APT (note however that unsupported settings will be ignored silently): - arch=arch1,arch2,… + + arch=arch1,arch2,… can be used to specify for which architectures information should be downloaded. If this option is not set all architectures defined by the APT::Architectures option will be downloaded. + arch+=arch1,arch2,… + and arch-=arch1,arch2,… + which can be used to add/remove architectures from the set which will be downloaded. trusted=yes can be set to indicate that packages from this source are always authenticated even if the Release file is not signed or the signature can't be checked. This disables parts of &apt-secure; diff --git a/test/integration/test-sourceslist-arch-plusminus-options b/test/integration/test-sourceslist-arch-plusminus-options new file mode 100755 index 000000000..0d4d7448f --- /dev/null +++ b/test/integration/test-sourceslist-arch-plusminus-options @@ -0,0 +1,85 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' + +testbinaries() { + msgtest 'Test acquired archs for' "$1" + shift + rm -f gotarchs.list + aptget update --print-uris | grep -o '/binary-[a-z0-9-]\+/Packages' | sort > gotarchs.list + while [ -n "$1" ]; do + echo "/binary-${1}/Packages" + shift + done | sort | checkdiff - gotarchs.list && msgpass || msgfail +} + +echo 'deb http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'default & native' 'amd64' +configarchitecture 'amd64' 'i386' +testbinaries 'default & native + foreign' 'amd64' 'i386' +configarchitecture 'amd64' 'i386' 'armel' 'armhf' 'mips' 'mipsel' +testbinaries 'default & native + many foreigns' 'amd64' 'i386' 'armel' 'armhf' 'mips' 'mipsel' + +echo 'deb [arch=amd64] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch=native' 'amd64' + +echo 'deb [arch=mips] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch=foreign' 'mips' + +echo 'deb [arch=kfreebsd-armel] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch=unknown' 'kfreebsd-armel' + +echo 'deb [arch=amd64,i386] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch=native,foreign' 'amd64' 'i386' + +echo 'deb [arch=mips,armhf] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch=foreign,foreign' 'mips' 'armhf' + +echo 'deb [arch=kfreebsd-armel,hurd-powerpc,mipsel,armel] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch=unknown,unknown,foreign,foreign' 'kfreebsd-armel' 'hurd-powerpc' 'mipsel' 'armel' + +echo 'deb [arch+=amd64] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch+=native' 'amd64' 'i386' 'armel' 'armhf' 'mips' 'mipsel' + +echo 'deb [arch+=mips] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch+=foreign' 'amd64' 'i386' 'armel' 'armhf' 'mips' 'mipsel' + +echo 'deb [arch+=mips,armhf,i386] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch+=foreign,foreign,foreign' 'amd64' 'i386' 'armel' 'armhf' 'mips' 'mipsel' + +echo 'deb [arch+=hurd-powerpc] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch+=unknown' 'amd64' 'i386' 'armel' 'armhf' 'mips' 'mipsel' 'hurd-powerpc' + +echo 'deb [arch+=mips,hurd-powerpc,i386] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch+=foreign,unknown,foreign' 'amd64' 'i386' 'armel' 'armhf' 'mips' 'mipsel' 'hurd-powerpc' + +echo 'deb [arch-=amd64] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch-=native' 'i386' 'armel' 'armhf' 'mips' 'mipsel' + +echo 'deb [arch-=mips] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch-=foreign' 'amd64' 'i386' 'armel' 'armhf' 'mipsel' + +echo 'deb [arch-=mips,armhf,i386] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch-=foreign,foreign,foreign' 'amd64' 'armel' 'mipsel' + +echo 'deb [arch-=hurd-powerpc] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch-=unknown' 'amd64' 'i386' 'armel' 'armhf' 'mips' 'mipsel' + +echo 'deb [arch-=mips,hurd-powerpc,i386] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'arch-=foreign,unknown,foreign' 'amd64' 'armel' 'armhf' 'mipsel' + +echo 'deb [arch=mips,i386 arch-=mips] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'substract from a arch-set' 'i386' + +echo 'deb [arch=mips,i386 arch-=mips] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'useless substract from a arch-set' 'i386' + +echo 'deb [arch=mips,i386 arch+=armhf] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'addition to a arch-set' 'i386' 'mips' 'armhf' + +echo 'deb [arch=mips,i386 arch+=mips] http://example.org/debian stable rocks' > rootdir/etc/apt/sources.list +testbinaries 'useless addition to a arch-set' 'i386' 'mips' -- cgit v1.2.3-70-g09d2 From 486d190d3c66bb7271509dc002f8ec9e9eb0166c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 10 Jul 2013 13:06:05 +0200 Subject: prevent MarkInstall of unsynced Multi-Arch:same siblings Multi-Arch: same packages can be co-installed, but need to have the same version for all installed packages (aka "siblings"). Otherwise the unsynced versions will fight against each other and the auto-install as wel as the problem resolver will later have to decide between holding the packages or to remove one of the siblings (usually a foreign) taking a bunch of packages (like the entire foreign setup) with them. The idea here is now to be more pro-active: MarkInstall will fail for a package if the siblings aren't synced, so we don't allow a situation in which a resolver has to decide if to hold or to remove-upgrade under the assumption that the remove-upgrade decision is always wrong and doesn't deserve to be explored (expect valid out-of-syncs of course). Thats a pretty bold move to take for a library which is used by different solvers so this check is done in IsInstallOk and can be overridden if front-ends want to. --- apt-pkg/depcache.cc | 53 ++++++++++- apt-pkg/depcache.h | 30 ++++--- ...prevent-markinstall-multiarch-same-versionscrew | 100 +++++++++++++++++++++ 3 files changed, 166 insertions(+), 17 deletions(-) create mode 100755 test/integration/test-prevent-markinstall-multiarch-same-versionscrew (limited to 'apt-pkg') diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc index 9f8422fec..2c6eb43bf 100644 --- a/apt-pkg/depcache.cc +++ b/apt-pkg/depcache.cc @@ -864,6 +864,11 @@ bool pkgDepCache::MarkDelete(PkgIterator const &Pkg, bool rPurge, dpkg holds are enforced by the private IsModeChangeOk */ bool pkgDepCache::IsDeleteOk(PkgIterator const &Pkg,bool rPurge, unsigned long Depth, bool FromUser) +{ + return IsDeleteOkProtectInstallRequests(Pkg, rPurge, Depth, FromUser); +} +bool pkgDepCache::IsDeleteOkProtectInstallRequests(PkgIterator const &Pkg, + bool const rPurge, unsigned long const Depth, bool const FromUser) { if (FromUser == false && Pkg->CurrentVer == 0) { @@ -1047,9 +1052,10 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg,bool AutoInst, return true; } - // check if we are allowed to install the package - if (IsInstallOk(Pkg,AutoInst,Depth,FromUser) == false) - return false; + // check if we are allowed to install the package (if we haven't already) + if (P.Mode != ModeInstall || P.InstallVer != P.CandidateVer) + if (IsInstallOk(Pkg,AutoInst,Depth,FromUser) == false) + return false; ActionGroup group(*this); P.iFlags &= ~AutoKept; @@ -1271,11 +1277,50 @@ bool pkgDepCache::MarkInstall(PkgIterator const &Pkg,bool AutoInst, /*}}}*/ // DepCache::IsInstallOk - check if it is ok to install this package /*{{{*/ // --------------------------------------------------------------------- -/* The default implementation does nothing. +/* The default implementation checks if the installation of an M-A:same + package would lead us into a version-screw and if so forbids it. dpkg holds are enforced by the private IsModeChangeOk */ bool pkgDepCache::IsInstallOk(PkgIterator const &Pkg,bool AutoInst, unsigned long Depth, bool FromUser) { + return IsInstallOkMultiArchSameVersionSynced(Pkg,AutoInst, Depth, FromUser); +} +bool pkgDepCache::IsInstallOkMultiArchSameVersionSynced(PkgIterator const &Pkg, + bool const AutoInst, unsigned long const Depth, bool const FromUser) +{ + if (FromUser == true) // as always: user is always right + return true; + + // ignore packages with none-M-A:same candidates + VerIterator const CandVer = PkgState[Pkg->ID].CandidateVerIter(*this); + if (unlikely(CandVer.end() == true) || CandVer == Pkg.CurrentVer() || + (CandVer->MultiArch & pkgCache::Version::Same) != pkgCache::Version::Same) + return true; + + GrpIterator const Grp = Pkg.Group(); + for (PkgIterator P = Grp.PackageList(); P.end() == false; P = Grp.NextPkg(P)) + { + // not installed or version synced: fine by definition + // (simple string-compare as stuff like '1' == '0:1-0' can't happen here) + if (P->CurrentVer == 0 || strcmp(Pkg.CandVersion(), P.CandVersion()) == 0) + continue; + // packages loosing M-A:same can be out-of-sync + VerIterator CV = PkgState[P->ID].CandidateVerIter(*this); + if (unlikely(CV.end() == true) || + (CV->MultiArch & pkgCache::Version::Same) != pkgCache::Version::Same) + continue; + + // not downloadable means the package is obsolete, so allow out-of-sync + if (CV.Downloadable() == false) + continue; + + PkgState[Pkg->ID].iFlags |= AutoKept; + if (unlikely(DebugMarker == true)) + std::clog << OutputInDepth(Depth) << "Ignore MarkInstall of " << Pkg + << " as its M-A:same siblings are not version-synced" << std::endl; + return false; + } + return true; } /*}}}*/ diff --git a/apt-pkg/depcache.h b/apt-pkg/depcache.h index 7358048ed..d9c95349b 100644 --- a/apt-pkg/depcache.h +++ b/apt-pkg/depcache.h @@ -442,16 +442,15 @@ class pkgDepCache : protected pkgCache::Namespace /** \return \b true if it's OK for MarkInstall to install * the given package. * - * See the default implementation for a simple example how this - * method can be used. - * Overriding implementations should use the hold-state-flag to cache - * results from previous checks of this package - also it should - * be used if the default resolver implementation is also used to - * ensure that these packages are handled like "normal" dpkg holds. + * The default implementation simply calls all IsInstallOk* + * method mentioned below. + * + * Overriding implementations should use the hold-state-flag to + * cache results from previous checks of this package - if possible. * * The parameters are the same as in the calling MarkInstall: * \param Pkg the package that MarkInstall wants to install. - * \param AutoInst needs a previous MarkInstall this package? + * \param AutoInst install this and all its dependencies * \param Depth recursive deep of this Marker call * \param FromUser was the install requested by the user? */ @@ -461,12 +460,8 @@ class pkgDepCache : protected pkgCache::Namespace /** \return \b true if it's OK for MarkDelete to remove * the given package. * - * See the default implementation for a simple example how this - * method can be used. - * Overriding implementations should use the hold-state-flag to cache - * results from previous checks of this package - also it should - * be used if the default resolver implementation is also used to - * ensure that these packages are handled like "normal" dpkg holds. + * The default implementation simply calls all IsDeleteOk* + * method mentioned below, see also #IsInstallOk. * * The parameters are the same as in the calling MarkDelete: * \param Pkg the package that MarkDelete wants to remove. @@ -498,6 +493,15 @@ class pkgDepCache : protected pkgCache::Namespace pkgDepCache(pkgCache *Cache,Policy *Plcy = 0); virtual ~pkgDepCache(); + protected: + // methods call by IsInstallOk + bool IsInstallOkMultiArchSameVersionSynced(PkgIterator const &Pkg, + bool const AutoInst, unsigned long const Depth, bool const FromUser); + + // methods call by IsDeleteOk + bool IsDeleteOkProtectInstallRequests(PkgIterator const &Pkg, + bool const rPurge, unsigned long const Depth, bool const FromUser); + private: bool IsModeChangeOk(ModeList const mode, PkgIterator const &Pkg, unsigned long const Depth, bool const FromUser); diff --git a/test/integration/test-prevent-markinstall-multiarch-same-versionscrew b/test/integration/test-prevent-markinstall-multiarch-same-versionscrew new file mode 100755 index 000000000..fed12dad0 --- /dev/null +++ b/test/integration/test-prevent-markinstall-multiarch-same-versionscrew @@ -0,0 +1,100 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' 'i386' 'armel' + +insertpackage 'stable' 'allarchs' 'all' '1' +insertpackage 'unstable' 'allarchs' 'all' '2' + +insertinstalledpackage 'fine' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'stable' 'fine' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'unstable' 'fine' 'amd64,i386' '2' 'Multi-Arch: same' + +insertinstalledpackage 'fine-installed' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'stable' 'fine-installed' 'i386,amd64,armel' '1' 'Multi-Arch: same' +insertpackage 'unstable' 'fine-installed' 'i386,amd64' '2' 'Multi-Arch: same' + +insertinstalledpackage 'out-of-sync-native' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'stable' 'out-of-sync-native' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'unstable' 'out-of-sync-native' 'amd64' '2' 'Multi-Arch: same' + +insertinstalledpackage 'out-of-sync-foreign' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'stable' 'out-of-sync-foreign' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'unstable' 'out-of-sync-foreign' 'i386' '2' 'Multi-Arch: same' + +insertinstalledpackage 'out-of-sync-gone-native' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'stable' 'out-of-sync-gone-native' 'i386' '1' 'Multi-Arch: same' +insertpackage 'unstable' 'out-of-sync-gone-native' 'i386' '2' 'Multi-Arch: same' + +insertinstalledpackage 'out-of-sync-gone-foreign' 'i386,amd64' '1' 'Multi-Arch: same' +insertpackage 'stable' 'out-of-sync-gone-foreign' 'amd64' '1' 'Multi-Arch: same' +insertpackage 'unstable' 'out-of-sync-gone-foreign' 'amd64' '2' 'Multi-Arch: same' + +insertpackage 'stable' 'libsame2' 'i386' '1' 'Multi-Arch: same' +insertpackage 'unstable' 'libsame2' 'amd64' '2' 'Multi-Arch: same' +insertpackage 'unstable' 'depender2' 'all' '2' 'Depends: libsame2 (= 2)' +insertpackage 'stable' 'libsame3' 'i386' '1' 'Multi-Arch: same' +insertpackage 'unstable' 'libsame3' 'i386,amd64' '3' 'Multi-Arch: same' +insertpackage 'unstable' 'depender3' 'all' '3' 'Depends: libsame3 (= 3)' +setupaptarchive + +testequal 'Reading package lists... +Building dependency tree... +The following packages will be REMOVED: + out-of-sync-gone-foreign:i386 out-of-sync-gone-native +The following packages have been kept back: + out-of-sync-foreign:i386 out-of-sync-native +The following packages will be upgraded: + fine fine:i386 fine-installed fine-installed:i386 out-of-sync-gone-foreign + out-of-sync-gone-native:i386 +6 upgraded, 0 newly installed, 2 to remove and 2 not upgraded. +Remv out-of-sync-gone-foreign:i386 [1] +Remv out-of-sync-gone-native [1] +Inst fine [1] (2 unstable [amd64]) [fine:amd64 on fine:i386] [fine:i386 on fine:amd64] [fine:i386 ] +Inst fine:i386 [1] (2 unstable [i386]) +Conf fine (2 unstable [amd64]) +Conf fine:i386 (2 unstable [i386]) +Inst fine-installed [1] (2 unstable [amd64]) [fine-installed:amd64 on fine-installed:i386] [fine-installed:i386 on fine-installed:amd64] [fine-installed:i386 ] +Inst fine-installed:i386 [1] (2 unstable [i386]) +Conf fine-installed (2 unstable [amd64]) +Conf fine-installed:i386 (2 unstable [i386]) +Inst out-of-sync-gone-foreign [1] (2 unstable [amd64]) +Inst out-of-sync-gone-native:i386 [1] (2 unstable [i386]) +Conf out-of-sync-gone-foreign (2 unstable [amd64]) +Conf out-of-sync-gone-native:i386 (2 unstable [i386])' aptget dist-upgrade -s #-o Debug::pkgDepCache::Marker=1 + +rm rootdir/var/lib/dpkg/status +insertinstalledpackage 'libsame2' 'i386' '1' 'Multi-Arch: same' +insertinstalledpackage 'libsame3' 'i386' '1' 'Multi-Arch: same' + +# the error message isn't great, but better than nothing, right? +testequal 'Reading package lists... +Building dependency tree... +Some packages could not be installed. This may mean that you have +requested an impossible situation or if you are using the unstable +distribution that some required packages have not yet been created +or been moved out of Incoming. +The following information may help to resolve the situation: + +The following packages have unmet dependencies: + depender2 : Depends: libsame2 (= 2) but it is not going to be installed +E: Unable to correct problems, you have held broken packages.' aptget install depender2 -s + +testequal 'Reading package lists... +Building dependency tree... +The following extra packages will be installed: + libsame3:i386 libsame3 +The following NEW packages will be installed: + depender3 libsame3 +The following packages will be upgraded: + libsame3:i386 +1 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. +Inst libsame3:i386 [1] (3 unstable [i386]) +Inst libsame3 (3 unstable [amd64]) +Inst depender3 (3 unstable [all]) +Conf libsame3:i386 (3 unstable [i386]) +Conf libsame3 (3 unstable [amd64]) +Conf depender3 (3 unstable [all])' aptget install depender3 -s -- cgit v1.2.3-70-g09d2 From 86fdeec2a76f6b134ee8400eb1e53d06fe0974fe Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 16 Jul 2013 23:15:55 +0200 Subject: fix if-clause to generate hook-info for 'rc' packages The code incorrectly skips printing of current version information, if the package has no current version (for APT, but for dpkg as it is the case for packages which are removed but not purged) by using an unintended "else if" rather than an "if". Closes: 717006 --- apt-pkg/deb/dpkgpm.cc | 2 +- ...bug-712116-dpkg-pre-install-pkgs-hook-multiarch | 25 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index fb0473535..d8fc8ef68 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -294,7 +294,7 @@ bool pkgDPkgPM::SendPkgsInfo(FILE * const F, unsigned int const &Version) if (CurVer.end() == true && (I->Op == Item::Remove || I->Op == Item::Purge)) CurVer = FindNowVersion(I->Pkg); - else if (CurVer.end() == true) + if (CurVer.end() == true) { if (Version <= 2) fprintf(F, "- "); diff --git a/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch b/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch index aee44f76b..a89cb7191 100755 --- a/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch +++ b/test/integration/test-bug-712116-dpkg-pre-install-pkgs-hook-multiarch @@ -13,6 +13,13 @@ buildsimplenativepackage 'libsame' 'i386,amd64' '2' 'unstable' 'Multi-Arch: same buildsimplenativepackage 'stuff' 'i386,amd64' '1' 'stable' 'Depends: libsame (= 1), toolkit (= 1)' buildsimplenativepackage 'stuff' 'i386,amd64' '2' 'unstable' 'Depends: libsame (= 2), toolkit (= 2)' +setupsimplenativepackage 'confpkg' 'amd64' '1' 'unstable' +BUILDDIR='incoming/confpkg-1' +echo 'foo "bar";' > ${BUILDDIR}/pkg.conf +echo 'pkg.conf /etc/pkg.conf' >> ${BUILDDIR}/debian/install +buildpackage "$BUILDDIR" 'unstable' 'main' 'amd64' +rm -rf "$BUILDDIR" + setupaptarchive hook='pre-install-pkgs' @@ -93,3 +100,21 @@ testfileequal "${hook}-v3.list" 'libsame 2 amd64 same > - - none **REMOVE** stuff 2 i386 none > - - none **REMOVE** libsame 2 i386 same > - - none **REMOVE** toolkit 2 amd64 foreign > - - none **REMOVE**' + +observehook install confpkg +testfileequal "${hook}-v2.list" 'confpkg - < 1 **CONFIGURE**' +testfileequal "${hook}-v3.list" 'confpkg - - none < 1 amd64 none **CONFIGURE**' + +observehook remove confpkg +testfileequal "${hook}-v2.list" 'confpkg 1 > - **REMOVE**' +testfileequal "${hook}-v3.list" 'confpkg 1 amd64 none > - - none **REMOVE**' + +msgtest 'Conffiles of package remained after remove' 'confpkg' +dpkg -l confpkg | grep -q '^rc' && msgpass || msgfail + +observehook purge confpkg +testfileequal "${hook}-v2.list" 'confpkg 1 > - **REMOVE**' +testfileequal "${hook}-v3.list" 'confpkg 1 amd64 none > - - none **REMOVE**' + +msgtest 'Conffiles are gone after purge' 'confpkg' +dpkg -l confpkg 2>/dev/null | grep -q '^rc' && msgfail || msgpass -- cgit v1.2.3-70-g09d2 From 267275c59cc35704789a228c6e9b1464c4cabd74 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 23 Jul 2013 15:00:53 +0200 Subject: remove double list include --- apt-pkg/cacheset.h | 1 - 1 file changed, 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/cacheset.h b/apt-pkg/cacheset.h index 2a45910ba..d7328d705 100644 --- a/apt-pkg/cacheset.h +++ b/apt-pkg/cacheset.h @@ -11,7 +11,6 @@ // Include Files /*{{{*/ #include #include -#include #include #include #include -- cgit v1.2.3-70-g09d2 From 96ab3c6f62becde2fc67b81c65eef2881856fd22 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:14:31 +0200 Subject: always "delete d" in FileFd::~FileFd to coverity happy --- apt-pkg/contrib/fileutl.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 0f88923cf..edf612810 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1218,11 +1218,9 @@ FileFd::~FileFd() { Close(); if (d != NULL) - { d->CloseDown(FileName); - delete d; - d = NULL; - } + delete d; + d = NULL; } /*}}}*/ // FileFd::Read - Read a bit of the file /*{{{*/ -- cgit v1.2.3-70-g09d2 From 26d5b68a008cfedec113ebdde782c3d18478a5b4 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:14:59 +0200 Subject: fix off-by-one error and do not use magic constant of 100 when checking StackPost --- apt-pkg/contrib/configuration.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/configuration.cc b/apt-pkg/contrib/configuration.cc index 808a708a1..376617401 100644 --- a/apt-pkg/contrib/configuration.cc +++ b/apt-pkg/contrib/configuration.cc @@ -811,7 +811,7 @@ bool ReadConfigFile(Configuration &Conf,const string &FName,bool const &AsSectio // Go down a level if (TermChar == '{') { - if (StackPos <= 100) + if (StackPos < sizeof(Stack)/sizeof(std::string)) Stack[StackPos++] = ParentTag; /* Make sectional tags incorperate the section into the -- cgit v1.2.3-70-g09d2 From fe0036dd7e3bbd808fa526e2e142fdb89105caae Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:22:43 +0200 Subject: rework the code in cdromutl.cc to make coverity (more) happy --- apt-pkg/contrib/cdromutl.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/cdromutl.cc b/apt-pkg/contrib/cdromutl.cc index 187f6bd59..afa01a562 100644 --- a/apt-pkg/contrib/cdromutl.cc +++ b/apt-pkg/contrib/cdromutl.cc @@ -122,8 +122,9 @@ bool MountCdrom(string Path, string DeviceName) if (Child == 0) { // Make all the fds /dev/null + int null_fd = open("/dev/null",O_RDWR); for (int I = 0; I != 3; I++) - dup2(open("/dev/null",O_RDWR),I); + dup2(null_fd, I); if (_config->Exists("Acquire::cdrom::"+Path+"::Mount") == true) { -- cgit v1.2.3-70-g09d2 From 3d165906327828990bec2c58a3c1f4ee77467523 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:26:11 +0200 Subject: fix resource leak (thanks coverity) --- apt-pkg/contrib/fileutl.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index edf612810..398830ff5 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -244,17 +244,21 @@ int GetLock(string File,bool Errors) fl.l_len = 0; if (fcntl(FD,F_SETLK,&fl) == -1) { + // always close to not leak resources + int Tmp = errno; + close(FD); + errno = Tmp; + if (errno == ENOLCK) { + _error->Warning(_("Not using locking for nfs mounted lock file %s"),File.c_str()); return dup(0); // Need something for the caller to close - } + } + if (Errors == true) _error->Errno("open",_("Could not get lock %s"),File.c_str()); - int Tmp = errno; - close(FD); - errno = Tmp; return -1; } -- cgit v1.2.3-70-g09d2 From a80a03448fe11f8ea3fd4a8c525fc413c9ce9037 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:36:23 +0200 Subject: call fdopen() after FileFd was checked --- apt-pkg/indexcopy.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/indexcopy.cc b/apt-pkg/indexcopy.cc index 1d61b974d..a2adb2d00 100644 --- a/apt-pkg/indexcopy.cc +++ b/apt-pkg/indexcopy.cc @@ -714,9 +714,9 @@ bool TranslationsCopy::CopyTranslations(string CDROM,string Name, /*{{{*/ } else { Target.Open(TargetF,FileFd::WriteAtomic); } - FILE *TargetFl = fdopen(dup(Target.Fd()),"w"); if (_error->PendingError() == true) return false; + FILE *TargetFl = fdopen(dup(Target.Fd()),"w"); if (TargetFl == 0) return _error->Errno("fdopen","Failed to reopen fd"); -- cgit v1.2.3-70-g09d2 From 60fc4f21783fddaae481494f99ec157e08a9bfdb Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:41:03 +0200 Subject: apt-pkg/indexcopy.cc: check for pending errors before calling fdopen() --- apt-pkg/indexcopy.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/indexcopy.cc b/apt-pkg/indexcopy.cc index a2adb2d00..4920efff8 100644 --- a/apt-pkg/indexcopy.cc +++ b/apt-pkg/indexcopy.cc @@ -106,9 +106,9 @@ bool IndexCopy::CopyPackages(string CDROM,string Name,vector &List, } else { Target.Open(TargetF,FileFd::WriteAtomic); } - FILE *TargetFl = fdopen(dup(Target.Fd()),"w"); if (_error->PendingError() == true) return false; + FILE *TargetFl = fdopen(dup(Target.Fd()),"w"); if (TargetFl == 0) return _error->Errno("fdopen","Failed to reopen fd"); -- cgit v1.2.3-70-g09d2 From a9d6b0ad873fdf38e7a7077fd1f07289ad66d45a Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:45:53 +0200 Subject: fix resource leak when verification fails --- apt-pkg/indexcopy.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'apt-pkg') diff --git a/apt-pkg/indexcopy.cc b/apt-pkg/indexcopy.cc index 4920efff8..7694cb1dd 100644 --- a/apt-pkg/indexcopy.cc +++ b/apt-pkg/indexcopy.cc @@ -601,6 +601,7 @@ bool SigVerify::CopyAndVerify(string CDROM,string Name,vector &SigList, (useInRelease ? inrelease.c_str() : releasegpg.c_str())); // something went wrong, don't copy the Release.gpg // FIXME: delete any existing gpg file? + delete MetaIndex; continue; } -- cgit v1.2.3-70-g09d2 From 6612c86ef3d8f2b8bccc6212791996ab9e053082 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:46:08 +0200 Subject: delete targets data --- apt-pkg/deb/debmetaindex.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc index 7dd5ab2bf..b597b6f3c 100644 --- a/apt-pkg/deb/debmetaindex.cc +++ b/apt-pkg/deb/debmetaindex.cc @@ -238,6 +238,7 @@ bool debReleaseIndex::GetIndexes(pkgAcquire *Owner, bool const &GetAll) const new pkgAcqIndex(Owner, (*Target)->URI, (*Target)->Description, (*Target)->ShortDesc, HashString()); } + delete targets; // this is normally created in pkgAcqMetaSig, but if we run // in --print-uris mode, we add it here -- cgit v1.2.3-70-g09d2 From 1b7bf822ad9504f6d01cd4422d830e8815143912 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 25 Jul 2013 20:55:18 +0200 Subject: add missing "free(buffer) for allocated buffer --- apt-pkg/contrib/fileutl.cc | 1 - methods/gpgv.cc | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 398830ff5..0b6e07f75 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -251,7 +251,6 @@ int GetLock(string File,bool Errors) if (errno == ENOLCK) { - _error->Warning(_("Not using locking for nfs mounted lock file %s"),File.c_str()); return dup(0); // Need something for the caller to close } diff --git a/methods/gpgv.cc b/methods/gpgv.cc index fe8bac6c9..ea8a26fd4 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -152,6 +152,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, } } fclose(pipein); + free(buffer); int status; waitpid(pid, &status, 0); -- cgit v1.2.3-70-g09d2 From a5b9f4890fc2b367b8f89d10ea2bdb88b8aa071a Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 25 Jul 2013 18:51:16 +0200 Subject: pick up Translation-* even if only compressed available On CD-ROMs Translation-* files are only in compressed form included in the Release file. This used to work while we had no record of Translation-* files in the Release file at all as APT would have just guessed the (compressed) filename and accepted it (unchecked), but now that it checks for the presents of entries and if it finds records it expects the uncompressed to be verifiable. This commit relaxes this requirement again to fix the regression. We are still secure "enough" as we can validate the compressed file we have downloaded, so we don't loose anything by not requiring a hashsum for the uncompressed files to double-check them. Closes: 717665 --- apt-pkg/acquire-item.cc | 15 +++++- .../test-bug-624218-Translation-file-handling | 55 +++++++++++++--------- 2 files changed, 47 insertions(+), 23 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index c48443eff..7bcdf285b 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1369,9 +1369,20 @@ void pkgAcqMetaIndex::QueueIndexes(bool verify) /*{{{*/ { HashString ExpectedIndexHash; const indexRecords::checkSum *Record = MetaIndexParser->Lookup((*Target)->MetaKey); + bool compressedAvailable = false; if (Record == NULL) { - if (verify == true && (*Target)->IsOptional() == false) + if ((*Target)->IsOptional() == true) + { + std::vector types = APT::Configuration::getCompressionTypes(); + for (std::vector::const_iterator t = types.begin(); t != types.end(); ++t) + if (MetaIndexParser->Exists(string((*Target)->MetaKey).append(".").append(*t)) == true) + { + compressedAvailable = true; + break; + } + } + else if (verify == true) { Status = StatAuthError; strprintf(ErrorText, _("Unable to find expected entry '%s' in Release file (Wrong sources.list entry or malformed file)"), (*Target)->MetaKey.c_str()); @@ -1400,7 +1411,7 @@ void pkgAcqMetaIndex::QueueIndexes(bool verify) /*{{{*/ if ((*Target)->IsSubIndex() == true) new pkgAcqSubIndex(Owner, (*Target)->URI, (*Target)->Description, (*Target)->ShortDesc, ExpectedIndexHash); - else if (transInRelease == false || MetaIndexParser->Exists((*Target)->MetaKey) == true) + else if (transInRelease == false || Record != NULL || compressedAvailable == true) { if (_config->FindB("Acquire::PDiffs",true) == true && transInRelease == true && MetaIndexParser->Exists(string((*Target)->MetaKey).append(".diff/Index")) == true) diff --git a/test/integration/test-bug-624218-Translation-file-handling b/test/integration/test-bug-624218-Translation-file-handling index d146b943c..d3c5b08ac 100755 --- a/test/integration/test-bug-624218-Translation-file-handling +++ b/test/integration/test-bug-624218-Translation-file-handling @@ -14,34 +14,47 @@ changetowebserver rm -rf rootdir/var/lib/apt/lists -msgtest 'No download of non-existent locals' 'with Index' -LC_ALL="" aptget update -o Acquire::Languages=en | grep -q -e 'Translation-[^e][^n] ' && msgfail || msgpass -rm -rf rootdir/var/lib/apt/lists +translationslisted() { + msgtest 'No download of non-existent locals' "$1" + LC_ALL="" aptget update -o Acquire::Languages=en | grep -q -e 'Translation-[^e][^n] ' && msgfail || msgpass + rm -rf rootdir/var/lib/apt/lists -msgtest 'Download of existent locals' 'with Index' -LC_ALL="" aptget update | grep -q -e 'Translation-en ' && msgpass || msgfail -rm -rf rootdir/var/lib/apt/lists + msgtest 'Download of existent locals' "$1" + LC_ALL="" aptget update | grep -q -e 'Translation-en ' && msgpass || msgfail + rm -rf rootdir/var/lib/apt/lists -msgtest 'Download of en in LC_ALL=C' 'with Index' -LC_ALL=C aptget update | grep -q -e 'Translation-en ' && msgpass || msgfail -rm -rf rootdir/var/lib/apt/lists + msgtest 'Download of en in LC_ALL=C' "$1" + LC_ALL=C aptget update | grep -q -e 'Translation-en ' && msgpass || msgfail + rm -rf rootdir/var/lib/apt/lists -msgtest 'Download of en as forced language' 'with Index' -aptget update -o Acquire::Languages=en | grep -q -e 'Translation-en ' && msgpass || msgfail -rm -rf rootdir/var/lib/apt/lists + msgtest 'Download of en as forced language' "$1" + aptget update -o Acquire::Languages=en | grep -q -e 'Translation-en ' && msgpass || msgfail + rm -rf rootdir/var/lib/apt/lists -msgtest 'Download of nothing else in forced language' 'with Index' -aptget update -o Acquire::Languages=en | grep -q -e 'Translation-[^e][^n] ' && msgfail || msgpass -rm -rf rootdir/var/lib/apt/lists + msgtest 'Download of nothing else in forced language' "$1" + aptget update -o Acquire::Languages=en | grep -q -e 'Translation-[^e][^n] ' && msgfail || msgpass + rm -rf rootdir/var/lib/apt/lists -msgtest 'Download no Translation- if forced language is non-existent' 'with Index' -aptget update -o Acquire::Languages=ast_DE | grep -q -e 'Translation-' && msgfail || msgpass -rm -rf rootdir/var/lib/apt/lists + msgtest 'Download no Translation- if forced language is non-existent' "$1" + aptget update -o Acquire::Languages=ast_DE | grep -q -e 'Translation-' && msgfail || msgpass + rm -rf rootdir/var/lib/apt/lists + + msgtest 'Download of nothing if none is forced' "$1" + aptget update -o Acquire::Languages=none | grep -q -e 'Translation' && msgfail || msgpass + rm -rf rootdir/var/lib/apt/lists +} + +translationslisted 'with full Index' + + +# only compressed files available (as it happens on CD-ROM) +sed -i '/i18n\/Translation-[^.]*$/ d' $(find aptarchive -name 'Release') +signreleasefiles + +translationslisted 'with partial Index' -msgtest 'Download of nothing if none is forced' 'with Index' -aptget update -o Acquire::Languages=none | grep -q -e 'Translation' && msgfail || msgpass -rm -rf rootdir/var/lib/apt/lists +# no records at all about Translation files (fallback to guessing) sed -i '/i18n\/Translation-.*$/ d' $(find aptarchive -name 'Release') signreleasefiles -- cgit v1.2.3-70-g09d2 From 9ca0d58180366275f5e62b6c5aad066b7b09a1de Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 26 Jul 2013 12:04:55 +0200 Subject: proper cleanup varargs in _error (uncovered by Coverity) Git-Dch: Ignore --- apt-pkg/contrib/error.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/error.cc b/apt-pkg/contrib/error.cc index 122e2c809..d457781c3 100644 --- a/apt-pkg/contrib/error.cc +++ b/apt-pkg/contrib/error.cc @@ -67,9 +67,10 @@ bool GlobalError::NAME (const char *Function, const char *Description,...) { \ int const errsv = errno; \ while (true) { \ va_start(args,Description); \ - if (InsertErrno(TYPE, Function, Description, args, errsv, msgSize) == false) \ - break; \ + bool const retry = InsertErrno(TYPE, Function, Description, args, errsv, msgSize); \ va_end(args); \ + if (retry == false) \ + break; \ } \ return false; \ } @@ -88,9 +89,10 @@ bool GlobalError::InsertErrno(MsgType const &type, const char *Function, int const errsv = errno; while (true) { va_start(args,Description); - if (InsertErrno(type, Function, Description, args, errsv, msgSize) == false) - break; + bool const retry = InsertErrno(type, Function, Description, args, errsv, msgSize); va_end(args); + if (retry == false) + break; } return false; } -- cgit v1.2.3-70-g09d2 From 638593bd058fc0c715a76de92ff5bd6691c101e4 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 26 Jul 2013 12:20:50 +0200 Subject: ensure that FileFd::Size returns 0 in error cases --- apt-pkg/contrib/fileutl.cc | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 0f88923cf..5debb4f92 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1598,7 +1598,11 @@ unsigned long long FileFd::Size() char ignore[1000]; unsigned long long read = 0; do { - Read(ignore, sizeof(ignore), &read); + if (Read(ignore, sizeof(ignore), &read) == false) + { + Seek(oldSeek); + return 0; + } } while(read != 0); size = Tell(); Seek(oldSeek); @@ -1615,10 +1619,16 @@ unsigned long long FileFd::Size() * bits of the file */ // FIXME: Size for gz-files is limited by 32bit… no largefile support if (lseek(iFd, -4, SEEK_END) < 0) - return FileFdErrno("lseek","Unable to seek to end of gzipped file"); - size = 0L; + { + FileFdErrno("lseek","Unable to seek to end of gzipped file"); + return 0; + } + size = 0; if (read(iFd, &size, 4) != 4) - return FileFdErrno("read","Unable to read original size of gzipped file"); + { + FileFdErrno("read","Unable to read original size of gzipped file"); + return 0; + } #ifdef WORDS_BIGENDIAN uint32_t tmp_size = size; @@ -1628,7 +1638,10 @@ unsigned long long FileFd::Size() #endif if (lseek(iFd, oldPos, SEEK_SET) < 0) - return FileFdErrno("lseek","Unable to seek in gzipped file"); + { + FileFdErrno("lseek","Unable to seek in gzipped file"); + return 0; + } return size; } -- cgit v1.2.3-70-g09d2 From 36a0c0f78675d10cae840a72268f70aed667fb96 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 26 Jul 2013 22:10:05 +0200 Subject: fix some coverity chroot() releated warnings --- apt-pkg/aptconfiguration.cc | 4 ++-- apt-pkg/deb/dpkgpm.cc | 1 + cmdline/apt-mark.cc | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/aptconfiguration.cc b/apt-pkg/aptconfiguration.cc index 37c846582..e32e553a4 100644 --- a/apt-pkg/aptconfiguration.cc +++ b/apt-pkg/aptconfiguration.cc @@ -388,12 +388,12 @@ std::vector const Configuration::getArchitectures(bool const &Cache if (dpkgMultiArch == 0) { close(external[0]); std::string const chrootDir = _config->FindDir("DPkg::Chroot-Directory"); - if (chrootDir != "/" && chroot(chrootDir.c_str()) != 0) - _error->WarningE("getArchitecture", "Couldn't chroot into %s for dpkg --print-foreign-architectures", chrootDir.c_str()); int const nullfd = open("/dev/null", O_RDONLY); dup2(nullfd, STDIN_FILENO); dup2(external[1], STDOUT_FILENO); dup2(nullfd, STDERR_FILENO); + if (chrootDir != "/" && chroot(chrootDir.c_str()) != 0) + _error->WarningE("getArchitecture", "Couldn't chroot into %s for dpkg --print-foreign-architectures", chrootDir.c_str()); execvp(Args[0], (char**) &Args[0]); _error->WarningE("getArchitecture", "Can't detect foreign architectures supported by dpkg!"); _exit(100); diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index fb0473535..588ab68c4 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -134,6 +134,7 @@ static void dpkgChrootDirectory() std::cerr << "Chrooting into " << chrootDir << std::endl; if (chroot(chrootDir.c_str()) != 0) _exit(100); + chdir("/"); } /*}}}*/ diff --git a/cmdline/apt-mark.cc b/cmdline/apt-mark.cc index c5b7ca496..4c0fc2893 100644 --- a/cmdline/apt-mark.cc +++ b/cmdline/apt-mark.cc @@ -202,13 +202,13 @@ bool DoHold(CommandLine &CmdL) if (dpkgAssertMultiArch == 0) { std::string const chrootDir = _config->FindDir("DPkg::Chroot-Directory"); - if (chrootDir != "/" && chroot(chrootDir.c_str()) != 0) - _error->WarningE("getArchitecture", "Couldn't chroot into %s for dpkg --assert-multi-arch", chrootDir.c_str()); // redirect everything to the ultimate sink as we only need the exit-status int const nullfd = open("/dev/null", O_RDONLY); dup2(nullfd, STDIN_FILENO); dup2(nullfd, STDOUT_FILENO); dup2(nullfd, STDERR_FILENO); + if (chrootDir != "/" && chroot(chrootDir.c_str()) != 0) + _error->WarningE("getArchitecture", "Couldn't chroot into %s for dpkg --assert-multi-arch", chrootDir.c_str()); execvp(Args[0], (char**) &Args[0]); _error->WarningE("dpkgGo", "Can't detect if dpkg supports multi-arch!"); _exit(2); -- cgit v1.2.3-70-g09d2 From 163dc55bd6891008adcdf6d683a94e890a00f8c7 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 26 Jul 2013 22:18:36 +0200 Subject: fix another missing va_end() --- apt-pkg/contrib/strutl.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index d0e74d8c5..df02c3499 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -1233,12 +1233,12 @@ char *safe_snprintf(char *Buffer,char *End,const char *Format,...) va_list args; int Did; - va_start(args,Format); - if (End <= Buffer) return End; - + va_start(args,Format); Did = vsnprintf(Buffer,End - Buffer,Format,args); + va_end(args); + if (Did < 0 || Buffer + Did > End) return End; return Buffer + Did; -- cgit v1.2.3-70-g09d2 From 096bd9f59750f6bea754d3a05e240c9eea0f6b53 Mon Sep 17 00:00:00 2001 From: Colin Watson Date: Thu, 1 Aug 2013 13:19:43 +0200 Subject: prefer native arch over higher priority for providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rational from the buglog: > The problem here is that the Priority field in one of the Packages files > is incorrect due to a mishap with reprepro configuration, […] the > amd64 version is Priority: standard but the arm64 version is Priority: > optional (and has a stray "optional: interpreters" field). > […] > However, Priority is a rather weak property of a package because it's > typically applied via overrides, and it's easy for maintainers of > third-party repositories to misconfigure them so that overrides aren't > applied correctly. It shouldn't be ranked ahead of choosing packages > from the native architecture. In this case, I have no user-mode > emulation for arm64 set up, so choosing m4:arm64 simply won't work. This effectly makes the priority the least interesting data point in chosing a provider, which is in line with the other checks we have already order above priority in the past and also has a certain appeal by the soft irony it provides. Closes: #718482 --- apt-pkg/depcache.cc | 6 +++--- ...prefer-native-architecture-over-higher-priority | 25 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100755 test/integration/test-prefer-native-architecture-over-higher-priority (limited to 'apt-pkg') diff --git a/apt-pkg/depcache.cc b/apt-pkg/depcache.cc index 2c6eb43bf..978a893f7 100644 --- a/apt-pkg/depcache.cc +++ b/apt-pkg/depcache.cc @@ -1007,9 +1007,6 @@ struct CompareProviders { else if ((B->Flags & pkgCache::Flag::Important) == pkgCache::Flag::Important) return true; } - // higher priority seems like a good idea - if (AV->Priority != BV->Priority) - return AV->Priority > BV->Priority; // prefer native architecture if (strcmp(A.Arch(), B.Arch()) != 0) { @@ -1024,6 +1021,9 @@ struct CompareProviders { else if (*a == B.Arch()) return true; } + // higher priority seems like a good idea + if (AV->Priority != BV->Priority) + return AV->Priority > BV->Priority; // unable to decide… return A->ID < B->ID; } diff --git a/test/integration/test-prefer-native-architecture-over-higher-priority b/test/integration/test-prefer-native-architecture-over-higher-priority new file mode 100755 index 000000000..2e5696376 --- /dev/null +++ b/test/integration/test-prefer-native-architecture-over-higher-priority @@ -0,0 +1,25 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework +setupenvironment +configarchitecture 'amd64' 'arm64' + +insertpackage 'unstable' 'm4' 'amd64' '1' 'Multi-Arch: foreign' 'optional' +insertpackage 'unstable' 'm4' 'arm64' '1' 'Multi-Arch: foreign' 'standard' +insertpackage 'unstable' 'autoconf' 'all' '1' 'Depends: m4' + +setupaptarchive + +testequal 'Reading package lists... +Building dependency tree... +The following extra packages will be installed: + m4 +The following NEW packages will be installed: + autoconf m4 +0 upgraded, 2 newly installed, 0 to remove and 0 not upgraded. +Inst m4 (1 unstable [amd64]) +Inst autoconf (1 unstable [all]) +Conf m4 (1 unstable [amd64]) +Conf autoconf (1 unstable [all])' aptget install autoconf -s -- cgit v1.2.3-70-g09d2 From 4ccd3b3ce69d6a6598e1773689a44fdec78a85cc Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 5 Aug 2013 22:40:28 +0200 Subject: fix some unitialized data members --- apt-pkg/cdrom.h | 2 +- apt-pkg/contrib/sha2.h | 4 +++- apt-pkg/tagfile.h | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/cdrom.h b/apt-pkg/cdrom.h index 4fc3d3928..7d19eb813 100644 --- a/apt-pkg/cdrom.h +++ b/apt-pkg/cdrom.h @@ -18,7 +18,7 @@ class pkgCdromStatus /*{{{*/ int totalSteps; public: - pkgCdromStatus() {}; + pkgCdromStatus() : totalSteps(0) {}; virtual ~pkgCdromStatus() {}; // total steps diff --git a/apt-pkg/contrib/sha2.h b/apt-pkg/contrib/sha2.h index 51c921dbd..8e0c99a1b 100644 --- a/apt-pkg/contrib/sha2.h +++ b/apt-pkg/contrib/sha2.h @@ -60,10 +60,11 @@ class SHA256Summation : public SHA2SummationBase res.Set(Sum); return res; }; - SHA256Summation() + SHA256Summation() { SHA256_Init(&ctx); Done = false; + memset(&Sum, 0, sizeof(Sum)); }; }; @@ -96,6 +97,7 @@ class SHA512Summation : public SHA2SummationBase { SHA512_Init(&ctx); Done = false; + memset(&Sum, 0, sizeof(Sum)); }; }; diff --git a/apt-pkg/tagfile.h b/apt-pkg/tagfile.h index 4718f5101..126f4219d 100644 --- a/apt-pkg/tagfile.h +++ b/apt-pkg/tagfile.h @@ -84,7 +84,7 @@ class pkgTagSection Stop = this->Stop; }; - pkgTagSection() : Section(0), TagCount(0), Stop(0) {}; + pkgTagSection() : Section(0), TagCount(0), Stop(0), d(NULL) {}; virtual ~pkgTagSection() {}; }; -- cgit v1.2.3-70-g09d2 From 1680800209dc85387c4c2da9a1edfcde44f87807 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 7 Aug 2013 18:00:07 +0200 Subject: specific pins below 1000 cause downgrades MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We start your quest by using the version of a package applying to a specific pin, but that version could very well be below the current version, which causes APT to suggest a downgrade even if it is advertised that it never does this below 1000. Its of course questionable what use a specific pin on a package has which has a newer version already installed, but reacting with the suggestion of a downgrade is really not appropriated (even if its kinda likely that this is actually the intend the user has – it could just as well be an outdated pin) and as pinning is complicated enough we should atleast do what is described in the manpage. So we look out for the specific pin and if we haven't seen it at the moment we see the installed version, we ignore the specific pin. Closes: 543966 --- apt-pkg/policy.cc | 25 +++++-- .../test-bug-543966-downgrade-below-1000-pin | 81 ++++++++++++++++++++++ 2 files changed, 99 insertions(+), 7 deletions(-) create mode 100755 test/integration/test-bug-543966-downgrade-below-1000-pin (limited to 'apt-pkg') diff --git a/apt-pkg/policy.cc b/apt-pkg/policy.cc index 4ae3b5f87..0a06cc6e3 100644 --- a/apt-pkg/policy.cc +++ b/apt-pkg/policy.cc @@ -166,11 +166,15 @@ pkgCache::VerIterator pkgPolicy::GetCandidateVer(pkgCache::PkgIterator const &Pk tracks the default when the default is taken away, and a permanent pin that stays at that setting. */ + bool PrefSeen = false; for (pkgCache::VerIterator Ver = Pkg.VersionList(); Ver.end() == false; ++Ver) { /* Lets see if this version is the installed version */ bool instVer = (Pkg.CurrentVer() == Ver); + if (Pref == Ver) + PrefSeen = true; + for (pkgCache::VerFileIterator VF = Ver.FileList(); VF.end() == false; ++VF) { /* If this is the status file, and the current version is not the @@ -187,26 +191,33 @@ pkgCache::VerIterator pkgPolicy::GetCandidateVer(pkgCache::PkgIterator const &Pk { Pref = Ver; Max = Prio; + PrefSeen = true; } if (Prio > MaxAlt) { PrefAlt = Ver; MaxAlt = Prio; - } - } - + } + } + if (instVer == true && Max < 1000) { + /* Not having seen the Pref yet means we have a specific pin below 1000 + on a version below the current installed one, so ignore the specific pin + as this would be a downgrade otherwise */ + if (PrefSeen == false || Pref.end() == true) + { + Pref = Ver; + PrefSeen = true; + } /* Elevate our current selection (or the status file itself) to the Pseudo-status priority. */ - if (Pref.end() == true) - Pref = Ver; Max = 1000; - + // Fast path optimize. if (StatusOverride == false) break; - } + } } // If we do not find our candidate, use the one with the highest pin. // This means that if there is a version available with pin > 0; there diff --git a/test/integration/test-bug-543966-downgrade-below-1000-pin b/test/integration/test-bug-543966-downgrade-below-1000-pin new file mode 100755 index 000000000..f602bea95 --- /dev/null +++ b/test/integration/test-bug-543966-downgrade-below-1000-pin @@ -0,0 +1,81 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'i386' + +insertpackage 'unstable' 'base-files' 'all' '5.0.0' +insertinstalledpackage 'base-files' 'all' '5.0.0-1' + +setupaptarchive + +STATUS=$(readlink -f rootdir/var/lib/dpkg/status) +APTARCHIVE="$(readlink -f aptarchive)/" + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0-1 + Version table: + *** 5.0.0-1 0 + 100 $STATUS + 5.0.0 0 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=0 + +echo 'Package: base-files +Pin: release a=unstable +Pin-Priority: 99' > rootdir/etc/apt/preferences + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0-1 + Package pin: 5.0.0 + Version table: + *** 5.0.0-1 99 + 100 $STATUS + 5.0.0 99 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=99 + +echo 'Package: base-files +Pin: release a=unstable +Pin-Priority: 100' > rootdir/etc/apt/preferences + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0-1 + Package pin: 5.0.0 + Version table: + *** 5.0.0-1 100 + 100 $STATUS + 5.0.0 100 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=100 + +echo 'Package: base-files +Pin: release a=unstable +Pin-Priority: 999' > rootdir/etc/apt/preferences + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0-1 + Package pin: 5.0.0 + Version table: + *** 5.0.0-1 999 + 100 $STATUS + 5.0.0 999 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=999 + +echo 'Package: base-files +Pin: release a=unstable +Pin-Priority: 1000' > rootdir/etc/apt/preferences + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0 + Package pin: 5.0.0 + Version table: + *** 5.0.0-1 1000 + 100 $STATUS + 5.0.0 1000 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=1000 -- cgit v1.2.3-70-g09d2 From 91c4cc14d3654636edf997d23852f05ad3de4853 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 7 Aug 2013 16:40:39 +0200 Subject: stop skipping "-----" sections in Release files The file we read will always be a Release file as the clearsign is stripped earlier in this method, so this check is just wasting CPU Its also removing the risk that this could ever be part of a valid section, even if I can't imagine how that should be valid. Git-Dch: Ignore --- apt-pkg/indexrecords.cc | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/indexrecords.cc b/apt-pkg/indexrecords.cc index e37a78cfb..6d89949a0 100644 --- a/apt-pkg/indexrecords.cc +++ b/apt-pkg/indexrecords.cc @@ -62,7 +62,7 @@ bool indexRecords::Load(const string Filename) /*{{{*/ if (OpenMaybeClearSignedFile(Filename, Fd) == false) return false; - pkgTagFile TagFile(&Fd, Fd.Size() + 256); // XXX + pkgTagFile TagFile(&Fd); if (_error->PendingError() == true) { strprintf(ErrorText, _("Unable to parse Release file %s"),Filename.c_str()); @@ -71,16 +71,11 @@ bool indexRecords::Load(const string Filename) /*{{{*/ pkgTagSection Section; const char *Start, *End; - // Skip over sections beginning with ----- as this is an idicator for clearsigns - do { - if (TagFile.Step(Section) == false) - { - strprintf(ErrorText, _("No sections in Release file %s"), Filename.c_str()); - return false; - } - - Section.Get (Start, End, 0); - } while (End - Start > 5 && strncmp(Start, "-----", 5) == 0); + if (TagFile.Step(Section) == false) + { + strprintf(ErrorText, _("No sections in Release file %s"), Filename.c_str()); + return false; + } Suite = Section.FindS("Suite"); Dist = Section.FindS("Codename"); -- cgit v1.2.3-70-g09d2 From e9737c7f6a3e03b2975927ef9b04c1194026ed9c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 7 Aug 2013 16:45:49 +0200 Subject: use pkgTagFile to parse "header" of Release files The handwritten parsing here was mostly done as we couldn't trust the Release file we got, but nowadays we are sure that the Release file is valid and contains just a single section we want it to include. Beside reducing code it also fixes a bug: Fieldnames in deb822 formatted files are case-insensitive and pkgTagFile does it correctly, but this selfbuilt stuff here didn't. --- apt-pkg/deb/deblistparser.cc | 104 ++++++++----------------------------------- 1 file changed, 19 insertions(+), 85 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc index 28857176b..c2707d0a5 100644 --- a/apt-pkg/deb/deblistparser.cc +++ b/apt-pkg/deb/deblistparser.cc @@ -805,94 +805,28 @@ bool debListParser::LoadReleaseInfo(pkgCache::PkgFileIterator &FileI, map_ptrloc const storage = WriteUniqString(component); FileI->Component = storage; - // FIXME: should use FileFd and TagSection - FILE* release = fdopen(dup(File.Fd()), "r"); - if (release == NULL) + pkgTagFile TagFile(&File); + pkgTagSection Section; + if (_error->PendingError() == true || TagFile.Step(Section) == false) return false; - char buffer[101]; - while (fgets(buffer, sizeof(buffer), release) != NULL) - { - size_t len = 0; - - // Skip empty lines - for (; buffer[len] == '\r' && buffer[len] == '\n'; ++len) - /* nothing */ - ; - if (buffer[len] == '\0') - continue; - - // seperate the tag from the data - const char* dataStart = strchr(buffer + len, ':'); - if (dataStart == NULL) - continue; - len = dataStart - buffer; - for (++dataStart; *dataStart == ' '; ++dataStart) - /* nothing */ - ; - const char* dataEnd = (const char*)rawmemchr(dataStart, '\0'); - // The last char should be a newline, but we can never be sure: #633350 - const char* lineEnd = dataEnd; - for (--lineEnd; *lineEnd == '\r' || *lineEnd == '\n'; --lineEnd) - /* nothing */ - ; - ++lineEnd; - - // which datastorage need to be updated - enum { Suite, Component, Version, Origin, Codename, Label, None } writeTo = None; - if (buffer[0] == ' ') - ; - #define APT_PARSER_WRITETO(X) else if (strncmp(#X, buffer, len) == 0) writeTo = X; - APT_PARSER_WRITETO(Suite) - APT_PARSER_WRITETO(Component) - APT_PARSER_WRITETO(Version) - APT_PARSER_WRITETO(Origin) - APT_PARSER_WRITETO(Codename) - APT_PARSER_WRITETO(Label) - #undef APT_PARSER_WRITETO - #define APT_PARSER_FLAGIT(X) else if (strncmp(#X, buffer, len) == 0) \ - pkgTagSection::FindFlag(FileI->Flags, pkgCache::Flag:: X, dataStart, lineEnd); - APT_PARSER_FLAGIT(NotAutomatic) - APT_PARSER_FLAGIT(ButAutomaticUpgrades) - #undef APT_PARSER_FLAGIT - - // load all data from the line and save it - string data; - if (writeTo != None) - data.append(dataStart, dataEnd); - if (sizeof(buffer) - 1 == (dataEnd - buffer)) - { - while (fgets(buffer, sizeof(buffer), release) != NULL) - { - if (writeTo != None) - data.append(buffer); - if (strlen(buffer) != sizeof(buffer) - 1) - break; - } - } - if (writeTo != None) - { - // remove spaces and stuff from the end of the data line - for (std::string::reverse_iterator s = data.rbegin(); - s != data.rend(); ++s) - { - if (*s != '\r' && *s != '\n' && *s != ' ') - break; - *s = '\0'; - } - map_ptrloc const storage = WriteUniqString(data); - switch (writeTo) { - case Suite: FileI->Archive = storage; break; - case Component: FileI->Component = storage; break; - case Version: FileI->Version = storage; break; - case Origin: FileI->Origin = storage; break; - case Codename: FileI->Codename = storage; break; - case Label: FileI->Label = storage; break; - case None: break; - } - } + std::string data; + #define APT_INRELEASE(TAG, STORE) \ + data = Section.FindS(TAG); \ + if (data.empty() == false) \ + { \ + map_ptrloc const storage = WriteUniqString(data); \ + STORE = storage; \ } - fclose(release); + APT_INRELEASE("Suite", FileI->Archive) + APT_INRELEASE("Component", FileI->Component) + APT_INRELEASE("Version", FileI->Version) + APT_INRELEASE("Origin", FileI->Origin) + APT_INRELEASE("Codename", FileI->Codename) + APT_INRELEASE("Label", FileI->Label) + #undef APT_INRELEASE + Section.FindFlag("NotAutomatic", FileI->Flags, pkgCache::Flag::NotAutomatic); + Section.FindFlag("ButAutomaticUpgrades", FileI->Flags, pkgCache::Flag::ButAutomaticUpgrades); return !_error->PendingError(); } -- cgit v1.2.3-70-g09d2 From f52037d629aea696f938015e7f1ec037eb079af8 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 8 Aug 2013 15:40:15 +0200 Subject: fix -Wall errors --- apt-pkg/deb/dpkgpm.cc | 6 ++++-- apt-pkg/tagfile.h | 2 +- apt-pkg/vendorlist.cc | 2 +- cmdline/apt-cdrom.cc | 3 ++- ftparchive/writer.cc | 3 ++- 5 files changed, 10 insertions(+), 6 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index b0bd6b184..34ae4e593 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -134,7 +134,8 @@ static void dpkgChrootDirectory() std::cerr << "Chrooting into " << chrootDir << std::endl; if (chroot(chrootDir.c_str()) != 0) _exit(100); - chdir("/"); + if (chdir("/") != 0) + _exit(100); } /*}}}*/ @@ -755,7 +756,8 @@ bool pkgDPkgPM::OpenLog() pw = getpwnam("root"); gr = getgrnam("adm"); if (pw != NULL && gr != NULL) - chown(logfile_name.c_str(), pw->pw_uid, gr->gr_gid); + if(chown(logfile_name.c_str(), pw->pw_uid, gr->gr_gid) != 0) + _error->Errno("OpenLog", "chown failed"); chmod(logfile_name.c_str(), 0640); fprintf(d->term_out, "\nLog started: %s\n", timestr); } diff --git a/apt-pkg/tagfile.h b/apt-pkg/tagfile.h index 126f4219d..fedd72701 100644 --- a/apt-pkg/tagfile.h +++ b/apt-pkg/tagfile.h @@ -84,7 +84,7 @@ class pkgTagSection Stop = this->Stop; }; - pkgTagSection() : Section(0), TagCount(0), Stop(0), d(NULL) {}; + pkgTagSection() : Section(0), TagCount(0), d(NULL), Stop(0) {}; virtual ~pkgTagSection() {}; }; diff --git a/apt-pkg/vendorlist.cc b/apt-pkg/vendorlist.cc index ecfc7db87..602425624 100644 --- a/apt-pkg/vendorlist.cc +++ b/apt-pkg/vendorlist.cc @@ -66,7 +66,7 @@ bool pkgVendorList::CreateList(Configuration& Cnf) /*{{{*/ Configuration Block(Top); string VendorID = Top->Tag; vector *Fingerprints = new vector; - struct Vendor::Fingerprint *Fingerprint = new struct Vendor::Fingerprint; + struct Vendor::Fingerprint *Fingerprint = new struct Vendor::Fingerprint(); string Origin = Block.Find("Origin"); Fingerprint->Print = Block.Find("Fingerprint"); diff --git a/cmdline/apt-cdrom.cc b/cmdline/apt-cdrom.cc index c153cca85..545edf439 100644 --- a/cmdline/apt-cdrom.cc +++ b/cmdline/apt-cdrom.cc @@ -66,7 +66,8 @@ void pkgCdromTextStatus::Prompt(const char *Text) { char C; cout << Text << ' ' << flush; - read(STDIN_FILENO,&C,1); + if (read(STDIN_FILENO,&C,1) < 0) + _error->Errno("pkgCdromTextStatus::Prompt", "failed to prompt"); if (C != '\n') cout << endl; } diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc index 3283128d8..7ecfe78ed 100644 --- a/ftparchive/writer.cc +++ b/ftparchive/writer.cc @@ -284,7 +284,8 @@ bool FTWScanner::Delink(string &FileName,const char *OriginalPath, if (link(FileName.c_str(),OriginalPath) != 0) { // Panic! Restore the symlink - symlink(OldLink,OriginalPath); + if (symlink(OldLink,OriginalPath) != 0) + _error->Errno("symlink", "failed to restore symlink"); return _error->Errno("link",_("*** Failed to link %s to %s"), FileName.c_str(), OriginalPath); -- cgit v1.2.3-70-g09d2 From ffcccd62f0d5822a71e24baf84126af5c93a5e69 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 10 Aug 2013 12:40:37 +0200 Subject: fix: --print-uris removes authentication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The constructors of our (clear)sign-acquire-items move a pre-existent file for error-recovery away, which gets restored or discarded later as the acquire progresses, but --print-uris never really starts the acquire process, so the files aren't restored (as they should). To fix this both get a destructor which checks for signs of acquire doing anything and if it hasn't the file is restored. Note that these virtual destructors theoretically break the API, but only with classes extending the sign-acquire-items and nobody does this, as it would be insane for library users to fiddle with Acquire internals – and these classes are internals. Closes: 719263 --- apt-pkg/acquire-item.cc | 31 +++++++++++++--- apt-pkg/acquire-item.h | 2 ++ test/integration/test-bug-602412-dequote-redirect | 2 +- ...st-bug-719263-print-uris-removes-authentication | 41 ++++++++++++++++++++++ 4 files changed, 71 insertions(+), 5 deletions(-) create mode 100755 test/integration/test-bug-719263-print-uris-removes-authentication (limited to 'apt-pkg') diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc index 7bcdf285b..95dadcd6d 100644 --- a/apt-pkg/acquire-item.cc +++ b/apt-pkg/acquire-item.cc @@ -1067,8 +1067,7 @@ pkgAcqMetaSig::pkgAcqMetaSig(pkgAcquire *Owner, /*{{{*/ string Final = _config->FindDir("Dir::State::lists"); Final += URItoFileName(RealURI); - struct stat Buf; - if (stat(Final.c_str(),&Buf) == 0) + if (RealFileExists(Final) == true) { // File was already in place. It needs to be re-downloaded/verified // because Release might have changed, we do give it a differnt @@ -1080,6 +1079,19 @@ pkgAcqMetaSig::pkgAcqMetaSig(pkgAcquire *Owner, /*{{{*/ } QueueURI(Desc); +} + /*}}}*/ +pkgAcqMetaSig::~pkgAcqMetaSig() /*{{{*/ +{ + // if the file was never queued undo file-changes done in the constructor + if (QueueCounter == 1 && Status == StatIdle && FileSize == 0 && Complete == false && + LastGoodSig.empty() == false) + { + string const Final = _config->FindDir("Dir::State::lists") + URItoFileName(RealURI); + if (RealFileExists(Final) == false && RealFileExists(LastGoodSig) == true) + Rename(LastGoodSig, Final); + } + } /*}}}*/ // pkgAcqMetaSig::Custom600Headers - Insert custom request headers /*{{{*/ @@ -1595,14 +1607,25 @@ pkgAcqMetaClearSig::pkgAcqMetaClearSig(pkgAcquire *Owner, /*{{{*/ // keep the old InRelease around in case of transistent network errors string const Final = _config->FindDir("Dir::State::lists") + URItoFileName(RealURI); - struct stat Buf; - if (stat(Final.c_str(),&Buf) == 0) + if (RealFileExists(Final) == true) { string const LastGoodSig = DestFile + ".reverify"; Rename(Final,LastGoodSig); } } /*}}}*/ +pkgAcqMetaClearSig::~pkgAcqMetaClearSig() /*{{{*/ +{ + // if the file was never queued undo file-changes done in the constructor + if (QueueCounter == 1 && Status == StatIdle && FileSize == 0 && Complete == false) + { + string const Final = _config->FindDir("Dir::State::lists") + URItoFileName(RealURI); + string const LastGoodSig = DestFile + ".reverify"; + if (RealFileExists(Final) == false && RealFileExists(LastGoodSig) == true) + Rename(LastGoodSig, Final); + } +} + /*}}}*/ // pkgAcqMetaClearSig::Custom600Headers - Insert custom request headers /*{{{*/ // --------------------------------------------------------------------- // FIXME: this can go away once the InRelease file is used widely diff --git a/apt-pkg/acquire-item.h b/apt-pkg/acquire-item.h index 51d539450..10c855e63 100644 --- a/apt-pkg/acquire-item.h +++ b/apt-pkg/acquire-item.h @@ -774,6 +774,7 @@ class pkgAcqMetaSig : public pkgAcquire::Item std::string MetaIndexURI, std::string MetaIndexURIDesc, std::string MetaIndexShortDesc, const std::vector* IndexTargets, indexRecords* MetaIndexParser); + virtual ~pkgAcqMetaSig(); }; /*}}}*/ /** \brief An item that is responsible for downloading the meta-index {{{ @@ -904,6 +905,7 @@ public: std::string const &MetaSigURI, std::string const &MetaSigURIDesc, std::string const &MetaSigShortDesc, const std::vector* IndexTargets, indexRecords* MetaIndexParser); + virtual ~pkgAcqMetaClearSig(); }; /*}}}*/ /** \brief An item that is responsible for fetching a package file. {{{ diff --git a/test/integration/test-bug-602412-dequote-redirect b/test/integration/test-bug-602412-dequote-redirect index 764e13667..bcebb57b8 100755 --- a/test/integration/test-bug-602412-dequote-redirect +++ b/test/integration/test-bug-602412-dequote-redirect @@ -26,4 +26,4 @@ Hit http://localhost:8080 unstable/main Translation-en Reading package lists...' aptget update #-o debug::pkgacquire=1 -o debug::pkgacquire::worker=1 msgtest 'Test redirection works in' 'package download' -testsuccess --nomsg aptget install unrelated --download-only +testsuccess --nomsg aptget install unrelated --download-only -y diff --git a/test/integration/test-bug-719263-print-uris-removes-authentication b/test/integration/test-bug-719263-print-uris-removes-authentication new file mode 100755 index 000000000..1c1a27ceb --- /dev/null +++ b/test/integration/test-bug-719263-print-uris-removes-authentication @@ -0,0 +1,41 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'amd64' + +insertinstalledpackage 'unrelated' 'all' '1' +buildsimplenativepackage 'unrelated' 'all' '2' 'unstable' + +setupaptarchive + +testnoact() { + cp -a rootdir/var/lib/dpkg/status rootdir/var/lib/dpkg/status-backup-noact + touch rootdir/var/lib/apt/extended_states + testequal 'Reading package lists... +Building dependency tree... +Reading state information... +The following packages will be upgraded: + unrelated +1 upgraded, 0 newly installed, 0 to remove and 0 not upgraded. +Inst unrelated [1] (2 unstable [all]) +Conf unrelated (2 unstable [all])' aptget install unrelated -s + testsuccess aptget install unrelated -y + testdpkginstalled unrelated + cp -a rootdir/var/lib/dpkg/status-backup-noact rootdir/var/lib/dpkg/status +} + +testnoact +testsuccess aptget update --print-uris +testnoact + +# same thing, just not with InRelease this time +rm -rf rootdir/var/lib/apt/lists +testsuccess aptget update -o Acquire::TryInRelease=0 + +testnoact +testsuccess aptget update --print-uris -o Acquire::TryInRelease=0 +testnoact -- cgit v1.2.3-70-g09d2 From 11b126f9d229c85bfc7a6092527af46f7314f188 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 13 Jul 2013 23:05:55 +0200 Subject: do not try to chown if not run as root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If this code is run as non-root we are in a special situation (e.g. in our testcases) where it is obvious that we can't enforce user/group on any file, so skip this code altogether instead of bugging users with an error message – which we also switch to a warning as a failure to open the file is "just" a warning, so the 'wrong' owner shouldn't be that much of an issue. The file is still handled with chmod, so all the security we can enforce is still enforced of course, which also gets a warning if it fails. Git-Dch: Ignore --- apt-pkg/deb/dpkgpm.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index 34ae4e593..5b2a8251e 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -751,14 +751,15 @@ bool pkgDPkgPM::OpenLog() return _error->WarningE("OpenLog", _("Could not open file '%s'"), logfile_name.c_str()); setvbuf(d->term_out, NULL, _IONBF, 0); SetCloseExec(fileno(d->term_out), true); - struct passwd *pw; - struct group *gr; - pw = getpwnam("root"); - gr = getgrnam("adm"); - if (pw != NULL && gr != NULL) - if(chown(logfile_name.c_str(), pw->pw_uid, gr->gr_gid) != 0) - _error->Errno("OpenLog", "chown failed"); - chmod(logfile_name.c_str(), 0640); + if (getuid() == 0) // if we aren't root, we can't chown a file, so don't try it + { + struct passwd *pw = getpwnam("root"); + struct group *gr = getgrnam("adm"); + if (pw != NULL && gr != NULL && chown(logfile_name.c_str(), pw->pw_uid, gr->gr_gid) != 0) + _error->WarningE("OpenLog", "chown to root:adm of file %s failed", logfile_name.c_str()); + } + if (chmod(logfile_name.c_str(), 0640) != 0) + _error->WarningE("OpenLog", "chmod 0640 of file %s failed", logfile_name.c_str()); fprintf(d->term_out, "\nLog started: %s\n", timestr); } -- cgit v1.2.3-70-g09d2 From 87281603a1342a7dc4d12f979574704a45bf6b7b Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 13 Jul 2013 23:14:41 +0200 Subject: use our _error stack to generate openpty errors While we don't want these error messages on our usual stack, we can use our usual infrastructure to generate an error message with all the usual bells like errno and strerror attached. Git-Dch: Ignore --- apt-pkg/deb/dpkgpm.cc | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index 5b2a8251e..959d06455 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -1238,16 +1238,13 @@ bool pkgDPkgPM::Go(int OutStatusFd) // if tcgetattr does not return zero there was a error // and we do not do any pty magic - if (tcgetattr(0, &tt) == 0) + _error->PushToStack(); + if (tcgetattr(STDOUT_FILENO, &tt) == 0) { ioctl(0, TIOCGWINSZ, (char *)&win); - if (openpty(&master, &slave, NULL, &tt, &win) < 0) + if (openpty(&master, &slave, NULL, &tt, &win) < 0) { - const char *s = _("Can not write log, openpty() " - "failed (/dev/pts not mounted?)\n"); - fprintf(stderr, "%s",s); - if(d->term_out) - fprintf(d->term_out, "%s",s); + _error->Errno("openpty", _("Can not write log (%s)"), _("Is /dev/pts mounted?")); master = slave = -1; } else { struct termios rtt; @@ -1265,6 +1262,15 @@ bool pkgDPkgPM::Go(int OutStatusFd) sigprocmask(SIG_SETMASK, &original_sigmask, 0); } } + // complain only if stdout is either a terminal (but still failed) or is an invalid + // descriptor otherwise we would complain about redirection to e.g. /dev/null as well. + else if (isatty(STDOUT_FILENO) == 1 || errno == EBADF) + _error->Errno("tcgetattr", _("Can not write log (%s)"), _("Is stdout a terminal?")); + + if (_error->PendingError() == true) + _error->DumpErrors(std::cerr); + _error->RevertToStack(); + // Fork dpkg pid_t Child; _config->Set("APT::Keep-Fds::",fd[1]); -- cgit v1.2.3-70-g09d2