Re: Bug#748936: apt doesnt understand architecture wildcards
On Mon, Feb 01, 2016 at 12:05:18AM +0100, David Kalnischkies wrote:
> On Wed, Jan 20, 2016 at 01:39:45PM +0000, Colin Watson wrote:
> > On Wed, Jan 20, 2016 at 12:31:52PM +0100, Balint Reczey wrote:
> > > On 06/04/2014 03:41 AM, Guillem Jover wrote:
> > > > * Other programs could “easily” use dpkg-architecture to check for
> > > > identity or a match. (This poses problems for programs that do not
> > >
> > > I think making apt call dpkg-architecture for matching would be a good
> > > way of ensuring consistency with dpkg. Caching the results in a hash
> > > table would make matching even faster than it is currently.
> >
> > dpkg-architecture is in dpkg-dev, so not reliably usable at run-time.
>
> Also, apt is trying to remain largely independent of the low-level
> package manager, so bluntly depending on it wouldn't be good, but …
>
> … apt could survive this (for now) as these architecture specifications
> can at the moment only be encountered in
> a) build-dependencies of source packages
> (effecting via python-apt also presumably stuff like dak)
> b) the commandline like 'apt install libfoo:linux-* foo:linux-any'
> (and aptitude uses it, too, for this)
>
> So, we could do that without too much pain, while keeping a fallback
> around for cases in which we don't have dpkg-architecture around for
> whatever reason as it doesn't effect apts primary operation (but might
> effect the primary operation of other tools which would need to be
> tested first).
>
>
> I wonder through if we aren't creating the debian version of a tar bomb
> (xkcd#1168) and to illustrate that a little quiz:
>
> dpkg-architecture -ai386 -ii386 true
> dpkg-architecture -ai386 -ilinux-i386 true
> dpkg-architecture -ai386 -iany-i386 true
> dpkg-architecture -aarmhf -iarmhf true
> dpkg-architecture -aarmhf -ilinux-armhf true
> dpkg-architecture -aarmhf -iany-armhf false
> dpkg-architecture -aarmhf -iarm false
> dpkg-architecture -aarmhf -ilinux-arm false
> dpkg-architecture -aarmhf -iany-arm true
>
> Now, given we want to go to <libc>-<kernel>-<cpu> lets see:
> dpkg-architecture -aarmhf -iany-linux-arm true
> dpkg-architecture -aarmhf -iany-any-arm true
> dpkg-architecture -aarmhf -ignu-any-arm false
> dpkg-architecture -aarmhf -ignueabihf-any-arm true
>
> And to cool off, think about what matches any-arm, linux-any, and if
> gnu-any is even supported and if what that matches…
>
>
> Truth be told, I would have died on 'any-armhf' already even through
> that is "obvious" as 'linux-armhf' is interpreted as a literal
> architecture, while 'any-armhf' is a pattern (just that neither dpkg nor
> the policy highlight that such a difference exists explicitly).
>
> Anyway, I somehow doubt it will be a good idea to trouble mere mortals
> with the difference between gnu and gnueabihf for matching proposes, so
> I wonder why we have to trouble them with the difference of armhf and
> arm depending on if that specification is a literal architecture or an
> architecture pattern – especially if the two are different only for some
> architectures… I would personally be more happy with any-armhf working
> (and a special case letting arm match all of the arms).
>
>
> So, I actually like how apt behaves currently as it just makes more
> sense in my head to expand armhf to gnu-linux-armhf and match it against
> gnu-any-armhf instead of gnueabihf-linux-arm and and gnueabihf-any-arm,
> but so be it – it tends to be more important to have a consistent answer
> in Debian than to keep me sane… (yeah, I know, that triplet makes
> perfect sense if you know history, Debian and arm – I just don't).
>
>
> I am therefore going to happily accept any patch flying my way
> implementing architecture wildcards differently if need be, but I am not
> going to do it myself mainly because I expect that to have fallout – not
> in apt, but in things using apt – and I don't have the energy (or the
> rights) to deal with such things efficiently.
Hi David,
I actually ran into this bug in the real world: elfutils has a
Build-Depends: gcc-multilib [any-amd64], which includes x32
(x32-gnu-linux-amd64), but apt build-dep didn't install it, so
dpkg-checkbuilddeps errored out when building. I agree some of this
seems crazy, but it's even more crazy to have apt and dpkg disagree IMO.
I have attached an initial proof-of-concept[0] patch for apt to embed the
list of architecture tuples at build-time. It's not especially pretty,
but it passes the now-modified test suite run during the build
(integration tests not yet run), which includes testing x32 and arm
behave like they do in dpkg. I also haven't actually tested that
"apt build-dep elfutils" on x32 installs gcc-multilib, but the testsuite
success makes me pretty confident. Thoughts?
Regards,
James
[0] Hence no patch tag for the bug
>From 706fa14b9d115a8b0cd52a39caa93dc13e422349 Mon Sep 17 00:00:00 2001
From: James Clarke <jrtc27@jrtc27.com>
Date: Sun, 8 Jan 2017 01:18:09 +0000
Subject: [PATCH] Embed real dpkg architecture tuples
Some of the cachefilter tests were actually wrong; dpkg does not
recognise architectures like (base-)gnu-linux-amd64; while there is a
base-gnu-linux-amd64 tuple, it is not an architecture name. However,
gnu-linux-any is a valid pattern, since wildcard patterns match on
tuples, not architectures.
Closes: #748936
---
apt-pkg/CMakeLists.txt | 16 +++++++-
apt-pkg/cachefilter.cc | 81 +++++++++++++++++++++++++++--------------
apt-pkg/contrib/strutl.cc | 12 ++++++
apt-pkg/contrib/strutl.h | 2 +
debian/control | 2 +
debian/rules | 9 ++++-
generate-tuple-table | 29 +++++++++++++++
test/libapt/cachefilter_test.cc | 67 +++++++++++++++++++++++-----------
8 files changed, 165 insertions(+), 53 deletions(-)
diff --git a/apt-pkg/CMakeLists.txt b/apt-pkg/CMakeLists.txt
index 25ed13ec3..31d758001 100644
--- a/apt-pkg/CMakeLists.txt
+++ b/apt-pkg/CMakeLists.txt
@@ -12,9 +12,19 @@ execute_process(COMMAND ${PROJECT_SOURCE_DIR}/triehash/triehash.pl
--enum-name pkgTagSection::Key
--function-name pkgTagHash
--include "<apt-pkg/tagfile.h>"
- ${CMAKE_CURRENT_SOURCE_DIR}/tagfile-keys.list)
+ ${CMAKE_CURRENT_SOURCE_DIR}/tagfile-keys.list
+ RESULT_VARIABLE retcode)
+if(NOT ${retcode} EQUAL 0)
+ message(FATAL_ERROR "Error creating tagfile-keys.cc/h: exit code ${retcode}")
+endif()
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "tagfile-keys.list")
+execute_process(COMMAND ${PROJECT_SOURCE_DIR}/generate-tuple-table
+ ${CMAKE_CURRENT_BINARY_DIR}/tuple-table.cc
+ RESULT_VARIABLE retcode)
+if(NOT ${retcode} EQUAL 0)
+ message(FATAL_ERROR "Error creating tuple-table.cc: exit code ${retcode}")
+endif()
# Set the version of the library
execute_process(COMMAND awk -v ORS=. "/^\#define APT_PKG_M/ {print \$3}"
@@ -30,7 +40,9 @@ message(STATUS "Building libapt-pkg ${MAJOR} (release ${MINOR})")
set(APT_PKG_MAJOR ${MAJOR} PARENT_SCOPE) # exporting for methods/CMakeLists.txt
# Definition of the C++ files used to build the library
-file(GLOB_RECURSE library "*.cc" "${CMAKE_CURRENT_BINARY_DIR}/tagfile-keys.cc")
+file(GLOB_RECURSE library "*.cc"
+ "${CMAKE_CURRENT_BINARY_DIR}/tagfile-keys.cc"
+ "${CMAKE_CURRENT_BINARY_DIR}/tuple-table.cc")
file(GLOB_RECURSE headers "*.h")
# Create a library using the C++ files
diff --git a/apt-pkg/cachefilter.cc b/apt-pkg/cachefilter.cc
index b1adf5de3..fd5207849 100644
--- a/apt-pkg/cachefilter.cc
+++ b/apt-pkg/cachefilter.cc
@@ -14,7 +14,9 @@
#include <apt-pkg/strutl.h>
#include <apt-pkg/macros.h>
+#include <algorithm>
#include <string>
+#include <unordered_map>
#include <string.h>
#include <regex.h>
#include <fnmatch.h>
@@ -22,6 +24,9 @@
#include <apti18n.h>
/*}}}*/
namespace APT {
+
+extern std::unordered_map<std::string, std::vector<std::string>> const ArchToTupleMap;
+
namespace CacheFilter {
APT_CONST Matcher::~Matcher() {}
APT_CONST PackageMatcher::~PackageMatcher() {}
@@ -68,38 +73,58 @@ bool PackageNameMatchesFnmatch::operator() (pkgCache::GrpIterator const &Grp) {
return fnmatch(Pattern.c_str(), Grp.Name(), FNM_CASEFOLD) == 0;
}
/*}}}*/
-// Architecture matches <libc>-<kernel>-<cpu> specification /*{{{*/
+// Architecture matches <abi>-<libc>-<kernel>-<cpu> specification /*{{{*/
//----------------------------------------------------------------------
-/* The complete architecture, consisting of <libc>-<kernel>-<cpu>. */
+
+static std::vector<std::string> ArchToTuple(std::string arch) {
+ // Strip leading linux- from arch if present
+ // dpkg says this may disappear in the future
+ if (APT::String::Startswith(arch, std::string("linux-")))
+ arch = arch.substr(6);
+
+ auto it = ArchToTupleMap.find(arch);
+ if (it != ArchToTupleMap.end())
+ return it->second;
+ else
+ return {};
+}
+
+static std::vector<std::string> PatternToTuple(std::string const &arch) {
+ std::vector<std::string> tuple = VectorizeString(arch, '-');
+ if (std::find(tuple.begin(), tuple.end(), std::string("any")) != tuple.end()) {
+ while (tuple.size() < 4) {
+ tuple.emplace(tuple.begin(), "any");
+ }
+ return tuple;
+ } else
+ return ArchToTuple(arch);
+}
+
+/* The complete architecture, consisting of <abi>-<libc>-<kernel>-<cpu>. */
static std::string CompleteArch(std::string const &arch, bool const isPattern) {
- auto const found = arch.find('-');
- if (found != std::string::npos)
- {
- // ensure that only -any- is replaced and not something like company-
- std::string complete = std::string("-").append(arch).append("-");
- size_t pos = 0;
- char const * const search = "-any-";
- auto const search_len = strlen(search) - 2;
- while((pos = complete.find(search, pos)) != std::string::npos) {
- complete.replace(pos + 1, search_len, "*");
- pos += 2;
+ auto tuple = isPattern ? PatternToTuple(arch) : ArchToTuple(arch);
+
+ if (tuple.empty()) {
+ // Fallback for unknown architectures
+ // Patterns never fail if they contain wildcards, so by this point, arch
+ // has no wildcards.
+ tuple = VectorizeString(arch, '-');
+ switch (tuple.size()) {
+ case 1:
+ tuple.emplace(tuple.begin(), "linux");
+ /* fall through */
+ case 2:
+ tuple.emplace(tuple.begin(), "gnu");
+ /* fall through */
+ case 3:
+ tuple.emplace(tuple.begin(), "base");
+ /* fall through */
+ break;
}
- complete = complete.substr(1, complete.size()-2);
- if (arch.find('-', found+1) != std::string::npos)
- // <libc>-<kernel>-<cpu> format
- return complete;
- // <kernel>-<cpu> format
- else if (isPattern)
- return "*-" + complete;
- else
- return "gnu-" + complete;
}
- else if (arch == "any")
- return "*-*-*";
- else if (isPattern)
- return "*-linux-" + arch;
- else
- return "gnu-linux-" + arch;
+
+ std::replace(tuple.begin(), tuple.end(), std::string("any"), std::string("*"));
+ return StringJoin(tuple, "-");
}
PackageArchitectureMatchesSpecification::PackageArchitectureMatchesSpecification(std::string const &pattern, bool const pisPattern) :
literal(pattern), complete(CompleteArch(pattern, pisPattern)), isPattern(pisPattern) {
diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc
index cf8feb970..9216bab15 100644
--- a/apt-pkg/contrib/strutl.cc
+++ b/apt-pkg/contrib/strutl.cc
@@ -27,6 +27,7 @@
#include <locale>
#include <sstream>
#include <string>
+#include <sstream>
#include <vector>
#include <stddef.h>
@@ -1319,6 +1320,17 @@ vector<string> StringSplit(std::string const &s, std::string const &sep,
}
return split;
}
+
+std::string StringJoin(std::vector<std::string> list, const std::string &sep)
+{
+ std::ostringstream oss;
+ for (auto it = list.begin(); it != list.end(); it++)
+ {
+ if (it != list.begin()) oss << sep;
+ oss << *it;
+ }
+ return oss.str();
+}
/*}}}*/
// RegexChoice - Simple regex list/list matcher /*{{{*/
// ---------------------------------------------------------------------
diff --git a/apt-pkg/contrib/strutl.h b/apt-pkg/contrib/strutl.h
index 918ac89c7..b23ce914b 100644
--- a/apt-pkg/contrib/strutl.h
+++ b/apt-pkg/contrib/strutl.h
@@ -131,6 +131,8 @@ std::vector<std::string> StringSplit(std::string const &input,
std::string const &sep,
unsigned int maxsplit=std::numeric_limits<unsigned int>::max()) APT_CONST;
+std::string StringJoin(std::vector<std::string> list, const std::string &sep);
+
void ioprintf(std::ostream &out,const char *format,...) APT_PRINTF(2);
void strprintf(std::string &out,const char *format,...) APT_PRINTF(2);
char *safe_snprintf(char *Buffer,char *End,const char *Format,...) APT_PRINTF(3);
diff --git a/debian/control b/debian/control
index 96bbef348..b2af95dda 100644
--- a/debian/control
+++ b/debian/control
@@ -12,6 +12,7 @@ Build-Depends: cmake (>= 3.4),
docbook-xml,
docbook-xsl,
dpkg-dev (>= 1.17.14),
+ libdpkg-perl (>= 1.18.11),
gettext (>= 0.12),
libbz2-dev,
libcurl4-gnutls-dev (>= 7.19.4~),
@@ -67,6 +68,7 @@ Breaks: appstream (<< 0.9.0-3~), apt (<< 1.1~exp14), libapt-inst1.5 (<< 0.9.9~)
Recommends: apt (>= ${binary:Version})
Section: libs
Provides: libapt-pkg (= ${binary:Version})
+Built-Using: ${sourcedep:libdpkg-perl}
Description: package management runtime library
This library provides the common functionality for searching and
managing packages as well as information about packages.
diff --git a/debian/rules b/debian/rules
index 62d913f0a..b0e03025e 100755
--- a/debian/rules
+++ b/debian/rules
@@ -10,6 +10,11 @@ export DPKG_GENSYMBOLS_CHECK_LEVEL=0
export CTEST_OUTPUT_ON_FAILURE=1
+sourcedep_libdpkg_perl := \
+ $(shell dpkg-query \
+ --showformat '$${source:Package} (= $${source:Version})' \
+ --show libdpkg-perl)
+
%:
dh $@
@@ -24,7 +29,9 @@ override_dh_install-arch:
install -m 755 debian/apt.auto-removal.sh debian/apt/etc/kernel/postinst.d/apt-auto-removal
override_dh_gencontrol:
- dh_gencontrol -- -Vapt:keyring="$(shell ./vendor/getinfo keyring-package)"
+ dh_gencontrol -- \
+ -Vapt:keyring="$(shell ./vendor/getinfo keyring-package)" \
+ -Vsourcedep:libdpkg-perl='$(sourcedep_libdpkg_perl)'
override_dh_installcron:
dh_installcron --name=apt-compat
diff --git a/generate-tuple-table b/generate-tuple-table
new file mode 100755
index 000000000..978c6ab67
--- /dev/null
+++ b/generate-tuple-table
@@ -0,0 +1,29 @@
+#!/usr/bin/perl -w
+
+use strict;
+use warnings;
+
+use Dpkg::Arch qw(get_valid_arches debarch_to_debtuple);
+
+my $filename = shift or die "Usage: $0 <output>";
+
+my $output;
+open($output, '>', $filename) or die "Cannot open $filename: $!";
+
+print $output "#include <string>\n";
+print $output "#include <unordered_map>\n";
+print $output "#include <vector>\n";
+print $output "\n";
+print $output "#include \"macros.h\"";
+print $output "\n";
+print $output "namespace APT {\n";
+print $output " APT_HIDDEN extern std::unordered_map<std::string, std::vector<std::string>> const ArchToTupleMap = {\n";
+my @arches = get_valid_arches();
+foreach my $arch (@arches) {
+ my ($abi, $libc, $os, $cpu) = debarch_to_debtuple($arch);
+ print $output " { \"$arch\", { \"$abi\", \"$libc\", \"$os\", \"$cpu\" } },\n";
+}
+print $output " };\n";
+print $output "}\n";
+
+close($output);
diff --git a/test/libapt/cachefilter_test.cc b/test/libapt/cachefilter_test.cc
index 28924b758..36228c13d 100644
--- a/test/libapt/cachefilter_test.cc
+++ b/test/libapt/cachefilter_test.cc
@@ -9,17 +9,14 @@
TEST(CacheFilterTest, ArchitectureSpecification)
{
{
- SCOPED_TRACE("Pattern is any-armhf");
- APT::CacheFilter::PackageArchitectureMatchesSpecification ams("any-armhf");
- EXPECT_TRUE(ams("armhf"));
- EXPECT_FALSE(ams("armel"));
- EXPECT_TRUE(ams("linux-armhf"));
- EXPECT_FALSE(ams("linux-armel"));
- EXPECT_TRUE(ams("kfreebsd-armhf"));
- EXPECT_TRUE(ams("gnu-linux-armhf"));
- EXPECT_FALSE(ams("gnu-linux-armel"));
- EXPECT_TRUE(ams("gnu-kfreebsd-armhf"));
- EXPECT_TRUE(ams("musl-linux-armhf"));
+ SCOPED_TRACE("Pattern is any-i386");
+ APT::CacheFilter::PackageArchitectureMatchesSpecification ams("any-i386");
+ EXPECT_TRUE(ams("i386"));
+ EXPECT_FALSE(ams("amd64"));
+ EXPECT_TRUE(ams("linux-i386"));
+ EXPECT_FALSE(ams("linux-amd64"));
+ EXPECT_TRUE(ams("kfreebsd-i386"));
+ EXPECT_TRUE(ams("musl-linux-i386"));
}
{
SCOPED_TRACE("Pattern is linux-any");
@@ -29,9 +26,6 @@ TEST(CacheFilterTest, ArchitectureSpecification)
EXPECT_TRUE(ams("linux-armhf"));
EXPECT_TRUE(ams("linux-armel"));
EXPECT_FALSE(ams("kfreebsd-armhf"));
- EXPECT_TRUE(ams("gnu-linux-armhf"));
- EXPECT_TRUE(ams("gnu-linux-armel"));
- EXPECT_FALSE(ams("gnu-kfreebsd-armhf"));
EXPECT_TRUE(ams("musl-linux-armhf"));
}
{
@@ -42,11 +36,11 @@ TEST(CacheFilterTest, ArchitectureSpecification)
EXPECT_TRUE(ams("linux-armhf"));
EXPECT_TRUE(ams("linux-armel"));
EXPECT_TRUE(ams("kfreebsd-armhf"));
- EXPECT_TRUE(ams("gnu-linux-armhf"));
- EXPECT_TRUE(ams("gnu-linux-armel"));
- EXPECT_TRUE(ams("gnu-kfreebsd-armhf"));
EXPECT_FALSE(ams("musl-linux-armhf"));
}
+ // Weird ones - armhf's tuple is actually eabihf-gnu-linux-arm
+ // armel's tuple is actually eabi-gnu-linux-arm
+ // x32's tuple is actually x32-gnu-linux-amd64
{
SCOPED_TRACE("Architecture is armhf");
APT::CacheFilter::PackageArchitectureMatchesSpecification ams("armhf", false);
@@ -54,13 +48,42 @@ TEST(CacheFilterTest, ArchitectureSpecification)
EXPECT_FALSE(ams("armel"));
EXPECT_TRUE(ams("linux-any"));
EXPECT_FALSE(ams("kfreebsd-any"));
- EXPECT_TRUE(ams("any-armhf"));
- EXPECT_FALSE(ams("any-armel"));
+ EXPECT_TRUE(ams("any-arm"));
EXPECT_TRUE(ams("linux-armhf"));
EXPECT_FALSE(ams("kfreebsd-armhf"));
- EXPECT_TRUE(ams("gnu-linux-armhf"));
- EXPECT_FALSE(ams("gnu-linux-armel"));
- EXPECT_FALSE(ams("gnu-kfreebsd-armhf"));
EXPECT_FALSE(ams("musl-linux-armhf"));
}
+ {
+ SCOPED_TRACE("Pattern is any-arm");
+ APT::CacheFilter::PackageArchitectureMatchesSpecification ams("any-arm");
+ EXPECT_TRUE(ams("armhf"));
+ EXPECT_TRUE(ams("armel"));
+ EXPECT_TRUE(ams("linux-armhf"));
+ EXPECT_TRUE(ams("linux-armel"));
+ EXPECT_TRUE(ams("kfreebsd-armhf"));
+ EXPECT_TRUE(ams("musl-linux-armhf"));
+ EXPECT_TRUE(ams("uclibc-linux-armel"));
+
+ EXPECT_FALSE(ams("arm64"));
+ EXPECT_FALSE(ams("linux-arm64"));
+ EXPECT_FALSE(ams("kfreebsd-arm64"));
+ EXPECT_FALSE(ams("musl-linux-arm64"));
+ }
+ {
+ SCOPED_TRACE("Pattern is any-amd64");
+ APT::CacheFilter::PackageArchitectureMatchesSpecification ams("any-amd64");
+ EXPECT_TRUE(ams("amd64"));
+ EXPECT_TRUE(ams("x32"));
+ EXPECT_TRUE(ams("linux-amd64"));
+ EXPECT_TRUE(ams("linux-x32"));
+ EXPECT_TRUE(ams("kfreebsd-amd64"));
+ EXPECT_TRUE(ams("musl-linux-amd64"));
+ EXPECT_TRUE(ams("uclibc-linux-amd64"));
+
+ EXPECT_FALSE(ams("i386"));
+ EXPECT_FALSE(ams("linux-i386"));
+ EXPECT_FALSE(ams("kfreebsd-i386"));
+ EXPECT_FALSE(ams("musl-linux-i386"));
+ EXPECT_FALSE(ams("uclibc-linux-i386"));
+ }
}
--
2.11.0
Reply to: