Support more complex dist catching.

ognibuild/__main__.py

@@ -170,7 +170,8 @@ def main():  # noqa: C901
                 from .dist import run_dist
 
                 run_dist(
-                    session=session, buildsystems=bss, resolver=resolver, fixers=fixers
+                    session=session, buildsystems=bss, resolver=resolver, fixers=fixers,
+                    target_directory='.'
                 )
             if args.subcommand == "build":
                 from .build import run_build

ognibuild/buildsystem.py

@@ -26,6 +26,7 @@ from typing import Optional, Tuple
 import warnings
 
 from . import shebang_binary, UnidentifiedError
+from .dist_catcher import DistCatcher
 from .outputs import (
     BinaryOutput,
     PythonPackageOutput,
@@ -71,7 +72,7 @@ class BuildSystem(object):
     def __str__(self):
         return self.name
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory: str, quiet=False) -> str:
         raise NotImplementedError(self.dist)
 
     def test(self, session, resolver, fixers):
@@ -107,9 +108,11 @@ class Pear(BuildSystem):
     def setup(self, resolver):
         resolver.install([BinaryRequirement("pear")])
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory: str, quiet=False):
         self.setup(resolver)
-        run_with_build_fixers(session, ["pear", "package"], fixers)
+        with DistCatcher([session.external_path('.')]) as dc:
+            run_with_build_fixers(session, ["pear", "package"], fixers)
+        return dc.copy_single(target_directory)
 
     def test(self, session, resolver, fixers):
         self.setup(resolver)
@@ -336,7 +339,7 @@ class SetupPy(BuildSystem):
         else:
             raise NotImplementedError
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         # TODO(jelmer): Look at self.build_backend
         if self.has_setup_py:
             preargs = []
@@ -345,11 +348,13 @@ class SetupPy(BuildSystem):
             # Preemptively install setuptools since some packages fail in
             # some way without it.
             resolver.install([PythonPackageRequirement('setuptools')])
-            self._run_setup(session, resolver, preargs + ["sdist"], fixers)
+            with DistCatcher([session.external_path('dist')]) as dc:
-            return
+                self._run_setup(session, resolver, preargs + ["sdist"], fixers)
+            return dc.copy_single(target_directory)
         elif self.pyproject:
-            run_with_build_fixers(session, [self.DEFAULT_PYTHON, "-m", "pep517.build", "--source", "."], fixers)
+            with DistCatcher([session.external_path('dist')]) as dc:
-            return
+                run_with_build_fixers(session, [self.DEFAULT_PYTHON, "-m", "pep517.build", "--source", "."], fixers)
+            return dc.copy_single(target_directory)
         raise AssertionError("no setup.py or pyproject.toml")
 
     def clean(self, session, resolver, fixers):
@@ -566,8 +571,10 @@ class Gradle(BuildSystem):
     def test(self, session, resolver, fixers):
         self._run(session, resolver, 'test', [], fixers)
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
-        self._run(session, resolver, 'distTar', [], fixers)
+        with DistCatcher([session.external_path('.')]) as dc:
+            self._run(session, resolver, 'distTar', [], fixers)
+        return dc.copy_single(target_directory)
 
     def install(self, session, resolver, fixers, install_target):
         raise NotImplementedError
@@ -590,9 +597,11 @@ class R(BuildSystem):
     def build(self, session, resolver, fixers):
         pass
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         r_path = guaranteed_which(session, resolver, "R")
-        run_with_build_fixers(session, [r_path, "CMD", "build", "."], fixers)
+        with DistCatcher([session.external_path('.')]) as dc:
+            run_with_build_fixers(session, [r_path, "CMD", "build", "."], fixers)
+        return dc.copy_single(target_directory)
 
     def install(self, session, resolver, fixers, install_target):
         r_path = guaranteed_which(session, resolver, "R")
@@ -657,9 +666,11 @@ class Meson(BuildSystem):
         self._setup(session, fixers)
         run_with_build_fixers(session, ["ninja", "-C", "build"], fixers)
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         self._setup(session, fixers)
-        run_with_build_fixers(session, ["ninja", "-C", "build", "dist"], fixers)
+        with DistCatcher([session.external_path('build/meson-dist')]) as dc:
+            run_with_build_fixers(session, ["ninja", "-C", "build", "dist"], fixers)
+        return dc.copy_single(target_directory)
 
     def test(self, session, resolver, fixers):
         self._setup(session, fixers)
@@ -704,9 +715,11 @@ class Npm(BuildSystem):
     def setup(self, resolver):
         resolver.install([BinaryRequirement("npm")])
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         self.setup(resolver)
-        run_with_build_fixers(session, ["npm", "pack"], fixers)
+        with DistCatcher([session.external_path('.')]) as dc:
+            run_with_build_fixers(session, ["npm", "pack"], fixers)
+        return dc.copy_single(target_directory)
 
     def test(self, session, resolver, fixers):
         self.setup(resolver)
@@ -749,9 +762,11 @@ class Waf(BuildSystem):
     def setup(self, session, resolver, fixers):
         resolver.install([BinaryRequirement("python3")])
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         self.setup(session, resolver, fixers)
-        run_with_build_fixers(session, ["./waf", "dist"], fixers)
+        with DistCatcher.default(session.external_path('.')) as dc:
+            run_with_build_fixers(session, ["./waf", "dist"], fixers)
+        return dc.copy_single(target_directory)
 
     def test(self, session, resolver, fixers):
         self.setup(session, resolver, fixers)
@@ -774,14 +789,16 @@ class Gem(BuildSystem):
     def setup(self, resolver):
         resolver.install([BinaryRequirement("gem2deb")])
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         self.setup(resolver)
         gemfiles = [
             entry.name for entry in session.scandir(".") if entry.name.endswith(".gem")
         ]
         if len(gemfiles) > 1:
             logging.warning("More than one gemfile. Trying the first?")
-        run_with_build_fixers(session, ["gem2tgz", gemfiles[0]], fixers)
+        with DistCatcher.default(session.external_path('.')) as dc:
+            run_with_build_fixers(session, ["gem2tgz", gemfiles[0]], fixers)
+        return dc.copy_single(target_directory)
 
     @classmethod
     def probe(cls, path):
@@ -821,15 +838,19 @@ class DistZilla(BuildSystem):
             ]
         )
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         self.setup(resolver)
         if self.name == "dist-inkt":
             resolver.install([PerlModuleRequirement(self.dist_inkt_class)])
-            run_with_build_fixers(session, ["distinkt-dist"], fixers)
+            with DistCatcher.default(session.external_path('.')) as dc:
+                run_with_build_fixers(session, ["distinkt-dist"], fixers)
+            return dc.copy_single(target_directory)
         else:
             # Default to invoking Dist::Zilla
             resolver.install([PerlModuleRequirement("Dist::Zilla")])
-            run_with_build_fixers(session, ["dzil", "build", "--tgz"], fixers)
+            with DistCatcher.default(session.external_path('.')) as dc:
+                run_with_build_fixers(session, ["dzil", "build", "--tgz"], fixers)
+            return dc.copy_single(target_directory)
 
     def test(self, session, resolver, fixers):
         self.setup(resolver)
@@ -944,57 +965,59 @@ class Make(BuildSystem):
         self.setup(session, resolver, fixers)
         run_with_build_fixers(session, ["make", "install"], fixers)
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         self.setup(session, resolver, fixers)
-        try:
+        with DistCatcher.default(session.external_path('.')) as dc:
-            run_with_build_fixers(session, ["make", "dist"], fixers)
+            try:
-        except UnidentifiedError as e:
-            if "make: *** No rule to make target 'dist'.  Stop." in e.lines:
-                raise NotImplementedError
-            elif "make[1]: *** No rule to make target 'dist'. Stop." in e.lines:
-                raise NotImplementedError
-            elif (
-                "Reconfigure the source tree "
-                "(via './config' or 'perl Configure'), please."
-            ) in e.lines:
-                run_with_build_fixers(session, ["./config"], fixers)
                 run_with_build_fixers(session, ["make", "dist"], fixers)
-            elif (
+            except UnidentifiedError as e:
-                "Please try running 'make manifest' and then run "
+                if "make: *** No rule to make target 'dist'.  Stop." in e.lines:
-                "'make dist' again." in e.lines
+                    raise NotImplementedError
-            ):
+                elif "make[1]: *** No rule to make target 'dist'. Stop." in e.lines:
-                run_with_build_fixers(session, ["make", "manifest"], fixers)
+                    raise NotImplementedError
-                run_with_build_fixers(session, ["make", "dist"], fixers)
+                elif (
-            elif "Please run ./configure first" in e.lines:
+                    "Reconfigure the source tree "
-                run_with_build_fixers(session, ["./configure"], fixers)
+                    "(via './config' or 'perl Configure'), please."
-                run_with_build_fixers(session, ["make", "dist"], fixers)
+                ) in e.lines:
-            elif any(
+                    run_with_build_fixers(session, ["./config"], fixers)
-                [
+                    run_with_build_fixers(session, ["make", "dist"], fixers)
-                    re.match(
+                elif (
-                        r"(Makefile|GNUmakefile|makefile):[0-9]+: "
+                    "Please try running 'make manifest' and then run "
-                        r"\*\*\* Missing \'Make.inc\' "
+                    "'make dist' again." in e.lines
-                        r"Run \'./configure \[options\]\' and retry.  Stop.",
+                ):
-                        line,
+                    run_with_build_fixers(session, ["make", "manifest"], fixers)
-                    )
+                    run_with_build_fixers(session, ["make", "dist"], fixers)
-                    for line in e.lines
+                elif "Please run ./configure first" in e.lines:
-                ]
+                    run_with_build_fixers(session, ["./configure"], fixers)
-            ):
+                    run_with_build_fixers(session, ["make", "dist"], fixers)
-                run_with_build_fixers(session, ["./configure"], fixers)
+                elif any(
-                run_with_build_fixers(session, ["make", "dist"], fixers)
+                    [
-            elif any(
+                        re.match(
-                [
+                            r"(Makefile|GNUmakefile|makefile):[0-9]+: "
-                    re.match(
+                            r"\*\*\* Missing \'Make.inc\' "
-                        r"Problem opening MANIFEST: No such file or directory "
+                            r"Run \'./configure \[options\]\' and retry.  Stop.",
-                        r"at .* line [0-9]+\.",
+                            line,
-                        line,
+                        )
-                    )
+                        for line in e.lines
-                    for line in e.lines
+                    ]
-                ]
+                ):
-            ):
+                    run_with_build_fixers(session, ["./configure"], fixers)
-                run_with_build_fixers(session, ["make", "manifest"], fixers)
+                    run_with_build_fixers(session, ["make", "dist"], fixers)
-                run_with_build_fixers(session, ["make", "dist"], fixers)
+                elif any(
-            else:
+                    [
-                raise
+                        re.match(
+                            r"Problem opening MANIFEST: No such file or directory "
+                            r"at .* line [0-9]+\.",
+                            line,
+                        )
+                        for line in e.lines
+                    ]
+                ):
+                    run_with_build_fixers(session, ["make", "manifest"], fixers)
+                    run_with_build_fixers(session, ["make", "dist"], fixers)
+                else:
+                    raise
+        return dc.copy_single(target_directory)
 
     def get_declared_dependencies(self, session, fixers=None):
         # TODO(jelmer): Split out the perl-specific stuff?
@@ -1144,7 +1167,7 @@ class Maven(BuildSystem):
     def build(self, session, resolver, fixers):
         run_with_build_fixers(session, ["mvn", "compile"], fixers)
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
         # TODO(jelmer): 'mvn generate-sources' creates a jar in target/.
         # is that what we need?
         raise NotImplementedError
@@ -1177,8 +1200,12 @@ class Cabal(BuildSystem):
     def test(self, session, resolver, fixers):
         self._run(session, ["test"], fixers)
 
-    def dist(self, session, resolver, fixers, quiet=False):
+    def dist(self, session, resolver, fixers, target_directory, quiet=False):
-        self._run(session, ["sdist"], fixers)
+        with DistCatcher([
+                session.external_path('dist-newstyle/sdist'),
+                session.external_path('dist')]) as dc:
+            self._run(session, ["sdist"], fixers)
+        return dc.copy_single(target_directory)
 
     @classmethod
     def probe(cls, path):

ognibuild/dist.py

@@ -27,7 +27,6 @@ import logging
 import os
 import shutil
 import sys
-import time
 from typing import Optional, List
 
 from debian.deb822 import Deb822
@@ -47,108 +46,19 @@ from .session import Session
 from .session.schroot import SchrootSession
 
 
-SUPPORTED_DIST_EXTENSIONS = [
+def run_dist(session, buildsystems, resolver, fixers, target_directory, quiet=False):
-    ".tar.gz",
-    ".tgz",
-    ".tar.bz2",
-    ".tar.xz",
-    ".tar.lzma",
-    ".tbz2",
-    ".tar",
-    ".zip",
-]
-
-
-def is_dist_file(fn):
-    for ext in SUPPORTED_DIST_EXTENSIONS:
-        if fn.endswith(ext):
-            return True
-    return False
-
-
-class DistNoTarball(Exception):
-    """Dist operation did not create a tarball."""
-
-
-def run_dist(session, buildsystems, resolver, fixers, quiet=False):
     # Some things want to write to the user's home directory,
     # e.g. pip caches in ~/.cache
     session.create_home()
 
     for buildsystem in buildsystems:
-        buildsystem.dist(session, resolver, fixers, quiet=quiet)
+        filename = buildsystem.dist(
-        return
+            session, resolver, fixers, target_directory, quiet=quiet)
+        return filename
 
     raise NoBuildToolsFound()
 
 
-class DistCatcher(object):
-    def __init__(self, directories):
-        self.directories = directories
-        self.files = []
-        self.existing_files = None
-        self.start_time = time.time()
-
-    @classmethod
-    def default(cls, directory):
-        return cls([
-            os.path.join(directory, 'dist'),
-            directory,
-            os.path.join(directory, '..')])
-
-    def __enter__(self):
-        self.existing_files = {}
-        for directory in self.directories:
-            try:
-                self.existing_files[directory] = {
-                    entry.name: entry for entry in os.scandir(directory)}
-            except FileNotFoundError:
-                self.existing_files[directory] = {}
-        return self
-
-    def find_files(self):
-        for directory in self.directories:
-            old_files = self.existing_files[directory]
-            possible_new = []
-            possible_updated = []
-            for entry in os.scandir(directory):
-                if not entry.is_file() or not is_dist_file(entry.name):
-                    continue
-                old_entry = old_files.get(entry.name)
-                if not old_entry:
-                    possible_new.append(entry)
-                    continue
-                if entry.stat().st_mtime < self.start_time:
-                    possible_updated.append(entry)
-                    continue
-            if len(possible_new) == 1:
-                fn = possible_new[0]
-                logging.info("Found new tarball %s in %s.", fn, directory)
-                self.files.append(os.path.join(directory, fn))
-                return entry.name
-            elif len(possible_new) > 1:
-                logging.warning(
-                    "Found multiple tarballs %r in %s.", possible_new, directory)
-                return
-
-            if len(possible_updated) == 1:
-                fn = possible_updated[0]
-                logging.info("Found updated tarball %s in %s.", fn, directory)
-                self.files.append(os.path.join(directory, fn))
-                return entry.name
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        self.find_files()
-        return False
-
-    def cleanup(self):
-        for path in self.files:
-            if os.path.isdir(path):
-                shutil.rmtree(path)
-            else:
-                os.unlink(path)
-
-
 def create_dist(
     session: Session,
     tree: Tree,
@@ -182,20 +92,8 @@ def create_dist(
         fixers.extend([
             GitIdentityFixer(session), SecretGpgKeyFixer(session)])
 
-    with DistCatcher.default(export_directory) as dc:
+    session.chdir(reldir)
-        session.chdir(reldir)
+    return run_dist(session, buildsystems, resolver, fixers, target_dir)
-        run_dist(session, buildsystems, resolver, fixers)
-
-    try:
-        for path in dc.files:
-            shutil.copy(path, target_dir)
-            return os.path.join(target_dir, os.path.basename(path))
-    finally:
-        if cleanup:
-            dc.cleanup()
-
-    logging.info("No tarball created :(")
-    raise DistNoTarball()
 
 
 def create_dist_schroot(
@@ -225,6 +123,7 @@ if __name__ == "__main__":
     import breezy.bzr  # noqa: F401
     import breezy.git  # noqa: F401
     from breezy.export import export
+    from .dist_catcher import DistNoTarball
 
     parser = argparse.ArgumentParser()
     parser.add_argument(

ognibuild/dist_catcher.py

@@ -0,0 +1,116 @@
+#!/usr/bin/python3
+# Copyright (C) 2020 Jelmer Vernooij <jelmer@jelmer.uk>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+import os
+import logging
+import shutil
+import time
+
+
+class DistNoTarball(Exception):
+    """Dist operation did not create a tarball."""
+
+
+SUPPORTED_DIST_EXTENSIONS = [
+    ".tar.gz",
+    ".tgz",
+    ".tar.bz2",
+    ".tar.xz",
+    ".tar.lzma",
+    ".tbz2",
+    ".tar",
+    ".zip",
+]
+
+
+def is_dist_file(fn):
+    for ext in SUPPORTED_DIST_EXTENSIONS:
+        if fn.endswith(ext):
+            return True
+    return False
+
+
+class DistCatcher(object):
+    def __init__(self, directories):
+        self.directories = [os.path.abspath(d) for d in directories]
+        self.files = []
+        self.existing_files = None
+        self.start_time = time.time()
+
+    @classmethod
+    def default(cls, directory):
+        return cls([
+            os.path.join(directory, 'dist'),
+            directory,
+            os.path.join(directory, '..')])
+
+    def __enter__(self):
+        self.existing_files = {}
+        for directory in self.directories:
+            try:
+                self.existing_files[directory] = {
+                    entry.name: entry for entry in os.scandir(directory)}
+            except FileNotFoundError:
+                self.existing_files[directory] = {}
+        return self
+
+    def find_files(self):
+        for directory in self.directories:
+            old_files = self.existing_files[directory]
+            possible_new = []
+            possible_updated = []
+            if not os.path.isdir(directory):
+                continue
+            for entry in os.scandir(directory):
+                if not entry.is_file() or not is_dist_file(entry.name):
+                    continue
+                old_entry = old_files.get(entry.name)
+                if not old_entry:
+                    possible_new.append(entry)
+                    continue
+                if entry.stat().st_mtime > self.start_time:
+                    possible_updated.append(entry)
+                    continue
+            if len(possible_new) == 1:
+                entry = possible_new[0]
+                logging.info("Found new tarball %s in %s.", entry.name, directory)
+                self.files.append(entry.path)
+                return entry.name
+            elif len(possible_new) > 1:
+                logging.warning(
+                    "Found multiple tarballs %r in %s.", possible_new, directory)
+                return
+
+            if len(possible_updated) == 1:
+                entry = possible_updated[0]
+                logging.info("Found updated tarball %s in %s.", entry.name, directory)
+                self.files.append(entry.path)
+                return entry.name
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self.find_files()
+        return False
+
+    def copy_single(self, target_dir):
+        for path in self.files:
+            try:
+                shutil.copy(path, target_dir)
+            except shutil.SameFileError:
+                pass
+            return os.path.basename(path)
+        logging.info("No tarball created :(")
+        raise DistNoTarball()