More refactoring.

notes/architecture.md

@@ -13,14 +13,16 @@ requirements are met.
 
 Then we attempt to build.
 
-If any problems are found in the log, buildlog-consultant will report them.
+If any Problems are found in the log, buildlog-consultant will report them.
 
-ognibuild can then invoke "fixers" to address Problems.
+ognibuild can then invoke "fixers" to address Problems. Fixers can do things
+like e.g. upgrade configure.ac to a newer version, or invoke autoreconf.
+
+A list of possible fixers can be provided. Each fixer will be called
+(in order) until one of them claims to ahve fixed the issue.
 
 Problems can be converted to UpstreamRequirements by UpstreamRequirementFixer
 
-Other Fixer can do things like e.g. upgrade configure.ac to a newer version.
-
 UpstreamRequirementFixer uses a UpstreamRequirementResolver object that
 can translate UpstreamRequirement objects into apt package names or
 e.g. cpan commands.
@@ -28,3 +30,22 @@ e.g. cpan commands.
 ognibuild keeps finding problems, resolving them and rebuilding until it finds
 a problem it can not resolve or that it thinks it has already resolved
 (i.e. seen before).
+
+Operations are run in a Session - this can represent a virtualized
+environment of some sort (e.g. a chroot or virtualenv) or simply
+on the host machine.
+
+For e.g. PerlModuleRequirement, need to be able to:
+
+ * install from apt package
+  + DebianInstallFixer(AptResolver()).fix(problem)
+ * update debian package (source, runtime, test) deps to include apt package
+  + DebianPackageDepFixer(AptResolver()).fix(problem, ('test', 'foo'))
+ * suggest command to run to install from apt package
+  + DebianInstallFixer(AptResolver()).command(problem)
+ * install from cpan
+  + CpanInstallFixer().fix(problem)
+ * suggest command to run to install from cpan package
+  + CpanInstallFixer().command(problem)
+ * update source package reqs to depend on perl module
+  + PerlDepFixer().fix(problem)

ognibuild/__init__.py

@@ -28,6 +28,15 @@ class DetailedFailure(Exception):
         self.error = error
 
 
+class UnidentifiedError(Exception):
+
+    def __init__(self, retcode, argv, lines, secondary=None):
+        self.retcode = retcode
+        self.argv = argv
+        self.lines = lines
+        self.secondary = secondary
+
+
 def shebang_binary(p):
     if not (os.stat(p).st_mode & stat.S_IEXEC):
         return None
@@ -49,6 +58,9 @@ class UpstreamRequirement(object):
     def __init__(self, family):
         self.family = family
 
+    def possible_paths(self):
+        raise NotImplementedError
+
 
 class UpstreamOutput(object):
 

ognibuild/__main__.py

@@ -18,7 +18,7 @@
 import logging
 import os
 import sys
-from .apt import UnidentifiedError
+from . import UnidentifiedError
 from .buildsystem import NoBuildToolsFound, detect_buildsystems
 from .build import run_build
 from .clean import run_clean
@@ -127,7 +127,8 @@ def main():  # noqa: C901
             return 1
         except MissingDependencies as e:
             for req in e.requirements:
-                logging.info("Missing dependency (%s:%s)", (req.family, req.name))
+                logging.info("Missing dependency (%s:%s)",
+                             req.family, req.name)
                 for resolver in [
                     AptResolver.from_session(session),
                     NativeResolver.from_session(session),

ognibuild/buildsystem.py

@@ -22,7 +22,7 @@ import os
 import re
 import warnings
 
-from . import shebang_binary, UpstreamOutput
+from . import shebang_binary, UpstreamOutput, UnidentifiedError
 from .requirements import (
     BinaryRequirement,
     PythonPackageRequirement,
@@ -30,7 +30,6 @@ from .requirements import (
     NodePackageRequirement,
     CargoCrateRequirement,
     )
-from .apt import UnidentifiedError
 from .fix_build import run_with_build_fixer
 
 
@@ -136,6 +135,10 @@ class SetupPy(BuildSystem):
         self.setup(resolver)
         self._run_setup(session, resolver, ["test"])
 
+    def build(self, session, resolver):
+        self.setup(resolver)
+        self._run_setup(session, resolver, ["build"])
+
     def dist(self, session, resolver):
         self.setup(resolver)
         self._run_setup(session, resolver, ["sdist"])
@@ -370,6 +373,11 @@ class Make(BuildSystem):
         if not session.exists("Makefile") and session.exists("configure"):
             session.check_call(["./configure"])
 
+    def build(self, session, resolver):
+        self.setup(session, resolver)
+        resolver.install([BinaryRequirement("make")])
+        run_with_build_fixer(session, ["make", "all"])
+
     def dist(self, session, resolver):
         self.setup(session, resolver)
         resolver.install([BinaryRequirement("make")])

ognibuild/debian/apt.py

@@ -26,19 +26,10 @@ from buildlog_consultant.apt import (
 )
 from debian.deb822 import Release
 
-from .. import DetailedFailure
+from .. import DetailedFailure, UnidentifiedError
 from ..session import Session, run_with_tee
 
 
-class UnidentifiedError(Exception):
-
-    def __init__(self, retcode, argv, lines, secondary=None):
-        self.retcode = retcode
-        self.argv = argv
-        self.lines = lines
-        self.secondary = secondary
-
-
 def run_apt(session: Session, args: List[str]) -> None:
     """Run apt."""
     args = ["apt", "-y"] + args

ognibuild/debian/fix_build.py

@@ -22,7 +22,7 @@ __all__ = [
 import logging
 import os
 import sys
-from typing import List, Callable, Type, Tuple, Set, Optional
+from typing import List, Set, Optional
 
 from debian.deb822 import (
     Deb822,
@@ -105,10 +105,9 @@ from buildlog_consultant.sbuild import (
     SbuildFailure,
 )
 
-from .apt import AptManager, LocalAptManager
+from .apt import LocalAptManager
-from ..fix_build import BuildFixer, SimpleBuildFixer
+from ..fix_build import BuildFixer, SimpleBuildFixer, resolve_error, DependencyContext
 from ..resolver.apt import (
-    AptResolver,
     NoAptPackage,
     get_package_for_python_module,
     )
@@ -151,31 +150,6 @@ class CircularDependency(Exception):
         self.package = package
 
 
-class DependencyContext(object):
-    def __init__(
-        self,
-        tree: MutableTree,
-        apt: AptManager,
-        subpath: str = "",
-        committer: Optional[str] = None,
-        update_changelog: bool = True,
-    ):
-        self.tree = tree
-        self.apt = apt
-        self.resolver = AptResolver(apt)
-        self.subpath = subpath
-        self.committer = committer
-        self.update_changelog = update_changelog
-
-    def resolve_apt(self, req):
-        return self.resolver.resolve(req)
-
-    def add_dependency(
-        self, package: str, minimum_version: Optional[Version] = None
-    ) -> bool:
-        raise NotImplementedError(self.add_dependency)
-
-
 class BuildDependencyContext(DependencyContext):
     def add_dependency(self, package: str, minimum_version: Optional[Version] = None):
         return add_build_dependency(
@@ -462,90 +436,95 @@ def fix_missing_python_module(error, context):
     return True
 
 
-def fix_missing_requirement(error, context):
+def problem_to_upstream_requirement(problem, context):
-    if isinstance(error, MissingFile):
+    if isinstance(problem, MissingFile):
-        req = PathRequirement(error.path)
+        return PathRequirement(problem.path)
-    elif isinstance(error, MissingCommand):
+    elif isinstance(problem, MissingCommand):
-        req = BinaryRequirement(error.command)
+        return BinaryRequirement(problem.command)
-    elif isinstance(error, MissingPkgConfig):
+    elif isinstance(problem, MissingPkgConfig):
-        req = PkgConfigRequirement(
+        return PkgConfigRequirement(
-            error.module, error.minimum_version)
+            problem.module, problem.minimum_version)
-    elif isinstance(error, MissingCHeader):
+    elif isinstance(problem, MissingCHeader):
-        req = CHeaderRequirement(error.header)
+        return CHeaderRequirement(problem.header)
-    elif isinstance(error, MissingJavaScriptRuntime):
+    elif isinstance(problem, MissingJavaScriptRuntime):
-        req = JavaScriptRuntimeRequirement()
+        return JavaScriptRuntimeRequirement()
-    elif isinstance(error, MissingRubyGem):
+    elif isinstance(problem, MissingRubyGem):
-        req = RubyGemRequirement(error.gem, error.version)
+        return RubyGemRequirement(problem.gem, problem.version)
-    elif isinstance(error, MissingValaPackage):
+    elif isinstance(problem, MissingValaPackage):
-        req = ValaPackageRequirement(error.package)
+        return ValaPackageRequirement(problem.package)
-    elif isinstance(error, MissingGoPackage):
+    elif isinstance(problem, MissingGoPackage):
-        req = GoPackageRequirement(error.package)
+        return GoPackageRequirement(problem.package)
-    elif isinstance(error, DhAddonLoadFailure):
+    elif isinstance(problem, DhAddonLoadFailure):
-        req = DhAddonRequirement(error.path)
+        return DhAddonRequirement(problem.path)
-    elif isinstance(error, MissingPhpClass):
+    elif isinstance(problem, MissingPhpClass):
-        req = PhpClassRequirement(error.php_class)
+        return PhpClassRequirement(problem.php_class)
-    elif isinstance(error, MissingRPackage):
+    elif isinstance(problem, MissingRPackage):
-        req = RPackageRequirement(error.package, error.minimum_version)
+        return RPackageRequirement(problem.package, problem.minimum_version)
-    elif isinstance(error, MissingNodeModule):
+    elif isinstance(problem, MissingNodeModule):
-        req = NodePackageRequirement(error.module)
+        return NodePackageRequirement(problem.module)
-    elif isinstance(error, MissingLibrary):
+    elif isinstance(problem, MissingLibrary):
-        req = LibraryRequirement(error.library)
+        return LibraryRequirement(problem.library)
-    elif isinstance(error, MissingRubyFile):
+    elif isinstance(problem, MissingRubyFile):
-        req = RubyFileRequirement(error.filename)
+        return RubyFileRequirement(problem.filename)
-    elif isinstance(error, MissingXmlEntity):
+    elif isinstance(problem, MissingXmlEntity):
-        req = XmlEntityRequirement(error.url)
+        return XmlEntityRequirement(problem.url)
-    elif isinstance(error, MissingSprocketsFile):
+    elif isinstance(problem, MissingSprocketsFile):
-        req = SprocketsFileRequirement(error.content_type, error.name)
+        return SprocketsFileRequirement(problem.content_type, problem.name)
-    elif isinstance(error, MissingJavaClass):
+    elif isinstance(problem, MissingJavaClass):
-        req = JavaClassRequirement(error.classname)
+        return JavaClassRequirement(problem.classname)
-    elif isinstance(error, MissingHaskellDependencies):
+    elif isinstance(problem, MissingHaskellDependencies):
         # TODO(jelmer): Create multiple HaskellPackageRequirement objects?
-        req = HaskellPackageRequirement(error.package)
+        return HaskellPackageRequirement(problem.package)
-    elif isinstance(error, MissingMavenArtifacts):
+    elif isinstance(problem, MissingMavenArtifacts):
         # TODO(jelmer): Create multiple MavenArtifactRequirement objects?
-        req = MavenArtifactRequirement(error.artifacts)
+        return MavenArtifactRequirement(problem.artifacts)
-    elif isinstance(error, MissingCSharpCompiler):
+    elif isinstance(problem, MissingCSharpCompiler):
-        req = BinaryRequirement('msc')
+        return BinaryRequirement('msc')
-    elif isinstance(error, GnomeCommonMissing):
+    elif isinstance(problem, GnomeCommonMissing):
-        req = GnomeCommonRequirement()
+        return GnomeCommonRequirement()
-    elif isinstance(error, MissingJDKFile):
+    elif isinstance(problem, MissingJDKFile):
-        req = JDKFileRequirement(error.jdk_path, error.filename)
+        return JDKFileRequirement(problem.jdk_path, problem.filename)
-    elif isinstance(error, MissingGnomeCommonDependency):
+    elif isinstance(problem, MissingGnomeCommonDependency):
-        if error.package == "glib-gettext":
+        if problem.package == "glib-gettext":
-            req = BinaryRequirement('glib-gettextize')
+            return BinaryRequirement('glib-gettextize')
         else:
             logging.warning(
                 "No known command for gnome-common dependency %s",
-                error.package)
+                problem.package)
             return None
-    elif isinstance(error, MissingXfceDependency):
+    elif isinstance(problem, MissingXfceDependency):
-        if error.package == "gtk-doc":
+        if problem.package == "gtk-doc":
-            req = BinaryRequirement("gtkdocize")
+            return BinaryRequirement("gtkdocize")
         else:
             logging.warning(
                 "No known command for xfce dependency %s",
-                error.package)
+                problem.package)
             return None
-    elif isinstance(error, MissingPerlModule):
+    elif isinstance(problem, MissingPerlModule):
-        req = PerlModuleRequirement(
+        return PerlModuleRequirement(
-            module=error.module,
+            module=problem.module,
-            filename=error.filename,
+            filename=problem.filename,
-            inc=error.inc)
+            inc=problem.inc)
-    elif isinstance(error, MissingPerlFile):
+    elif isinstance(problem, MissingPerlFile):
-        req = PerlFileRequirement(filename=error.filename)
+        return PerlFileRequirement(filename=problem.filename)
-    elif isinstance(error, MissingAutoconfMacro):
+    elif isinstance(problem, MissingAutoconfMacro):
-        req = AutoconfMacroRequirement(error.macro)
+        return AutoconfMacroRequirement(problem.macro)
     else:
         return None
 
-    try:
-        package = context.resolve_apt(req)
-    except NoAptPackage:
-        return False
-    return context.add_dependency(package)
 
+class UpstreamRequirementFixer(BuildFixer):
 
-DEFAULT_PERL_PATHS = ["/usr/share/perl5"]
+    def fix_missing_requirement(self, error, context):
+        req = problem_to_upstream_requirement(error)
+        if req is None:
+            return False
+
+        try:
+            package = context.resolver.resolve(req)
+        except NoAptPackage:
+            return False
+        return context.add_dependency(package)
 
 
 def retry_apt_failure(error, context):
@@ -646,6 +625,7 @@ VERSIONED_PACKAGE_FIXERS: List[BuildFixer] = [
         NeedPgBuildExtUpdateControl, run_pgbuildext_updatecontrol),
     SimpleBuildFixer(MissingConfigure, fix_missing_configure),
     SimpleBuildFixer(MissingAutomakeInput, fix_missing_automake_input),
+    SimpleBuildFixer(MissingConfigStatusInput, fix_missing_config_status_input),
 ]
 
 
@@ -653,36 +633,15 @@ APT_FIXERS: List[BuildFixer] = [
     SimpleBuildFixer(MissingPythonModule, fix_missing_python_module),
     SimpleBuildFixer(MissingPythonDistribution, fix_missing_python_distribution),
     SimpleBuildFixer(AptFetchFailure, retry_apt_failure),
-    SimpleBuildFixer(MissingPerlFile, fix_missing_makefile_pl),
+    UpstreamRequirementFixer(),
-    SimpleBuildFixer(Problem, fix_missing_requirement),
 ]
 
 
 GENERIC_FIXERS: List[BuildFixer] = [
-    SimpleBuildFixer(MissingConfigStatusInput, fix_missing_config_status_input),
+    SimpleBuildFixer(MissingPerlFile, fix_missing_makefile_pl),
 ]
 
 
-def resolve_error(error, context, fixers):
-    relevant_fixers = []
-    for error_cls, fixer in fixers:
-        if isinstance(error, error_cls):
-            relevant_fixers.append(fixer)
-    if not relevant_fixers:
-        logging.warning("No fixer found for %r", error)
-        return False
-    for fixer in relevant_fixers:
-        logging.info("Attempting to use fixer %r to address %r", fixer, error)
-        try:
-            made_changes = fixer(error, context)
-        except GeneratedFile:
-            logging.warning("Control file is generated, unable to edit.")
-            return False
-        if made_changes:
-            return True
-    return False
-
-
 def build_incrementally(
     local_tree,
     apt,
@@ -714,17 +673,17 @@ def build_incrementally(
             if e.error is None:
                 logging.warning("Build failed with unidentified error. Giving up.")
                 raise
-            if e.context is None:
+            if e.phase is None:
                 logging.info("No relevant context, not making any changes.")
                 raise
-            if (e.error, e.context) in fixed_errors:
+            if (e.error, e.phase) in fixed_errors:
                 logging.warning("Error was still not fixed on second try. Giving up.")
                 raise
             if max_iterations is not None and len(fixed_errors) > max_iterations:
                 logging.warning("Last fix did not address the issue. Giving up.")
                 raise
             reset_tree(local_tree, local_tree.basis_tree(), subpath=subpath)
-            if e.context[0] == "build":
+            if e.phase[0] == "build":
                 context = BuildDependencyContext(
                     local_tree,
                     apt,
@@ -732,9 +691,9 @@ def build_incrementally(
                     committer=committer,
                     update_changelog=update_changelog,
                 )
-            elif e.context[0] == "autopkgtest":
+            elif e.phase[0] == "autopkgtest":
                 context = AutopkgtestDependencyContext(
-                    e.context[1],
+                    e.phase[1],
                     local_tree,
                     apt,
                     subpath=subpath,
@@ -742,7 +701,7 @@ def build_incrementally(
                     update_changelog=update_changelog,
                 )
             else:
-                logging.warning("unable to install for context %r", e.context)
+                logging.warning("unable to install for context %r", e.phase)
                 raise
             try:
                 if not resolve_error(
@@ -750,13 +709,18 @@ def build_incrementally(
                 ):
                     logging.warning("Failed to resolve error %r. Giving up.", e.error)
                     raise
+            except GeneratedFile:
+                logging.warning(
+                    "Control file is generated, unable to edit to "
+                    "resolver error %r.", e.error)
+                raise e
             except CircularDependency:
                 logging.warning(
                     "Unable to fix %r; it would introduce a circular " "dependency.",
                     e.error,
                 )
                 raise e
-            fixed_errors.append((e.error, e.context))
+            fixed_errors.append((e.error, e.phase))
             if os.path.exists(os.path.join(output_directory, "build.log")):
                 i = 1
                 while os.path.exists(
@@ -772,7 +736,7 @@ def build_incrementally(
 def main(argv=None):
     import argparse
 
-    parser = argparse.ArgumentParser("janitor.fix_build")
+    parser = argparse.ArgumentParser("ognibuild.debian.fix_build")
     parser.add_argument(
         "--suffix", type=str, help="Suffix to use for test builds.", default="fixbuild1"
     )

ognibuild/fix_build.py

@@ -25,14 +25,10 @@ from buildlog_consultant.common import (
     MissingPythonDistribution,
     MissingCommand,
 )
+from breezy.mutabletree import MutableTree
 
-from . import DetailedFailure
+from . import DetailedFailure, UnidentifiedError
-from .apt import UnidentifiedError, AptManager
+from .debian.apt import AptManager
-from .debian.fix_build import (
-    DependencyContext,
-    resolve_error,
-    APT_FIXERS,
-)
 from .session import Session, run_with_tee
 
 
@@ -64,6 +60,28 @@ class SimpleBuildFixer(BuildFixer):
         return self._fn(problem, context)
 
 
+class DependencyContext(object):
+    def __init__(
+        self,
+        tree: MutableTree,
+        apt: AptManager,
+        subpath: str = "",
+        committer: Optional[str] = None,
+        update_changelog: bool = True,
+    ):
+        self.tree = tree
+        self.apt = apt
+        self.resolver = AptResolver(apt)
+        self.subpath = subpath
+        self.committer = committer
+        self.update_changelog = update_changelog
+
+    def add_dependency(
+        self, package: str, minimum_version: Optional['Version'] = None
+    ) -> bool:
+        raise NotImplementedError(self.add_dependency)
+
+
 class SchrootDependencyContext(DependencyContext):
     def __init__(self, session):
         self.session = session
@@ -144,3 +162,19 @@ def run_with_build_fixer(
             logging.warning("Failed to find resolution for error %r. Giving up.", error)
             raise DetailedFailure(retcode, args, error)
         fixed_errors.append(error)
+
+
+def resolve_error(error, context, fixers):
+    relevant_fixers = []
+    for error_cls, fixer in fixers:
+        if isinstance(error, error_cls):
+            relevant_fixers.append(fixer)
+    if not relevant_fixers:
+        logging.warning("No fixer found for %r", error)
+        return False
+    for fixer in relevant_fixers:
+        logging.info("Attempting to use fixer %r to address %r", fixer, error)
+        made_changes = fixer(error, context)
+        if made_changes:
+            return True
+    return False

ognibuild/resolver/apt.py

@@ -381,6 +381,16 @@ APT_REQUIREMENT_RESOLVERS = [
 ]
 
 
+def resolve_requirement_apt(apt_mgr, req: UpstreamRequirement):
+    for rr_class, rr_fn in APT_REQUIREMENT_RESOLVERS:
+        if isinstance(req, rr_class):
+            deb_req = rr_fn(apt_mgr, req)
+            if deb_req is None:
+                raise NoAptPackage()
+            return deb_req
+    raise NotImplementedError(type(req))
+
+
 class AptResolver(Resolver):
 
     def __init__(self, apt):
@@ -401,17 +411,10 @@ class AptResolver(Resolver):
                 if not pps or not any(self.apt.session.exists(p) for p in pps):
                     missing.append(req)
         if missing:
-            self.apt.install(list(self.resolve(missing)))
+            self.apt.install([self.resolve(m) for m in missing])
 
     def explain(self, requirements):
         raise NotImplementedError(self.explain)
 
     def resolve(self, req: UpstreamRequirement):
-        for rr_class, rr_fn in APT_REQUIREMENT_RESOLVERS:
+        return resolve_requirement_apt(self.apt, req)
-            if isinstance(req, rr_class):
-                deb_req = rr_fn(self.apt, req)
-                if deb_req is None:
-                    raise NoAptPackage()
-                return deb_req
-        else:
-            raise NotImplementedError