[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#686220: libapt-pkg4.12: enable 'linux-*' to return all matches in other pci constructors



Package: libapt-pkg4.12
Version: 0.9.7.4
Severity: wishlist
Tags: patch

Dear Maintainer,

pci::FromGroup constructor has a reusable inner loop which could
provide wildcard matching for other constructors also.  This would
give more consistency between the various constructors also, which at
the moment ':linux-*' in FromRegex and FromTask is different to
FromGroup: only one package is selected by the first two.

Also, it is nice to be able to use this in other frontends, such as
with a hypothetical FromPattern constructor in aptitude, without
having to copy-paste the code across.  See #685731 which this would be
ideal for.

Attached is two patches, the first splits the group,arch matching loop
out of FromGroup, in to a helper function.  The second adjusts
FromRegex to demonstrate the use.

Note that I have left all status messages and error handling as
before, so that the new FromGroup helper does not produce any output
by itself.

Regards

-- System Information:
Debian Release: wheezy/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)
Foreign Architectures: amd64

Kernel: Linux 2.6.32-5-686-bigmem (SMP w/1 CPU core)
Locale: LANG=en_AU.utf8, LC_CTYPE=en_AU.utf8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages libapt-pkg4.12 depends on:
ii  libbz2-1.0         1.0.6-1
ii  libc6              2.13-34
ii  libgcc1            1:4.7.1-2
ii  libstdc++6         4.7.1-2
ii  multiarch-support  2.13-26
ii  zlib1g             1:1.2.7.dfsg-13

libapt-pkg4.12 recommends no packages.

libapt-pkg4.12 suggests no packages.

-- no debconf information
=== modified file 'apt-pkg/cacheset.cc'
--- apt-pkg/cacheset.cc	2012-07-18 09:46:36 +0000
+++ apt-pkg/cacheset.cc	2012-08-30 02:55:33 +0000
@@ -182,6 +182,41 @@
 	return Pkg;
 }
 									/*}}}*/
+// FromGroup - packages from group based on arch			/*{{{*/
+bool PackageContainerInterface::FromGroup(PackageContainerInterface * const pci,
+					  pkgCacheFile &Cache,
+					  pkgCache::GrpIterator const &Grp,
+					  std::string arch,
+					  CacheSetHelper &helper) {
+	if (arch.empty() == true) {
+		pkgCache::PkgIterator Pkg = Grp.FindPreferredPkg();
+		if (Pkg.end() == false) {
+			pci->insert(Pkg);
+			return true;
+		}
+	} else {
+		if (arch == "all" || arch == "native")
+			arch = _config->Find("APT::Architecture");
+
+		bool found = false;
+		// for 'linux-any' return the first package matching, for 'linux-*' return all matches
+		bool const isGlobal = arch.find('*') != std::string::npos;
+		APT::CacheFilter::PackageArchitectureMatchesSpecification pams(arch);
+		for (pkgCache::PkgIterator Pkg = Grp.PackageList(); Pkg.end() == false; Pkg = Grp.NextPkg(Pkg)) {
+			if (pams(Pkg) == false)
+				continue;
+			pci->insert(Pkg);
+			found = true;
+			if (isGlobal == false)
+				break;
+		}
+		if (found == true)
+			return true;
+	}
+
+	return false;
+}
+									/*}}}*/
 // FromGroup - Returns the package defined  by this string		/*{{{*/
 bool PackageContainerInterface::FromGroup(PackageContainerInterface * const pci, pkgCacheFile &Cache,
 			std::string pkg, CacheSetHelper &helper) {
@@ -193,36 +228,12 @@
 	if (archfound != std::string::npos) {
 		arch = pkg.substr(archfound+1);
 		pkg.erase(archfound);
-		if (arch == "all" || arch == "native")
-			arch = _config->Find("APT::Architecture");
 	}
 
 	pkgCache::GrpIterator Grp = Cache.GetPkgCache()->FindGrp(pkg);
-	if (Grp.end() == false) {
-		if (arch.empty() == true) {
-			pkgCache::PkgIterator Pkg = Grp.FindPreferredPkg();
-			if (Pkg.end() == false)
-			{
-			   pci->insert(Pkg);
-			   return true;
-			}
-		} else {
-			bool found = false;
-			// for 'linux-any' return the first package matching, for 'linux-*' return all matches
-			bool const isGlobal = arch.find('*') != std::string::npos;
-			APT::CacheFilter::PackageArchitectureMatchesSpecification pams(arch);
-			for (pkgCache::PkgIterator Pkg = Grp.PackageList(); Pkg.end() == false; Pkg = Grp.NextPkg(Pkg)) {
-				if (pams(Pkg) == false)
-					continue;
-				pci->insert(Pkg);
-				found = true;
-				if (isGlobal == false)
-					break;
-			}
-			if (found == true)
-				return true;
-		}
-	}
+	if (Grp.end() == false &&
+		 FromGroup(pci, Cache, Grp, arch, helper) == true)
+		return true;
 
 	pkgCache::PkgIterator Pkg = helper.canNotFindPkgName(Cache, pkg);
 	if (Pkg.end() == true)

=== modified file 'apt-pkg/cacheset.h'
--- apt-pkg/cacheset.h	2012-06-14 17:03:37 +0000
+++ apt-pkg/cacheset.h	2012-08-30 02:55:33 +0000
@@ -139,6 +139,7 @@
 	static bool FromTask(PackageContainerInterface * const pci, pkgCacheFile &Cache, std::string pattern, CacheSetHelper &helper);
 	static bool FromRegEx(PackageContainerInterface * const pci, pkgCacheFile &Cache, std::string pattern, CacheSetHelper &helper);
 	static pkgCache::PkgIterator FromName(pkgCacheFile &Cache, std::string const &pattern, CacheSetHelper &helper);
+	static bool FromGroup(PackageContainerInterface * const pci, pkgCacheFile &Cache, pkgCache::GrpIterator const &Grp, std::string arch, CacheSetHelper &helper);
 	static bool FromGroup(PackageContainerInterface * const pci, pkgCacheFile &Cache, std::string pattern, CacheSetHelper &helper);
 	static bool FromString(PackageContainerInterface * const pci, pkgCacheFile &Cache, std::string const &pattern, CacheSetHelper &helper);
 	static bool FromCommandLine(PackageContainerInterface * const pci, pkgCacheFile &Cache, const char **cmdline, CacheSetHelper &helper);

=== modified file 'apt-pkg/cacheset.cc'
--- apt-pkg/cacheset.cc	2012-08-30 02:55:33 +0000
+++ apt-pkg/cacheset.cc	2012-08-30 04:38:56 +0000
@@ -99,7 +99,22 @@
 									/*}}}*/
 // FromRegEx - Return all packages in the cache matching a pattern	/*{{{*/
 bool PackageContainerInterface::FromRegEx(PackageContainerInterface * const pci, pkgCacheFile &Cache, std::string pattern, CacheSetHelper &helper) {
-	static const char * const isregex = ".?+*|[^$";
+	// FromGroup uses '*' in arch as a wildcard, so this is not enough
+	// by itself to indicate that text after ':' is part of the regex;
+	// keep it first in isregex to make this easy later on.
+	static const char * const isregex = "*.?+|[^$";
+
+	size_t archfound = pattern.find_last_of(':');
+	std::string arch;
+	if (archfound != std::string::npos) {
+		arch = pattern.substr(archfound+1);
+		// isregex+1 to ignore any '*' in arch
+		if (arch.find_first_of(isregex+1) == std::string::npos)
+			pattern.erase(archfound);
+		else
+			arch.clear();
+	}
+
 	if (pattern.find_first_of(isregex) == std::string::npos)
 		return false;
 
@@ -107,48 +122,30 @@
 	if (wasEmpty == true)
 		pci->setConstructor(REGEX);
 
-	size_t archfound = pattern.find_last_of(':');
-	std::string arch = "native";
-	if (archfound != std::string::npos) {
-		arch = pattern.substr(archfound+1);
-		if (arch.find_first_of(isregex) == std::string::npos)
-			pattern.erase(archfound);
-		else
-			arch = "native";
-	}
-
 	if (unlikely(Cache.GetPkgCache() == 0))
 		return false;
 
 	APT::CacheFilter::PackageNameMatchesRegEx regexfilter(pattern);
 
-	bool found = false;
+	PackageSet pkgset;
 	for (pkgCache::GrpIterator Grp = Cache.GetPkgCache()->GrpBegin(); Grp.end() == false; ++Grp) {
 		if (regexfilter(Grp) == false)
 			continue;
-		pkgCache::PkgIterator Pkg = Grp.FindPkg(arch);
-		if (Pkg.end() == true) {
-			if (archfound == std::string::npos) {
-				std::vector<std::string> archs = APT::Configuration::getArchitectures();
-				for (std::vector<std::string>::const_iterator a = archs.begin();
-				     a != archs.end() && Pkg.end() != true; ++a)
-					Pkg = Grp.FindPkg(*a);
-			}
-			if (Pkg.end() == true)
-				continue;
-		}
-
-		pci->insert(Pkg);
-		helper.showRegExSelection(Pkg, pattern);
-		found = true;
+		FromGroup(&pkgset, Cache, Grp, arch, helper);
 	}
 
-	if (found == false) {
+	if (pkgset.empty() == true) {
 		helper.canNotFindRegEx(pci, Cache, pattern);
 		pci->setConstructor(UNKNOWN);
 		return false;
 	}
 
+	for (PackageSet::const_iterator P = pkgset.begin();
+		  P != pkgset.end(); ++P) {
+		pci->insert(P);
+		helper.showRegExSelection(P, pattern);
+	}
+
 	if (wasEmpty == false && pci->getConstructor() != UNKNOWN)
 		pci->setConstructor(UNKNOWN);
 


Reply to: