More refactoring.

ognibuild/__main__.py

@@ -67,8 +67,8 @@ STAGE_MAP = {
 
 
 def determine_fixers(session, resolver):
-    from .buildlog import RequirementFixer
-    return [RequirementFixer(resolver)]
+    from .buildlog import InstallFixer
+    return [InstallFixer(resolver)]
 
 
 def main():  # noqa: C901

ognibuild/buildlog.py

@@ -81,6 +81,7 @@ from .requirements import (
     PythonModuleRequirement,
     PythonPackageRequirement,
 )
+from .resolver import UnsatisfiedRequirements
 
 
 def problem_to_upstream_requirement(problem):  # noqa: C901
@@ -166,7 +167,7 @@ def problem_to_upstream_requirement(problem):  # noqa: C901
         return None
 
 
-class RequirementFixer(BuildFixer):
+class InstallFixer(BuildFixer):
     def __init__(self, resolver):
         self.resolver = resolver
 
@@ -188,11 +189,8 @@ class RequirementFixer(BuildFixer):
         if not isinstance(reqs, list):
             reqs = [reqs]
 
-        changed = False
-        for req in reqs:
-            package = self.resolver.resolve(req)
-            if package is None:
-                return False
-            if context.add_dependency(package):
-                changed = True
-        return changed
+        try:
+            self.resolver.install(reqs)
+        except UnsatisfiedRequirements:
+            return False
+        return True

ognibuild/buildsystem.py

@@ -457,7 +457,7 @@ class Make(BuildSystem):
                 run_with_build_fixers(session, ["autoreconf", "-i"], fixers)
 
         if not makefile_exists() and session.exists("configure"):
-            session.check_call(["./configure"])
+            run_with_build_fixers(session, ["./configure"], fixers)
 
     def build(self, session, resolver, fixers):
         self.setup(session, resolver, fixers)
@@ -553,9 +553,14 @@ class Cargo(BuildSystem):
 
     name = "cargo"
 
+    def __repr__(self):
+        return "%s(%r)" % (type(self).__name__, self.path)
+
     def __init__(self, path):
         from toml.decoder import load
 
+        self.path = path
+
         with open(path, "r") as f:
             self.cargo = load(f)
 
@@ -568,12 +573,30 @@ class Cargo(BuildSystem):
     def test(self, session, resolver, fixers):
         run_with_build_fixers(session, ["cargo", "test"], fixers)
 
+    def clean(self, session, resolver, fixers):
+        run_with_build_fixers(session, ["cargo", "clean"], fixers)
+
+    def build(self, session, resolver, fixers):
+        run_with_build_fixers(session, ["cargo", "build"], fixers)
+
 
 class Golang(BuildSystem):
     """Go builds."""
 
     name = "golang"
 
+    def __repr__(self):
+        return "%s()" % (type(self).__name__)
+
+    def test(self, session, resolver, fixers):
+        session.check_call(["go", "test"])
+
+    def build(self, session, resolver, fixers):
+        session.check_call(["go", "build"])
+
+    def clean(self, session, resolver, fixers):
+        session.check_call(["go", "clean"])
+
 
 class Maven(BuildSystem):
 
@@ -664,6 +687,7 @@ def detect_buildsystems(path, trust_package=False):  # noqa: C901
                 "GNUmakefile",
                 "makefile",
                 "Makefile.PL",
+                "CMakeLists.txt",
                 "autogen.sh",
                 "configure.ac",
                 "configure.in",
@@ -672,6 +696,7 @@ def detect_buildsystems(path, trust_package=False):  # noqa: C901
     ):
         yield Make()
 
+    seen_golang = False
     if os.path.exists(os.path.join(path, ".travis.yml")):
         import ruamel.yaml.reader
 
@@ -684,11 +709,13 @@ def detect_buildsystems(path, trust_package=False):  # noqa: C901
                 language = data.get("language")
                 if language == "go":
                     yield Golang()
+                    seen_golang = True
 
-    for entry in os.scandir(path):
-        if entry.name.endswith(".go"):
-            yield Golang()
-            break
+    if not seen_golang:
+        for entry in os.scandir(path):
+            if entry.name.endswith(".go"):
+                yield Golang()
+                break
 
 
 def get_buildsystem(path, trust_package=False):

ognibuild/debian/fix_build.py

@@ -79,8 +79,8 @@ from buildlog_consultant.sbuild import (
     SbuildFailure,
 )
 
+from ..buildlog import problem_to_upstream_requirement
 from ..fix_build import BuildFixer, resolve_error, DependencyContext
-from ..buildlog import RequirementFixer
 from ..resolver.apt import (
     AptRequirement,
     get_package_for_python_module,
@@ -98,7 +98,66 @@ class CircularDependency(Exception):
         self.package = package
 
 
+class PackageDependencyFixer(BuildFixer):
+
+    def __init__(self, apt_resolver):
+        self.apt_resolver = apt_resolver
+
+    def __repr__(self):
+        return "%s(%r)" % (type(self).__name__, self.apt_resolver)
+
+    def __str__(self):
+        return "upstream requirement fixer(%s)" % self.apt_resolver
+
+    def can_fix(self, error):
+        req = problem_to_upstream_requirement(error)
+        return req is not None
+
+    def fix(self, error, context):
+        reqs = problem_to_upstream_requirement(error)
+        if reqs is None:
+            return False
+
+        if not isinstance(reqs, list):
+            reqs = [reqs]
+
+        changed = False
+        for req in reqs:
+            package = self.apt_resolver.resolve(req)
+            if package is None:
+                return False
+            if context.phase[0] == "autopkgtest":
+                return add_test_dependency(
+                    context.tree,
+                    context.phase[1],
+                    package,
+                    committer=context.committer,
+                    subpath=context.subpath,
+                    update_changelog=context.update_changelog,
+                )
+            elif context.phase[0] == "build":
+                return add_build_dependency(
+                    context.tree,
+                    package,
+                    committer=context.committer,
+                    subpath=context.subpath,
+                    update_changelog=context.update_changelog,
+                )
+            else:
+                logging.warning('Unknown phase %r', context.phase)
+                return False
+        return changed
+
+
 class BuildDependencyContext(DependencyContext):
+    def __init__(
+        self, phase, tree, apt, subpath="", committer=None, update_changelog=True
+    ):
+        self.phase = phase
+        super(BuildDependencyContext, self).__init__(
+            tree, apt, subpath, committer, update_changelog
+        )
+
     def add_dependency(self, requirement: AptRequirement):
         return add_build_dependency(
             self.tree,
@@ -111,9 +170,9 @@ class BuildDependencyContext(DependencyContext):
 
 class AutopkgtestDependencyContext(DependencyContext):
     def __init__(
-        self, testname, tree, apt, subpath="", committer=None, update_changelog=True
+        self, phase, tree, apt, subpath="", committer=None, update_changelog=True
     ):
-        self.testname = testname
+        self.phase = phase
         super(AutopkgtestDependencyContext, self).__init__(
             tree, apt, subpath, committer, update_changelog
         )
@@ -498,13 +557,12 @@ def versioned_package_fixers(session):
 
 def apt_fixers(apt) -> List[BuildFixer]:
     from ..resolver.apt import AptResolver
-
     resolver = AptResolver(apt)
     return [
         SimpleBuildFixer(MissingPythonModule, fix_missing_python_module),
         SimpleBuildFixer(MissingPythonDistribution, fix_missing_python_distribution),
         SimpleBuildFixer(AptFetchFailure, retry_apt_failure),
-        RequirementFixer(resolver),
+        PackageDependencyFixer(resolver),
     ]
 
 
@@ -553,6 +611,7 @@ def build_incrementally(
             reset_tree(local_tree, local_tree.basis_tree(), subpath=subpath)
             if e.phase[0] == "build":
                 context = BuildDependencyContext(
+                    e.phase,
                     local_tree,
                     apt,
                     subpath=subpath,
@@ -561,7 +620,7 @@ def build_incrementally(
                 )
             elif e.phase[0] == "autopkgtest":
                 context = AutopkgtestDependencyContext(
-                    e.phase[1],
+                    e.phase,
                     local_tree,
                     apt,
                     subpath=subpath,

ognibuild/dist.py

@@ -133,7 +133,7 @@ def create_dist_schroot(
 ) -> str:
     from .buildsystem import detect_buildsystems
     from .resolver.apt import AptResolver
-    from .buildlog import RequirementFixer
+    from .buildlog import InstallFixer
 
     if subdir is None:
         subdir = "package"
@@ -159,7 +159,7 @@ def create_dist_schroot(
 
         buildsystems = list(detect_buildsystems(export_directory))
         resolver = AptResolver.from_session(session)
-        fixers = [RequirementFixer(resolver)]
+        fixers = [InstallFixer(resolver)]
 
         with DistCatcher(export_directory) as dc:
             oldcwd = os.getcwd()

ognibuild/fix_build.py

@@ -58,23 +58,10 @@ class DependencyContext(object):
         self.committer = committer
         self.update_changelog = update_changelog
 
-    def add_dependency(
-        self, package: str, minimum_version=None
-    ) -> bool:
+    def add_dependency(self, package) -> bool:
         raise NotImplementedError(self.add_dependency)
 
 
-class SchrootDependencyContext(DependencyContext):
-    def __init__(self, session):
-        self.session = session
-        self.apt = AptManager(session)
-
-    def add_dependency(self, package, minimum_version=None):
-        # TODO(jelmer): Handle minimum_version
-        self.apt.install([package])
-        return True
-
-
 def run_with_build_fixers(session: Session, args: List[str], fixers: List[BuildFixer]):
     logging.info("Running %r", args)
     fixed_errors = []
@@ -99,7 +86,7 @@ def run_with_build_fixers(session: Session, args: List[str], fixers: List[BuildF
             raise DetailedFailure(retcode, args, error)
         if not resolve_error(
             error,
-            SchrootDependencyContext(session),
+            None,
             fixers=fixers,
         ):
             logging.warning("Failed to find resolution for error %r. Giving up.", error)

ognibuild/requirements.py

@@ -98,6 +98,15 @@ class CargoCrateRequirement(Requirement):
         super(CargoCrateRequirement, self).__init__("cargo-crate")
         self.crate = crate
 
+    def __repr__(self):
+        return "%s(%r)" % (
+            type(self).__name__,
+            self.crate,
+        )
+
+    def __str__(self):
+        return "cargo crate: %s" % self.crate
+
 
 class PkgConfigRequirement(Requirement):
 

ognibuild/resolver/__init__.py

@@ -42,6 +42,9 @@ class CPANResolver(Resolver):
     def __str__(self):
         return "cpan"
 
+    def __repr__(self):
+        return "%s(%r)" % (type(self).__name__, self.session)
+
     def install(self, requirements):
         from ..requirements import PerlModuleRequirement
 
@@ -70,6 +73,9 @@ class HackageResolver(Resolver):
     def __str__(self):
         return "hackage"
 
+    def __repr__(self):
+        return "%s(%r)" % (type(self).__name__, self.session)
+
     def install(self, requirements):
         from ..requirements import HaskellPackageRequirement
 
@@ -88,31 +94,6 @@ class HackageResolver(Resolver):
         raise NotImplementedError(self.explain)
 
 
-class CargoResolver(Resolver):
-    def __init__(self, session):
-        self.session = session
-
-    def __str__(self):
-        return "cargo"
-
-    def install(self, requirements):
-        from ..requirements import CargoCrateRequirement
-
-        missing = []
-        for requirement in requirements:
-            if not isinstance(requirement, CargoCrateRequirement):
-                missing.append(requirement)
-                continue
-            self.session.check_call(
-                ["cargo", "install", requirement.crate], user="root"
-            )
-        if missing:
-            raise UnsatisfiedRequirements(missing)
-
-    def explain(self, requirements):
-        raise NotImplementedError(self.explain)
-
-
 class PypiResolver(Resolver):
     def __init__(self, session):
         self.session = session
@@ -120,6 +101,9 @@ class PypiResolver(Resolver):
     def __str__(self):
         return "pypi"
 
+    def __repr__(self):
+        return "%s(%r)" % (type(self).__name__, self.session)
+
     def install(self, requirements):
         from ..requirements import PythonPackageRequirement
 
@@ -148,6 +132,9 @@ class NpmResolver(Resolver):
     def __str__(self):
         return "npm"
 
+    def __repr__(self):
+        return "%s(%r)" % (type(self).__name__, self.session)
+
     def install(self, requirements):
         from ..requirements import NodePackageRequirement
 
@@ -195,7 +182,6 @@ def native_resolvers(session):
             CPANResolver(session),
             PypiResolver(session),
             NpmResolver(session),
-            CargoResolver(session),
             HackageResolver(session),
         ]
     )
@@ -227,7 +213,6 @@ def auto_resolver(session):
             CPANResolver(session),
             PypiResolver(session),
             NpmResolver(session),
-            CargoResolver(session),
             HackageResolver(session),
         ]
     )

ognibuild/session/plain.py

@@ -27,6 +27,9 @@ class PlainSession(Session):
 
     location = "/"
 
+    def __repr__(self):
+        return "%s()" % (type(self).__name__, )
+
     def create_home(self):
         pass
 

ognibuild/tests/test_debian_fix_build.py

@@ -98,6 +98,7 @@ blah (0.1) UNRELEASED; urgency=medium
         apt = AptManager(session)
         apt._searchers = [DummyAptSearcher(self._apt_files)]
         context = BuildDependencyContext(
+            ("build", ),
             self.tree,
             apt,
             subpath="",