Some more refactoring.

TODO

@@ -0,0 +1 @@
+- Need to be able to check up front whether a requirement is satisfied, before attempting to install it (which is more expensive)

ognibuild/__main__.py

@@ -83,8 +83,15 @@ def main():  # noqa: C901
         action="store_true",
         help="Ignore declared dependencies, follow build errors only",
     )
+    parser.add_argument(
+        "--verbose",
+        action="store_true",
+        help="Be verbose")
     args = parser.parse_args()
-    logging.basicConfig(level=logging.INFO)
+    if args.verbose:
+        logging.basicConfig(level=logging.DEBUG)
+    else:
+        logging.basicConfig(level=logging.INFO)
     if args.schroot:
         from .session.schroot import SchrootSession
 

ognibuild/debian/apt.py

@@ -46,18 +46,43 @@ def run_apt(session: Session, args: List[str]) -> None:
     raise UnidentifiedError(retcode, args, lines)
 
 
+class FileSearcher(object):
+    def search_files(self, path: str, regex: bool = False) -> Iterator[str]:
+        raise NotImplementedError(self.search_files)
+
+
 class AptManager(object):
 
     session: Session
+    _searchers: Optional[List[FileSearcher]]
 
     def __init__(self, session):
         self.session = session
+        self._apt_cache = None
+        self._searchers = None
 
-    def package_exists(self, package: str) -> bool:
+    def searchers(self):
-        raise NotImplementedError(self.package_exists)
+        if self._searchers is None:
+            self._searchers = [
+                RemoteAptContentsFileSearcher.from_session(self.session),
+                GENERATED_FILE_SEARCHER]
+        return self._searchers
+
+    def package_exists(self, package):
+        if self._apt_cache is None:
+            import apt_pkg
+
+            # TODO(jelmer): Load from self.session
+            self._apt_cache = apt_pkg.Cache()
+        for p in self._apt_cache.packages:
+            if p.name == package:
+                return True
+        return False
 
     def get_package_for_paths(self, paths, regex=False):
-        raise NotImplementedError(self.get_package_for_paths)
+        logging.debug('Searching for packages containing %r', paths)
+        # TODO(jelmer): Make sure we use whatever is configured in self.session
+        return get_package_for_paths(paths, self.searchers(), regex=regex)
 
     def missing(self, packages):
         root = getattr(self.session, "location", "/")
@@ -84,45 +109,22 @@ class AptManager(object):
         run_apt(self.session, ["satisfy"] + deps)
 
 
-class LocalAptManager(AptManager):
-
-    def __init__(self):
-        from ..session.plain import PlainSession
-        self.session = PlainSession()
-        self._apt_cache = None
-
-    def package_exists(self, package):
-        if self._apt_cache is None:
-            import apt_pkg
-
-            self._apt_cache = apt_pkg.Cache()
-        for p in self._apt_cache.packages:
-            if p.name == package:
-                return True
-        return False
-
-    def get_package_for_paths(self, paths, regex=False):
-        # TODO(jelmer): Make sure we use whatever is configured in self.session
-        return get_package_for_paths(paths, regex=regex)
-
-
-class FileSearcher(object):
-    def search_files(self, path: str, regex: bool = False) -> Iterator[str]:
-        raise NotImplementedError(self.search_files)
-
-
 class ContentsFileNotFound(Exception):
     """The contents file was not found."""
 
 
-class AptContentsFileSearcher(FileSearcher):
+class RemoteAptContentsFileSearcher(FileSearcher):
     def __init__(self):
         self._db = {}
 
     @classmethod
-    def from_env(cls):
+    def from_session(cls, session):
-        sources = os.environ["REPOSITORIES"].split(":")
+        logging.info('Loading apt contents information')
-        return cls.from_repositories(sources)
+        # TODO(jelmer): what about sources.list.d?
+        with open(os.path.join(session.location, 'etc/apt/sources.list'), 'r') as f:
+            return cls.from_repositories(
+                f.readlines(),
+                cache_dir=os.path.join(session.location, 'var/lib/apt/lists'))
 
     def __setitem__(self, path, package):
         self._db[path] = package
@@ -144,36 +146,75 @@ class AptContentsFileSearcher(FileSearcher):
             self[decoded_path] = package.decode("utf-8")
 
     @classmethod
-    def from_urls(cls, urls):
+    def _load_cache_file(cls, url, cache_dir):
+        from urllib.parse import urlparse
+        parsed = urlparse(url)
+        p = os.path.join(
+            cache_dir,
+            parsed.hostname + parsed.path.replace('/', '_') + '.lz4')
+        logging.debug('Loading cached contents file %s', p)
+        if not os.path.exists(p):
+            return None
+        import lz4.frame
+        return lz4.frame.open(p, mode='rb')
+
+    @classmethod
+    def from_urls(cls, urls, cache_dir=None):
         self = cls()
-        for url in urls:
+        for url, mandatory in urls:
-            self.load_url(url)
+            f = cls._load_cache_file(url, cache_dir)
+            if f is not None:
+                self.load_file(f)
+            elif not mandatory and self._db:
+                logging.debug(
+                    'Not attempting to fetch optional contents file %s', url)
+            else:
+                logging.debug('Fetching contents file %s', url)
+                try:
+                    self.load_url(url)
+                except ContentsFileNotFound:
+                    if mandatory:
+                        raise
+                    logging.debug(
+                        'Unable to fetch optional contents file %s', url)
         return self
 
     @classmethod
-    def from_repositories(cls, sources):
+    def from_repositories(cls, sources, cache_dir=None):
-        from .debian.build import get_build_architecture
+        # TODO(jelmer): Use aptsources.sourceslist.SourcesList
+        from .build import get_build_architecture
         # TODO(jelmer): Verify signatures, etc.
         urls = []
-        arches = [get_build_architecture(), "all"]
+        arches = [(get_build_architecture(), True), ("all", False)]
         for source in sources:
+            if not source.strip():
+                continue
+            if source.strip().startswith('#'):
+                continue
             parts = source.split(" ")
+            if parts[0] == "deb-src":
+                continue
             if parts[0] != "deb":
                 logging.warning("Invalid line in sources: %r", source)
                 continue
-            base_url = parts[1]
+            base_url = parts[1].strip().rstrip("/")
-            name = parts[2]
+            name = parts[2].strip()
-            components = parts[3:]
+            components = [c.strip() for c in parts[3:]]
-            response = cls._get("%s/%s/Release" % (base_url, name))
+            if components:
-            r = Release(response)
+                dists_url = base_url + "/dists"
-            desired_files = set()
+            else:
-            for component in components:
+                dists_url = base_url
-                for arch in arches:
+            if components:
-                    desired_files.add("%s/Contents-%s" % (component, arch))
+                for component in components:
-            for entry in r["MD5Sum"]:
+                    for arch, mandatory in arches:
-                if entry["name"] in desired_files:
+                        urls.append(
-                    urls.append("%s/%s/%s" % (base_url, name, entry["name"]))
+                            ("%s/%s/%s/Contents-%s" % (
-        return cls.from_urls(urls)
+                                dists_url, name, component, arch), mandatory))
+            else:
+                for arch, mandatory in arches:
+                    urls.append(
+                        ("%s/%s/Contents-%s" % (dists_url, name.rstrip('/'), arch), mandatory))
+        return cls.from_urls(urls, cache_dir=cache_dir)
 
     @staticmethod
     def _get(url):
@@ -182,19 +223,27 @@ class AptContentsFileSearcher(FileSearcher):
         request = Request(url, headers={"User-Agent": "Debian Janitor"})
         return urlopen(request)
 
-    def load_url(self, url):
+    def load_url(self, url, allow_cache=True):
         from urllib.error import HTTPError
 
-        try:
+        for ext in ['.xz', '.gz', '']:
-            response = self._get(url)
+            try:
-        except HTTPError as e:
+                response = self._get(url + ext)
-            if e.status == 404:
+            except HTTPError as e:
-                raise ContentsFileNotFound(url)
+                if e.status == 404:
-            raise
+                   continue
-        if url.endswith(".gz"):
+                raise
+            break
+        else:
+            raise ContentsFileNotFound(url)
+        if ext == '.gz':
             import gzip
 
             f = gzip.GzipFile(fileobj=response)
+        elif ext == '.xz':
+            import lzma
+            from io import BytesIO
+            f = BytesIO(lzma.decompress(response.read()))
         elif response.headers.get_content_type() == "text/plain":
             f = response
         else:
@@ -228,23 +277,12 @@ GENERATED_FILE_SEARCHER = GeneratedFileSearcher(
 )
 
 
-_apt_file_searcher = None
+def get_package_for_paths(
-
+        paths: List[str], searchers: List[FileSearcher], regex: bool = False) -> Optional[str]:
-
-def search_apt_file(path: str, regex: bool = False) -> Iterator[str]:
-    global _apt_file_searcher
-    if _apt_file_searcher is None:
-        # TODO(jelmer): cache file
-        _apt_file_searcher = AptContentsFileSearcher.from_env()
-    if _apt_file_searcher:
-        yield from _apt_file_searcher.search_files(path, regex=regex)
-    yield from GENERATED_FILE_SEARCHER.search_files(path, regex=regex)
-
-
-def get_package_for_paths(paths: List[str], regex: bool = False) -> Optional[str]:
     candidates: Set[str] = set()
     for path in paths:
-        candidates.update(search_apt_file(path, regex=regex))
+        for searcher in searchers:
+            candidates.update(searcher.search_files(path, regex=regex))
         if candidates:
             break
     if len(candidates) == 0:

ognibuild/debian/fix_build.py

@@ -105,7 +105,6 @@ from buildlog_consultant.sbuild import (
     SbuildFailure,
 )
 
-from .apt import LocalAptManager
 from ..fix_build import BuildFixer, SimpleBuildFixer, resolve_error, DependencyContext
 from ..resolver.apt import (
     NoAptPackage,
@@ -332,7 +331,7 @@ def fix_missing_python_distribution(error, context):  # noqa: C901
     default = not targeted
 
     pypy_pkg = context.apt.get_package_for_paths(
-        ["/usr/lib/pypy/dist-packages/%s-.*.egg-info" % error.distribution], regex=True
+        ["/usr/lib/pypy/dist-packages/%s-.*.egg-info/PKG-INFO" % error.distribution], regex=True
     )
     if pypy_pkg is None:
         pypy_pkg = "pypy-%s" % error.distribution
@@ -340,7 +339,7 @@ def fix_missing_python_distribution(error, context):  # noqa: C901
             pypy_pkg = None
 
     py2_pkg = context.apt.get_package_for_paths(
-        ["/usr/lib/python2\\.[0-9]/dist-packages/%s-.*.egg-info" % error.distribution],
+        ["/usr/lib/python2\\.[0-9]/dist-packages/%s-.*.egg-info/PKG-INFO" % error.distribution],
         regex=True,
     )
     if py2_pkg is None:
@@ -349,7 +348,7 @@ def fix_missing_python_distribution(error, context):  # noqa: C901
             py2_pkg = None
 
     py3_pkg = context.apt.get_package_for_paths(
-        ["/usr/lib/python3/dist-packages/%s-.*.egg-info" % error.distribution],
+        ["/usr/lib/python3/dist-packages/%s-.*.egg-info/PKG-INFO" % error.distribution],
         regex=True,
     )
     if py3_pkg is None:
@@ -784,8 +783,9 @@ def main(argv=None):
 
     args = parser.parse_args()
     from breezy.workingtree import WorkingTree
-
+    from .apt import AptManager
-    apt = LocalAptManager()
+    from ..session.plain import PlainSession
+    apt = AptManager(PlainSession())
 
     tree = WorkingTree.open(".")
     build_incrementally(

ognibuild/requirements.py

@@ -32,6 +32,11 @@ class PythonPackageRequirement(UpstreamRequirement):
         self.python_version = python_version
         self.minimum_version = minimum_version
 
+    def __repr__(self):
+        return "%s(%r, %r, %r)" % (
+            type(self).__name__, self.package, self.python_version,
+            self.minimum_version)
+
 
 class BinaryRequirement(UpstreamRequirement):
 

ognibuild/resolver/__init__.py

@@ -30,6 +30,9 @@ class Resolver(object):
     def explain(self, requirements):
         raise NotImplementedError(self.explain)
 
+    def met(self, requirement):
+        raise NotImplementedError(self.met)
+
 
 class NativeResolver(Resolver):
     def __init__(self, session):

ognibuild/resolver/apt.py

@@ -48,6 +48,8 @@ from ..requirements import (
     PerlModuleRequirement,
     PerlFileRequirement,
     AutoconfMacroRequirement,
+    PythonModuleRequirement,
+    PythonPackageRequirement,
     )
 
 
@@ -55,6 +57,23 @@ class NoAptPackage(Exception):
     """No apt package."""
 
 
+def get_package_for_python_package(apt_mgr, package, python_version):
+    if python_version == "pypy":
+        return apt_mgr.get_package_for_paths(
+            ["/usr/lib/pypy/dist-packages/%s-.*.egg-info/PKG-INFO" % package],
+            regex=True)
+    elif python_version == "cpython2":
+        return apt_mgr.get_package_for_paths(
+            ["/usr/lib/python2\\.[0-9]/dist-packages/%s-.*.egg-info/PKG-INFO" % package],
+            regex=True)
+    elif python_version == "cpython3":
+        return apt_mgr.get_package_for_paths(
+            ["/usr/lib/python3/dist-packages/%s-.*.egg-info/PKG-INFO" % package],
+            regex=True)
+    else:
+        raise NotImplementedError
+
+
 def get_package_for_python_module(apt_mgr, module, python_version):
     if python_version == "python3":
         paths = [
@@ -354,6 +373,24 @@ def resolve_autoconf_macro_req(apt_mgr, req):
     return apt_mgr.get_package_for_paths([path])
 
 
+def resolve_python_module_req(apt_mgr, req):
+    if req.python_version == 2:
+        return get_package_for_python_module(apt_mgr, req.module, "cpython2")
+    elif req.python_version in (None, 3):
+        return get_package_for_python_module(apt_mgr, req.module, "cpython3")
+    else:
+        return None
+
+
+def resolve_python_package_req(apt_mgr, req):
+    if req.python_version == 2:
+        return get_package_for_python_package(apt_mgr, req.package, "cpython2")
+    elif req.python_version in (None, 3):
+        return get_package_for_python_package(apt_mgr, req.package, "cpython3")
+    else:
+        return None
+
+
 APT_REQUIREMENT_RESOLVERS = [
     (BinaryRequirement, resolve_binary_req),
     (PkgConfigRequirement, resolve_pkg_config_req),
@@ -379,6 +416,8 @@ APT_REQUIREMENT_RESOLVERS = [
     (PerlModuleRequirement, resolve_perl_module_req),
     (PerlFileRequirement, resolve_perl_file_req),
     (AutoconfMacroRequirement, resolve_autoconf_macro_req),
+    (PythonModuleRequirement, resolve_python_module_req),
+    (PythonPackageRequirement, resolve_python_package_req),
 ]
 
 
@@ -387,7 +426,7 @@ def resolve_requirement_apt(apt_mgr, req: UpstreamRequirement):
         if isinstance(req, rr_class):
             deb_req = rr_fn(apt_mgr, req)
             if deb_req is None:
-                raise NoAptPackage()
+                raise NoAptPackage(req)
             return deb_req
     raise NotImplementedError(type(req))
 
@@ -401,16 +440,18 @@ class AptResolver(Resolver):
     def from_session(cls, session):
         return cls(AptManager(session))
 
+    def met(self, requirement):
+        pps = list(requirement.possible_paths())
+        return any(self.apt.session.exists(p) for p in pps)
+
     def install(self, requirements):
         missing = []
         for req in requirements:
             try:
-                pps = list(req.possible_paths())
+                if not self.met(req):
+                    missing.append(req)
             except NotImplementedError:
                 missing.append(req)
-            else:
-                if not pps or not any(self.apt.session.exists(p) for p in pps):
-                    missing.append(req)
         if missing:
             self.apt.install([self.resolve(m) for m in missing])