From c184e01aef01bc94ec2ffd7ad749b58aaace9af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jelmer=20Vernoo=C4=B3?= Date: Thu, 25 Feb 2021 03:22:55 +0000 Subject: [PATCH] More refactoring. --- notes/architecture.md | 29 ++++- ROADMAP => notes/roadmap.md | 0 ognibuild/__init__.py | 12 ++ ognibuild/__main__.py | 5 +- ognibuild/buildsystem.py | 12 +- ognibuild/debian/apt.py | 11 +- ognibuild/debian/fix_build.py | 222 ++++++++++++++-------------------- ognibuild/fix_build.py | 48 ++++++-- ognibuild/resolver/apt.py | 21 ++-- 9 files changed, 197 insertions(+), 163 deletions(-) rename ROADMAP => notes/roadmap.md (100%) diff --git a/notes/architecture.md b/notes/architecture.md index 960892c..02ee04f 100644 --- a/notes/architecture.md +++ b/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) diff --git a/ROADMAP b/notes/roadmap.md similarity index 100% rename from ROADMAP rename to notes/roadmap.md diff --git a/ognibuild/__init__.py b/ognibuild/__init__.py index 132e417..eb32b9d 100644 --- a/ognibuild/__init__.py +++ b/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): diff --git a/ognibuild/__main__.py b/ognibuild/__main__.py index ab562ce..808eb76 100644 --- a/ognibuild/__main__.py +++ b/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), diff --git a/ognibuild/buildsystem.py b/ognibuild/buildsystem.py index d36f019..6d311f8 100644 --- a/ognibuild/buildsystem.py +++ b/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")]) diff --git a/ognibuild/debian/apt.py b/ognibuild/debian/apt.py index e8a6934..cd55fa5 100644 --- a/ognibuild/debian/apt.py +++ b/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 diff --git a/ognibuild/debian/fix_build.py b/ognibuild/debian/fix_build.py index 677de43..50ecc24 100644 --- a/ognibuild/debian/fix_build.py +++ b/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 ..fix_build import BuildFixer, SimpleBuildFixer +from .apt import LocalAptManager +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): - if isinstance(error, MissingFile): - req = PathRequirement(error.path) - elif isinstance(error, MissingCommand): - req = BinaryRequirement(error.command) - elif isinstance(error, MissingPkgConfig): - req = PkgConfigRequirement( - error.module, error.minimum_version) - elif isinstance(error, MissingCHeader): - req = CHeaderRequirement(error.header) - elif isinstance(error, MissingJavaScriptRuntime): - req = JavaScriptRuntimeRequirement() - elif isinstance(error, MissingRubyGem): - req = RubyGemRequirement(error.gem, error.version) - elif isinstance(error, MissingValaPackage): - req = ValaPackageRequirement(error.package) - elif isinstance(error, MissingGoPackage): - req = GoPackageRequirement(error.package) - elif isinstance(error, DhAddonLoadFailure): - req = DhAddonRequirement(error.path) - elif isinstance(error, MissingPhpClass): - req = PhpClassRequirement(error.php_class) - elif isinstance(error, MissingRPackage): - req = RPackageRequirement(error.package, error.minimum_version) - elif isinstance(error, MissingNodeModule): - req = NodePackageRequirement(error.module) - elif isinstance(error, MissingLibrary): - req = LibraryRequirement(error.library) - elif isinstance(error, MissingRubyFile): - req = RubyFileRequirement(error.filename) - elif isinstance(error, MissingXmlEntity): - req = XmlEntityRequirement(error.url) - elif isinstance(error, MissingSprocketsFile): - req = SprocketsFileRequirement(error.content_type, error.name) - elif isinstance(error, MissingJavaClass): - req = JavaClassRequirement(error.classname) - elif isinstance(error, MissingHaskellDependencies): +def problem_to_upstream_requirement(problem, context): + if isinstance(problem, MissingFile): + return PathRequirement(problem.path) + elif isinstance(problem, MissingCommand): + return BinaryRequirement(problem.command) + elif isinstance(problem, MissingPkgConfig): + return PkgConfigRequirement( + problem.module, problem.minimum_version) + elif isinstance(problem, MissingCHeader): + return CHeaderRequirement(problem.header) + elif isinstance(problem, MissingJavaScriptRuntime): + return JavaScriptRuntimeRequirement() + elif isinstance(problem, MissingRubyGem): + return RubyGemRequirement(problem.gem, problem.version) + elif isinstance(problem, MissingValaPackage): + return ValaPackageRequirement(problem.package) + elif isinstance(problem, MissingGoPackage): + return GoPackageRequirement(problem.package) + elif isinstance(problem, DhAddonLoadFailure): + return DhAddonRequirement(problem.path) + elif isinstance(problem, MissingPhpClass): + return PhpClassRequirement(problem.php_class) + elif isinstance(problem, MissingRPackage): + return RPackageRequirement(problem.package, problem.minimum_version) + elif isinstance(problem, MissingNodeModule): + return NodePackageRequirement(problem.module) + elif isinstance(problem, MissingLibrary): + return LibraryRequirement(problem.library) + elif isinstance(problem, MissingRubyFile): + return RubyFileRequirement(problem.filename) + elif isinstance(problem, MissingXmlEntity): + return XmlEntityRequirement(problem.url) + elif isinstance(problem, MissingSprocketsFile): + return SprocketsFileRequirement(problem.content_type, problem.name) + elif isinstance(problem, MissingJavaClass): + return JavaClassRequirement(problem.classname) + elif isinstance(problem, MissingHaskellDependencies): # TODO(jelmer): Create multiple HaskellPackageRequirement objects? - req = HaskellPackageRequirement(error.package) - elif isinstance(error, MissingMavenArtifacts): + return HaskellPackageRequirement(problem.package) + elif isinstance(problem, MissingMavenArtifacts): # TODO(jelmer): Create multiple MavenArtifactRequirement objects? - req = MavenArtifactRequirement(error.artifacts) - elif isinstance(error, MissingCSharpCompiler): - req = BinaryRequirement('msc') - elif isinstance(error, GnomeCommonMissing): - req = GnomeCommonRequirement() - elif isinstance(error, MissingJDKFile): - req = JDKFileRequirement(error.jdk_path, error.filename) - elif isinstance(error, MissingGnomeCommonDependency): - if error.package == "glib-gettext": - req = BinaryRequirement('glib-gettextize') + return MavenArtifactRequirement(problem.artifacts) + elif isinstance(problem, MissingCSharpCompiler): + return BinaryRequirement('msc') + elif isinstance(problem, GnomeCommonMissing): + return GnomeCommonRequirement() + elif isinstance(problem, MissingJDKFile): + return JDKFileRequirement(problem.jdk_path, problem.filename) + elif isinstance(problem, MissingGnomeCommonDependency): + if problem.package == "glib-gettext": + 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): - if error.package == "gtk-doc": - req = BinaryRequirement("gtkdocize") + elif isinstance(problem, MissingXfceDependency): + if problem.package == "gtk-doc": + return BinaryRequirement("gtkdocize") else: logging.warning( "No known command for xfce dependency %s", - error.package) + problem.package) return None - elif isinstance(error, MissingPerlModule): - req = PerlModuleRequirement( - module=error.module, - filename=error.filename, - inc=error.inc) - elif isinstance(error, MissingPerlFile): - req = PerlFileRequirement(filename=error.filename) - elif isinstance(error, MissingAutoconfMacro): - req = AutoconfMacroRequirement(error.macro) + elif isinstance(problem, MissingPerlModule): + return PerlModuleRequirement( + module=problem.module, + filename=problem.filename, + inc=problem.inc) + elif isinstance(problem, MissingPerlFile): + return PerlFileRequirement(filename=problem.filename) + elif isinstance(problem, MissingAutoconfMacro): + 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), - SimpleBuildFixer(Problem, fix_missing_requirement), + UpstreamRequirementFixer(), ] 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" ) diff --git a/ognibuild/fix_build.py b/ognibuild/fix_build.py index a6326f2..5b6aef0 100644 --- a/ognibuild/fix_build.py +++ b/ognibuild/fix_build.py @@ -25,14 +25,10 @@ from buildlog_consultant.common import ( MissingPythonDistribution, MissingCommand, ) +from breezy.mutabletree import MutableTree -from . import DetailedFailure -from .apt import UnidentifiedError, AptManager -from .debian.fix_build import ( - DependencyContext, - resolve_error, - APT_FIXERS, -) +from . import DetailedFailure, UnidentifiedError +from .debian.apt import AptManager 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 diff --git a/ognibuild/resolver/apt.py b/ognibuild/resolver/apt.py index 6864119..a5a5cc5 100644 --- a/ognibuild/resolver/apt.py +++ b/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: - 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 + return resolve_requirement_apt(self.apt, req)