Process wheelhouse.txt holistically rather than per-layer

charmtools/build/builder.py

@@ -148,6 +148,7 @@ def __init__(self):
         self._top_layer = None
         self.hide_metrics = os.environ.get('CHARM_HIDE_METRICS', False)
         self.wheelhouse_overrides = None
+        self.wheelhouse_per_layer = False
         self._warned_home = False
 
     @property
@@ -904,11 +905,17 @@ def main(args=None):
                         "from the interface service.")
     parser.add_argument('-n', '--name',
                         help="Build a charm of 'name' from 'charm'")
-    parser.add_argument('-r', '--report', action="store_true",
+    parser.add_argument('-r', '--report', action="store_true", default=True,
                         help="Show post-build report of changes")
+    parser.add_argument('-R', '--no-report', action="store_false",
+                        dest='report', default=True,
+                        help="Don't show post-build report of changes")
     parser.add_argument('-w', '--wheelhouse-overrides', type=path,
                         help="Provide a wheelhouse.txt file with overrides "
                              "for the built wheelhouse")
+    parser.add_argument('-W', '--wheelhouse-per-layer', action="store_true",
+                        help="Deprecated: Use original wheelhouse processing "
+                             "method (see PR juju/charm-tools#569)")
     parser.add_argument('-v', '--verbose', action='store_true', default=False,
                         help="Increase output (same as -l DEBUG)")
     parser.add_argument('--debug', action='store_true',
@@ -932,6 +939,8 @@ def main(args=None):
                                    build.layer_index)
     LayerFetcher.NO_LOCAL_LAYERS = build.no_local_layers
 
+    WheelhouseTactic.per_layer = build.wheelhouse_per_layer
+
     configLogging(build)
 
     try:

charmtools/build/tactics.py

@@ -7,6 +7,7 @@
 from inspect import getargspec
 
 from path import Path as path
+from pkg_resources import Requirement
 from ruamel import yaml
 from charmtools import utils
 from charmtools.build.errors import BuildError
@@ -1036,11 +1037,13 @@ class WheelhouseTactic(ExactMatch, Tactic):
     kind = "dynamic"
     FILENAME = 'wheelhouse.txt'
     removed = []  # has to be class level to affect all tactics during signing
+    per_layer = False
 
     def __init__(self, *args, **kwargs):
         super(WheelhouseTactic, self).__init__(*args, **kwargs)
         self.tracked = []
         self.previous = []
+        self.lines = None
         self._venv = None
         self.purge_wheels = False
 
@@ -1051,14 +1054,48 @@ def __str__(self):
     def combine(self, existing):
         ""  # suppress inherited doc
         self.previous = existing.previous + [existing]
+        existing.read()
+        self.read()
+        new_pkgs = set()
+        for line in self.lines:
+            try:
+                req = Requirement.parse(line)
+                new_pkgs.add(req.project_name)
+            except ValueError:
+                pass  # ignore comments, blank lines, etc
+        existing_lines = []
+        for line in existing.lines:
+            try:
+                req = Requirement.parse(line)
+                # new explicit reqs will override existing ones
+                if req.project_name not in new_pkgs:
+                    existing_lines.append(line)
+                else:
+                    existing_lines.append('# {}  # overridden by {}'.format(
+                        line, self.layer.url))
+            except ValueError:
+                existing_lines.append(line)  # ignore comments, blank lines, &c
+        self.lines = existing_lines + self.lines
         return self
 
+    def read(self):
+        if self.lines is None:
+            src = path(self.entity)
+            if src.exists():
+                self.lines = (['# ' + self.layer.url] +
+                              src.lines(retain=False) +
+                              [''])
+            else:
+                self.lines = []
+
     def _add(self, wheelhouse, *reqs):
         with utils.tempdir(chdir=False) as temp_dir:
             # put in a temp dir first to ensure we track all of the files
             self._pip('download', '--no-binary', ':all:', '-d', temp_dir,
                       *reqs)
+            log.debug('Copying wheels:')
             for wheel in temp_dir.files():
+                log.debug('  ' + wheel.name)
                 dest = wheelhouse / wheel.basename()
                 if dest in self.tracked:
                     return
@@ -1076,9 +1113,12 @@ def _add(self, wheelhouse, *reqs):
     def _run_in_venv(self, *args):
         assert self._venv is not None
         # have to use bash to activate the venv properly first
-        return utils.Process(('bash', '-c', ' '.join(
+        res = utils.Process(('bash', '-c', ' '.join(
             ('.', self._venv / 'bin' / 'activate', ';') + args
-        ))).exit_on_error()()
+        )))()
+        if res.exit_code != 0:
+            raise BuildError(res.output)
+        return res
 
     def _pip(self, *args):
         return self._run_in_venv('pip3', *args)
@@ -1092,21 +1132,42 @@ def __call__(self):
             utils.Process(
                 ('virtualenv', '--python', 'python3', self._venv)
             ).exit_on_error()()
+        if self.per_layer:
+            self._process_per_layer(wheelhouse)
+        else:
+            self._process_combined(wheelhouse)
+        # clean up
+        if create_venv:
+            self._venv.rmtree_p()
+            self._venv = None
+
+    def _process_per_layer(self, wheelhouse):
         # we are the top layer; process all lower layers first
         for tactic in self.previous:
             tactic()
         # process this layer
+        log.debug('Processing wheelhouse for {}'.format(self.layer.url))
         self._add(wheelhouse, '-r', self.entity)
-        # clean up
-        if create_venv:
-            self._venv.rmtree_p()
-            self._venv = None
+
+    def _process_combined(self, wheelhouse):
+        log.debug('Processing wheelhouse:')
+        self.read()
+        for line in self.lines:
+            log.debug('  ' + line.strip())
+        with utils.tempdir(chdir=False) as temp_dir:
+            wh_file = temp_dir / 'wheelhouse.txt'
+            wh_file.write_lines(self.lines)
+            self._add(wheelhouse, '-r', wh_file)
+            wh_file.move(self.target.directory / 'wheelhouse.txt')
 
     def sign(self):
         ""  # suppress inherited doc
         sigs = {}
-        for tactic in self.previous:
-            sigs.update(tactic.sign())
+        if self.per_layer:
+            for tactic in self.previous:
+                sigs.update(tactic.sign())
+        else:
+            sigs.update(super().sign())
         for d in self.tracked:
             if d in self.removed:
                 continue

tests/test_build.py

@@ -434,6 +434,7 @@ def test_wheelhouse(self, Process, mkdtemp, rmtree_p, ph, pi, pv):
         bu.hide_metrics = True
         bu.report = False
         bu.wheelhouse_overrides = self.dirname / 'wh-over.txt'
+        Process.return_value.return_value.exit_code = 0
 
         # remove the sign phase
         bu.PHASES = bu.PHASES[:-2]