Bug 1228444 - Make DIST_FILES a HierarchicalStringList, like FINAL_TARGET_FILES. r=gps

config/rules.mk

@@ -1268,11 +1268,6 @@ $(error $(srcdir) contains a jar.mn file but this file is not declared in a JAR_
 endif
 endif
 
-ifneq ($(DIST_FILES),)
-DIST_FILES_PATH := $(FINAL_TARGET)
-PP_TARGETS += DIST_FILES
-endif
-
 # When you move this out of the tools tier, please remove the corresponding
 # hacks in recursivemake.py that check if Makefile.in sets the variable.
 ifneq ($(XPI_PKGNAME),)

python/mozbuild/mozbuild/backend/fastermake.py

@@ -150,20 +150,17 @@ class FasterMakeBackend(CommonBackend):
                             mozpath.join(base, mozpath.basename(f))
                         )
 
-        elif isinstance(obj, FinalTargetFiles) and \
+        elif isinstance(obj, (FinalTargetFiles, DistFiles)) and \
                 obj.install_target.startswith('dist/bin'):
             for path, strings in obj.files.walk():
-                base = mozpath.join(obj.install_target, path)
                 for f in strings:
-                    self._install_manifests[obj.install_target].add_symlink(
-                        mozpath.join(obj.srcdir, f),
-                        mozpath.join(path, mozpath.basename(f))
-                    )
-
-        elif isinstance(obj, DistFiles) and \
-                obj.install_target.startswith('dist/bin'):
-            for f in obj.files:
-                self._add_preprocess(obj, f, '', defines=defines)
+                    if isinstance(obj, DistFiles):
+                        self._add_preprocess(obj, f, path, defines=defines)
+                    else:
+                        self._install_manifests[obj.install_target].add_symlink(
+                            mozpath.join(obj.srcdir, f),
+                            mozpath.join(path, mozpath.basename(f))
+                        )
 
         elif isinstance(obj, ChromeManifestEntry) and \
                 obj.install_target.startswith('dist/bin'):

python/mozbuild/mozbuild/backend/recursivemake.py

@@ -605,12 +605,7 @@ class RecursiveMakeBackend(CommonBackend):
             self._process_final_target_files(obj, obj.files, obj.target)
 
         elif isinstance(obj, DistFiles):
-            # We'd like to install these via manifests as preprocessed files.
-            # But they currently depend on non-standard flags being added via
-            # some Makefiles, so for now we just pass them through to the
-            # underlying Makefile.in.
-            for f in obj.files:
-                backend_file.write('DIST_FILES += %s\n' % f)
+            self._process_dist_files(obj, obj.files, obj.target, backend_file)
 
         elif isinstance(obj, AndroidResDirs):
             # Order matters.
@@ -1395,6 +1390,18 @@ INSTALL_TARGETS += %(prefix)s
                 dest = mozpath.join(reltarget, path, mozpath.basename(f))
                 install_manifest.add_symlink(source, dest)
 
+    def _process_dist_files(self, obj, files, target, backend_file):
+        # We'd like to install these via manifests as preprocessed files.
+        # But they currently depend on non-standard flags being added via
+        # some Makefiles, so for now we just pass them through to the
+        # underlying Makefile.in.
+        for i, (path, strings) in enumerate(files.walk()):
+            for f in strings:
+                backend_file.write('DIST_FILES_%d += %s\n' % (i, f))
+            backend_file.write('DIST_FILES_%d_PATH := $(DEPTH)/%s\n'
+                               % (i, mozpath.join(target, path)))
+            backend_file.write('PP_TARGETS += DIST_FILES_%d\n' % i)
+
     def _process_chrome_manifest_entry(self, obj, backend_file):
         fragment = Makefile()
         rule = fragment.create_rule(targets=['misc:'])

python/mozbuild/mozbuild/frontend/context.py

@@ -997,10 +997,8 @@ VARIABLES = {
         disabled.
         """, None),
 
-    'DIST_FILES': (StrictOrderingOnAppendList, list,
-        """Additional files to place in ``FINAL_TARGET`` (typically ``dist/bin``).
-
-        Unlike ``FINAL_TARGET_FILES``, these files are preprocessed.
+    'DIST_FILES': (HierarchicalStringList, list,
+        """Like ``FINAL_TARGET_FILES``, with preprocessing.
         """, 'libs'),
 
     'EXTRA_COMPONENTS': (StrictOrderingOnAppendList, list,

python/mozbuild/mozbuild/frontend/data.py

@@ -856,8 +856,7 @@ class FinalTargetFiles(ContextDerived):
 
 
 class DistFiles(ContextDerived):
-    """Sandbox container object for FINAL_TARGET_FILES, which is a
-    HierarchicalStringList.
+    """Sandbox container object for DIST_FILES, which is a HierarchicalStringList.
 
     We need an object derived from ContextDerived for use in the backend, so
     this object fills that role. It just has a reference to the underlying

python/mozbuild/mozbuild/frontend/emitter.py

@@ -682,27 +682,22 @@ class TreeMetadataEmitter(LoggingMixin):
                     local_include.full_path), context)
             yield LocalInclude(context, local_include)
 
-        final_target_files = context.get('FINAL_TARGET_FILES')
-        if final_target_files:
-            for _, files in final_target_files.walk():
+        for var, cls in (
+            ('FINAL_TARGET_FILES', FinalTargetFiles),
+            ('DIST_FILES', DistFiles),
+        ):
+            all_files = context.get(var)
+            if not all_files:
+                continue
+            for _, files in all_files.walk():
                 for f in files:
                     path = os.path.join(context.srcdir, f)
                     if not os.path.exists(path):
                         raise SandboxValidationError(
-                            'File listed in FINAL_TARGET_FILES does not exist:'
-                            ' %s' % f, context)
+                            'File listed in %s does not exist: %s'
+                            % (var, f), context)
 
-            yield FinalTargetFiles(context, final_target_files, context['FINAL_TARGET'])
-
-        dist_files = context.get('DIST_FILES')
-        if dist_files:
-            for f in dist_files:
-                path = os.path.join(context.srcdir, f)
-                if not os.path.exists(path):
-                    raise SandboxValidationError('File listed in DIST_FILES '
-                        'does not exist: %s' % f, context)
-
-            yield DistFiles(context, dist_files, context['FINAL_TARGET'])
+            yield cls(context, all_files, context['FINAL_TARGET'])
 
         branding_files = context.get('BRANDING_FILES')
         if branding_files:

python/mozbuild/mozbuild/test/backend/test_recursivemake.py

@@ -673,11 +673,13 @@ class TestRecursiveMakeBackend(BackendTester):
         lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:]]
 
         expected = [
-            'DIST_FILES += install.rdf',
-            'DIST_FILES += main.js',
+            'DIST_FILES_0 += install.rdf',
+            'DIST_FILES_0 += main.js',
+            'DIST_FILES_0_PATH := $(DEPTH)/dist/bin/',
+            'PP_TARGETS += DIST_FILES_0',
         ]
 
-        found = [str for str in lines if str.startswith('DIST_FILES')]
+        found = [str for str in lines if 'DIST_FILES' in str]
         self.assertEqual(found, expected)
 
     def test_config(self):

python/mozbuild/mozbuild/test/frontend/test_emitter.py

@@ -895,11 +895,15 @@ class TestEmitterBasic(unittest.TestCase):
         self.assertEqual(len(objs), 1)
         self.assertIsInstance(objs[0], DistFiles)
 
-        self.assertEqual(len(objs[0].files), 2)
+        # Ideally we'd test hierarchies, but that would just be testing
+        # the HierarchicalStringList class, which we test separately.
+        for path, files in objs[0].files.walk():
+            self.assertEqual(path, '')
+            self.assertEqual(len(files), 2)
 
-        expected = {'install.rdf', 'main.js'}
-        for f in objs[0].files:
-            self.assertTrue(f in expected)
+            expected = {'install.rdf', 'main.js'}
+            for f in files:
+                self.assertTrue(f in expected)
 
     def test_missing_dist_files(self):
         """Test that DIST_FILES with missing files throws errors."""