From c978a88653fa74396dc38df6c3707fbe7c49af30 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 7 Aug 2014 02:58:53 +0900 Subject: [PATCH] Bug 1049281 - Remove static dirs handling, now that there aren't any. r=gps Also do some overdue cleanup from the removal of parallel and tools dirs --- config/makefiles/debugmake.mk | 2 +- config/recurse.mk | 3 +- config/rules.mk | 2 +- .../mozbuild/backend/recursivemake.py | 41 ++++------------ python/mozbuild/mozbuild/frontend/data.py | 2 - python/mozbuild/mozbuild/frontend/emitter.py | 1 - python/mozbuild/mozbuild/frontend/reader.py | 12 ++--- .../mozbuild/frontend/sandbox_symbols.py | 8 +--- .../test/backend/test_recursivemake.py | 47 +++++++++++-------- .../data/traversal-tier-simple/moz.build | 4 +- .../mozbuild/test/frontend/test_emitter.py | 6 +-- .../mozbuild/test/frontend/test_reader.py | 2 +- .../mozbuild/test/frontend/test_sandbox.py | 20 +++----- 13 files changed, 56 insertions(+), 94 deletions(-) diff --git a/config/makefiles/debugmake.mk b/config/makefiles/debugmake.mk index 2c33a3052964..a092738ac545 100644 --- a/config/makefiles/debugmake.mk +++ b/config/makefiles/debugmake.mk @@ -29,7 +29,7 @@ echo-tiers: @echo $(TIERS) echo-tier-dirs: - @$(foreach tier,$(TIERS),echo '$(tier):'; echo ' dirs: $(tier_$(tier)_dirs)'; $(if $(tier_$(tier)_staticdirs),echo ' staticdirs: $(tier_$(tier)_staticdirs)';) ) + @$(foreach tier,$(TIERS),echo '$(tier):'; echo ' dirs: $(tier_$(tier)_dirs)') echo-dirs: @echo $(call shell_quote,$(DIRS)) diff --git a/config/recurse.mk b/config/recurse.mk index 117ae7638535..13bf017e3f96 100644 --- a/config/recurse.mk +++ b/config/recurse.mk @@ -82,7 +82,7 @@ ifeq ($(CURRENT_TIER),compile) # https://savannah.gnu.org/bugs/index.php?42833 .PHONY: $(compile_targets) $(compile_targets): - $(call SUBMAKE,$(if $(filter $(@D),$(staticdirs)),,$(@F)),$(@D)) + $(call SUBMAKE,$(@F),$(@D)) else @@ -135,7 +135,6 @@ ifdef TIERS libs export tools:: $(call BUILDSTATUS,TIER_START $@) $(foreach tier,$(TIERS), $(if $(filter-out libs_precompile tools_precompile,$@_$(tier)), \ - $(if $(filter libs,$@),$(foreach dir, $(tier_$(tier)_staticdirs), $(call TIER_DIR_SUBMAKE,$@,$(tier),$(dir),,1))) \ $(foreach dir, $(tier_$(tier)_dirs), $(call TIER_DIR_SUBMAKE,$@,$(tier),$(dir),$@)))) $(call BUILDSTATUS,TIER_FINISH $@) diff --git a/config/rules.mk b/config/rules.mk index 3e464a61d22e..bfcf205e5010 100644 --- a/config/rules.mk +++ b/config/rules.mk @@ -649,7 +649,7 @@ clean clobber realclean clobber_all:: ifdef TIERS clean clobber realclean clobber_all distclean:: $(foreach dir, \ - $(foreach tier, $(TIERS), $(tier_$(tier)_staticdirs) $(tier_$(tier)_dirs)), \ + $(foreach tier, $(TIERS), $(tier_$(tier)_dirs)), \ -$(call SUBMAKE,$@,$(dir))) else clean clobber realclean clobber_all distclean:: diff --git a/python/mozbuild/mozbuild/backend/recursivemake.py b/python/mozbuild/mozbuild/backend/recursivemake.py index 9caa179fe0fa..2af17c303d03 100644 --- a/python/mozbuild/mozbuild/backend/recursivemake.py +++ b/python/mozbuild/mozbuild/backend/recursivemake.py @@ -141,38 +141,26 @@ class RecursiveMakeTraversal(object): from Makefiles. Each directory may have one or more types of subdirectories: - - parallel - - static - (normal) dirs - tests - - The "traditional" recursive make backend recurses through those by first - building the current directory, followed by parallel directories (in - parallel), then static directories, dirs, tests and tools (all - sequentially). """ - SubDirectoryCategories = ['parallel', 'static', 'dirs', 'tests'] + SubDirectoryCategories = ['dirs', 'tests'] SubDirectoriesTuple = namedtuple('SubDirectories', SubDirectoryCategories) class SubDirectories(SubDirectoriesTuple): def __new__(self): - return RecursiveMakeTraversal.SubDirectoriesTuple.__new__(self, [], [], [], []) + return RecursiveMakeTraversal.SubDirectoriesTuple.__new__(self, [], []) def __init__(self): self._traversal = {} - def add(self, dir, **kargs): + def add(self, dir, dirs=[], tests=[]): """ - Function signature is, in fact: - def add(self, dir, parallel=[], static=[], dirs=[], - tests=[], tools=[]) - but it's done with **kargs to avoid repetitive code. - Adds a directory to traversal, registering its subdirectories, sorted by categories. If the directory was already added to traversal, adds the new subdirectories to the already known lists. """ subdirs = self._traversal.setdefault(dir, self.SubDirectories()) - for key, value in kargs.items(): + for key, value in (('dirs', dirs), ('tests', tests)): assert(key in self.SubDirectoryCategories) getattr(subdirs, key).extend(value) @@ -181,8 +169,7 @@ class RecursiveMakeTraversal(object): """ Default filter for use with compute_dependencies and traverse. """ - return current, subdirs.parallel, \ - subdirs.static + subdirs.dirs + subdirs.tests + return current, [], subdirs.dirs + subdirs.tests def call_filter(self, current, filter): """ @@ -512,8 +499,7 @@ class RecursiveMakeBackend(CommonBackend): # Traverse directories in parallel, and skip static dirs def parallel_filter(current, subdirs): - all_subdirs = subdirs.parallel + subdirs.dirs + \ - subdirs.tests + all_subdirs = subdirs.dirs + subdirs.tests if current in self._may_skip[tier] \ or current.startswith('subtiers/'): current = None @@ -526,8 +512,7 @@ class RecursiveMakeBackend(CommonBackend): if current in self._may_skip['libs'] \ or current.startswith('subtiers/'): current = None - return current, [], subdirs.parallel + \ - subdirs.dirs + subdirs.tests + return current, [], subdirs.dirs + subdirs.tests # Because of bug 925236 and possible other unknown race conditions, # don't parallelize the tools tier. There aren't many directories for @@ -536,8 +521,7 @@ class RecursiveMakeBackend(CommonBackend): if current not in self._no_skip['tools'] \ or current.startswith('subtiers/'): current = None - return current, [], subdirs.parallel + \ - subdirs.dirs + subdirs.tests + return current, [], subdirs.dirs + subdirs.tests filters = [ ('export', parallel_filter), @@ -828,16 +812,9 @@ class RecursiveMakeBackend(CommonBackend): self._traversal.add('subtiers/%s' % tier, dirs=relativize(dirs)) - # tier_static_dirs should have the same keys as tier_dirs. - if obj.tier_static_dirs[tier]: - fh.write('staticdirs += %s\n' % ( - ' '.join(obj.tier_static_dirs[tier]))) - self._traversal.add('subtiers/%s' % tier, - static=relativize(obj.tier_static_dirs[tier])) - self._traversal.add('', dirs=['subtiers/%s' % tier]) - for d in dirs + obj.tier_static_dirs[tier]: + for d in dirs: if d.trigger: self._triggers[d.trigger].add('%s/target' % d) diff --git a/python/mozbuild/mozbuild/frontend/data.py b/python/mozbuild/mozbuild/frontend/data.py index 6b2d662a5a39..9858a1437c25 100644 --- a/python/mozbuild/mozbuild/frontend/data.py +++ b/python/mozbuild/mozbuild/frontend/data.py @@ -104,7 +104,6 @@ class DirectoryTraversal(SandboxDerived): 'dirs', 'test_dirs', 'tier_dirs', - 'tier_static_dirs', ) def __init__(self, sandbox): @@ -113,7 +112,6 @@ class DirectoryTraversal(SandboxDerived): self.dirs = [] self.test_dirs = [] self.tier_dirs = OrderedDict() - self.tier_static_dirs = OrderedDict() class BaseConfigSubstitution(SandboxDerived): diff --git a/python/mozbuild/mozbuild/frontend/emitter.py b/python/mozbuild/mozbuild/frontend/emitter.py index b63b7e28e582..13c8dc68ad85 100644 --- a/python/mozbuild/mozbuild/frontend/emitter.py +++ b/python/mozbuild/mozbuild/frontend/emitter.py @@ -949,6 +949,5 @@ class TreeMetadataEmitter(LoggingMixin): for tier in sandbox['TIERS']: o.tier_dirs[tier] = sandbox['TIERS'][tier]['regular'] + \ sandbox['TIERS'][tier]['external'] - o.tier_static_dirs[tier] = sandbox['TIERS'][tier]['static'] yield o diff --git a/python/mozbuild/mozbuild/frontend/reader.py b/python/mozbuild/mozbuild/frontend/reader.py index d6708c1d3f2e..44019976f8f8 100644 --- a/python/mozbuild/mozbuild/frontend/reader.py +++ b/python/mozbuild/mozbuild/frontend/reader.py @@ -281,8 +281,7 @@ class MozbuildSandbox(Sandbox): data.is_library = True return data - def _add_tier_directory(self, tier, reldir, static=False, external=False, - trigger=None): + def _add_tier_directory(self, tier, reldir, external=False, trigger=None): """Register a tier directory with the build.""" if isinstance(reldir, text_type): reldir = [reldir] @@ -290,15 +289,10 @@ class MozbuildSandbox(Sandbox): if not tier in self['TIERS']: self['TIERS'][tier] = { 'regular': [], - 'static': [], 'external': [], } - key = 'static' if static else 'external' if external else 'regular' - if external and static: - raise Exception('Only one of external or static can be set at the ' - 'same time') - + key = 'external' if external else 'regular' for path in reldir: if path in self['TIERS'][tier][key]: raise Exception('Directory has already been registered with ' @@ -877,7 +871,7 @@ class BuildReader(object): 'TIERS defined but it should not be', sandbox) for tier, values in sandbox['TIERS'].items(): - # We don't descend into static directories because static by + # We don't descend into external directories because external by # definition is external to the build system. for d in values['regular']: if d in recurse_info: diff --git a/python/mozbuild/mozbuild/frontend/sandbox_symbols.py b/python/mozbuild/mozbuild/frontend/sandbox_symbols.py index d166226feaa9..803e75e7545a 100644 --- a/python/mozbuild/mozbuild/frontend/sandbox_symbols.py +++ b/python/mozbuild/mozbuild/frontend/sandbox_symbols.py @@ -898,12 +898,8 @@ FUNCTIONS = { add_tier_dir('app', ['components', 'base']) - Register a directory as having static content (no dependencies):: - - add_tier_dir('base', 'foo', static=True) - - Register a directory as having external content (same as static - content, but traversed with export, libs, and tools subtiers:: + Register a directory as having external content (no dependencies, + and traversed with export, libs, and tools subtiers:: add_tier_dir('base', 'bar', external=True) diff --git a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py index 868054dfac58..d7f21d5149fe 100644 --- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py +++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py @@ -32,21 +32,21 @@ class TestRecursiveMakeTraversal(unittest.TestCase): traversal.add('', dirs=['D']) traversal.add('A') traversal.add('B', dirs=['E', 'F']) - traversal.add('C', parallel=['G', 'H']) - traversal.add('D', parallel=['I'], dirs=['K']) - traversal.add('D', parallel=['J'], dirs=['L']) + traversal.add('C', dirs=['G', 'H']) + traversal.add('D', dirs=['I', 'K']) + traversal.add('D', dirs=['J', 'L']) traversal.add('E') traversal.add('F') traversal.add('G') traversal.add('H') traversal.add('I', dirs=['M', 'N']) - traversal.add('J', parallel=['O', 'P']) - traversal.add('K', parallel=['Q', 'R']) + traversal.add('J', dirs=['O', 'P']) + traversal.add('K', dirs=['Q', 'R']) traversal.add('L', dirs=['S']) traversal.add('M') traversal.add('N', dirs=['T']) traversal.add('O') - traversal.add('P', parallel=['U']) + traversal.add('P', dirs=['U']) traversal.add('Q') traversal.add('R', dirs=['V']) traversal.add('S', dirs=['W']) @@ -56,8 +56,14 @@ class TestRecursiveMakeTraversal(unittest.TestCase): traversal.add('W', dirs=['X']) traversal.add('X') - start, deps = traversal.compute_dependencies() + parallels = set(('G', 'H', 'I', 'J', 'O', 'P', 'Q', 'R', 'U')) + def filter(current, subdirs): + return (current, [d for d in subdirs.dirs if d in parallels], + [d for d in subdirs.dirs if d not in parallels]) + + start, deps = traversal.compute_dependencies(filter) self.assertEqual(start, ('X',)) + self.maxDiff = None self.assertEqual(deps, { 'A': ('',), 'B': ('A',), @@ -85,21 +91,21 @@ class TestRecursiveMakeTraversal(unittest.TestCase): 'X': ('W',), }) - self.assertEqual(list(traversal.traverse('')), + self.assertEqual(list(traversal.traverse('', filter)), ['', 'A', 'B', 'E', 'F', 'C', 'G', 'H', 'D', 'I', 'M', 'N', 'T', 'J', 'O', 'P', 'U', 'K', 'Q', 'R', 'V', 'L', 'S', 'W', 'X']) - self.assertEqual(list(traversal.traverse('C')), + self.assertEqual(list(traversal.traverse('C', filter)), ['C', 'G', 'H']) def test_traversal_2(self): traversal = RecursiveMakeTraversal() traversal.add('', dirs=['A', 'B', 'C']) traversal.add('A') - traversal.add('B', static=['D'], dirs=['E', 'F']) - traversal.add('C', parallel=['G', 'H'], dirs=['I']) - # Don't register D + traversal.add('B', dirs=['D', 'E', 'F']) + traversal.add('C', dirs=['G', 'H', 'I']) + traversal.add('D') traversal.add('E') traversal.add('F') traversal.add('G') @@ -116,16 +122,16 @@ class TestRecursiveMakeTraversal(unittest.TestCase): 'E': ('D',), 'F': ('E',), 'G': ('C',), - 'H': ('C',), - 'I': ('G', 'H'), + 'H': ('G',), + 'I': ('H',), }) def test_traversal_filter(self): traversal = RecursiveMakeTraversal() traversal.add('', dirs=['A', 'B', 'C']) traversal.add('A') - traversal.add('B', static=['D'], dirs=['E', 'F']) - traversal.add('C', parallel=['G', 'H'], dirs=['I']) + traversal.add('B', dirs=['D', 'E', 'F']) + traversal.add('C', dirs=['G', 'H', 'I']) traversal.add('D') traversal.add('E') traversal.add('F') @@ -136,18 +142,19 @@ class TestRecursiveMakeTraversal(unittest.TestCase): def filter(current, subdirs): if current == 'B': current = None - return current, subdirs.parallel, subdirs.dirs + return current, [], subdirs.dirs start, deps = traversal.compute_dependencies(filter) self.assertEqual(start, ('I',)) self.assertEqual(deps, { 'A': ('',), 'C': ('F',), - 'E': ('A',), + 'D': ('A',), + 'E': ('D',), 'F': ('E',), 'G': ('C',), - 'H': ('C',), - 'I': ('G', 'H'), + 'H': ('G',), + 'I': ('H',), }) class TestRecursiveMakeBackend(BackendTester): diff --git a/python/mozbuild/mozbuild/test/frontend/data/traversal-tier-simple/moz.build b/python/mozbuild/mozbuild/test/frontend/data/traversal-tier-simple/moz.build index e971685d6c73..be8d23fd7ed9 100644 --- a/python/mozbuild/mozbuild/test/frontend/data/traversal-tier-simple/moz.build +++ b/python/mozbuild/mozbuild/test/frontend/data/traversal-tier-simple/moz.build @@ -3,7 +3,7 @@ # http://creativecommons.org/publicdomain/zero/1.0/ add_tier_dir('t1', 'foo') -add_tier_dir('t1', 'foo_static', static=True) +add_tier_dir('t1', 'foo_static') add_tier_dir('t2', 'bar') -add_tier_dir('t3', 'baz', static=True) +add_tier_dir('t3', 'baz') diff --git a/python/mozbuild/mozbuild/test/frontend/test_emitter.py b/python/mozbuild/mozbuild/test/frontend/test_emitter.py index 8f277a7165de..adef5d4ae19e 100644 --- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py +++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py @@ -89,7 +89,6 @@ class TestEmitterBasic(unittest.TestCase): self.assertIsInstance(o, DirectoryTraversal) self.assertEqual(o.test_dirs, []) self.assertEqual(len(o.tier_dirs), 0) - self.assertEqual(len(o.tier_static_dirs), 0) self.assertTrue(os.path.isabs(o.sandbox_main_path)) self.assertEqual(len(o.sandbox_all_paths), 1) @@ -120,10 +119,11 @@ class TestEmitterBasic(unittest.TestCase): def test_tier_simple(self): reader = self.reader('traversal-tier-simple') objs = self.read_topsrcdir(reader, filter_common=False) - self.assertEqual(len(objs), 4) + self.assertEqual(len(objs), 6) reldirs = [o.relativedir for o in objs] - self.assertEqual(reldirs, ['', 'foo', 'foo/biz', 'bar']) + self.assertEqual(reldirs, ['', 'foo', 'foo/biz', 'foo_static', 'bar', + 'baz']) def test_config_file_substitution(self): reader = self.reader('config-file-substitution') diff --git a/python/mozbuild/mozbuild/test/frontend/test_reader.py b/python/mozbuild/mozbuild/test/frontend/test_reader.py index 827f0047b0c6..a7bb2e22ece8 100644 --- a/python/mozbuild/mozbuild/test/frontend/test_reader.py +++ b/python/mozbuild/mozbuild/test/frontend/test_reader.py @@ -72,7 +72,7 @@ class TestBuildReader(unittest.TestCase): reader = self.reader('traversal-tier-simple') sandboxes = list(reader.read_topsrcdir()) - self.assertEqual(len(sandboxes), 4) + self.assertEqual(len(sandboxes), 6) for sandbox in sandboxes: self.assertIsInstance(sandbox.metadata, dict) diff --git a/python/mozbuild/mozbuild/test/frontend/test_sandbox.py b/python/mozbuild/mozbuild/test/frontend/test_sandbox.py index 4bca7010099f..e482976d677a 100644 --- a/python/mozbuild/mozbuild/test/frontend/test_sandbox.py +++ b/python/mozbuild/mozbuild/test/frontend/test_sandbox.py @@ -170,7 +170,7 @@ class TestSandbox(unittest.TestCase): sandbox.exec_source('add_tier_dir("t1", "foo")', 'foo.py') self.assertEqual(sandbox['TIERS']['t1'], - {'regular': ['foo'], 'static': [], 'external': []}) + {'regular': ['foo'], 'external': []}) def test_add_tier_dir_regular_list(self): sandbox = self.sandbox() @@ -178,23 +178,15 @@ class TestSandbox(unittest.TestCase): sandbox.exec_source('add_tier_dir("t1", ["foo", "bar"])', 'foo.py') self.assertEqual(sandbox['TIERS']['t1'], - {'regular': ['foo', 'bar'], 'static': [], 'external': []}) + {'regular': ['foo', 'bar'], 'external': []}) - def test_add_tier_dir_static(self): - sandbox = self.sandbox() - - sandbox.exec_source('add_tier_dir("t1", "foo", static=True)', 'foo.py') - - self.assertEqual(sandbox['TIERS']['t1'], - {'regular': [], 'static': ['foo'], 'external': []}) - - def test_add_tier_dir_static(self): + def test_add_tier_dir_external(self): sandbox = self.sandbox() sandbox.exec_source('add_tier_dir("t1", "foo", external=True)', 'foo.py') self.assertEqual(sandbox['TIERS']['t1'], - {'regular': [], 'static': [], 'external': ['foo']}) + {'regular': [], 'external': ['foo']}) def test_tier_order(self): sandbox = self.sandbox() @@ -202,9 +194,9 @@ class TestSandbox(unittest.TestCase): source = ''' add_tier_dir('t1', 'foo') add_tier_dir('t1', 'bar') -add_tier_dir('t2', 'baz', static=True) +add_tier_dir('t2', 'baz') add_tier_dir('t3', 'biz') -add_tier_dir('t1', 'bat', static=True) +add_tier_dir('t1', 'bat') ''' sandbox.exec_source(source, 'foo.py')