From c161aa945c811b260f7f2981e592f63d16c3a106 Mon Sep 17 00:00:00 2001 From: dpranke Date: Mon, 14 Sep 2015 13:21:13 -0700 Subject: [PATCH] Make MB clobber build directories when switching between GYP and GN. GYP and GN put generated files in different places inside the build directory. This means that if you do a GYP build and then a GN build into the same directory, you may get unpredictable results depending on which generated files are picked up at what times. In order to avoid this, we modify MB to keep track of whether a given build directory has build files generated by GYP or GN; if there is a mismatch (like when we flip a bot from GYP to GN), we clobber the build directory and start from scratch to be safe. In addition, the first time we enable MB on a bot, we will not know what the existing build directory contains; to be safe, we do a clobber in this situation as well. R=brettw@chromium.org BUG= Review URL: https://codereview.chromium.org/1338123002 Cr-Commit-Position: refs/heads/master@{#348705} --- tools/mb/mb.py | 43 ++++++++++++++++++++++++++++++++++++++++++- tools/mb/mb_unittest.py | 43 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/tools/mb/mb.py b/tools/mb/mb.py index f1a223ea373a..57054e9ea3fb 100755 --- a/tools/mb/mb.py +++ b/tools/mb/mb.py @@ -135,6 +135,9 @@ class MetaBuildWrapper(object): def CmdGen(self): vals = self.GetConfig() + + self.ClobberIfNeeded(vals) + if vals['type'] == 'gn': return self.RunGNGen(vals) if vals['type'] == 'gyp': @@ -290,7 +293,7 @@ class MetaBuildWrapper(object): 'type': None, 'gn_args': [], 'gyp_config': [], - 'gyp_defines': [], + 'gyp_defines': '', 'gyp_crosscompile': False, } @@ -328,6 +331,33 @@ class MetaBuildWrapper(object): self.FlattenMixins(mixin_vals['mixins'], vals, visited) return vals + def ClobberIfNeeded(self, vals): + path = self.args.path[0] + build_dir = self.ToAbsPath(path) + mb_type_path = os.path.join(build_dir, 'mb_type') + needs_clobber = False + new_mb_type = vals['type'] + if self.Exists(build_dir): + if self.Exists(mb_type_path): + old_mb_type = self.ReadFile(mb_type_path) + if old_mb_type != new_mb_type: + self.Print("Build type mismatch: was %s, will be %s, clobbering %s" % + (old_mb_type, new_mb_type, path)) + needs_clobber = True + else: + # There is no 'mb_type' file in the build directory, so this probably + # means that the prior build(s) were not done through mb, and we + # have no idea if this was a GYP build or a GN build. Clobber it + # to be safe. + self.Print("%s/mb_type missing, clobbering to be safe" % path) + needs_clobber = True + + if needs_clobber: + self.RemoveDirectory(build_dir) + + self.MaybeMakeDirectory(build_dir) + self.WriteFile(mb_type_path, new_mb_type) + def RunGNGen(self, vals): path = self.args.path[0] @@ -821,6 +851,17 @@ class MetaBuildWrapper(object): # This function largely exists so it can be overriden for testing. os.remove(path) + def RemoveDirectory(self, abs_path): + if sys.platform == 'win32': + # In other places in chromium, we often have to retry this command + # because we're worried about other processes still holding on to + # file handles, but when MB is invoked, it will be early enough in the + # build that their should be no other processes to interfere. We + # can change this if need be. + self.Run(['cmd.exe', '/c', 'rmdir', '/q', '/s', abs_path]) + else: + shutil.rmtree(abs_path, ignore_errors=True) + def TempFile(self, mode='w'): # This function largely exists so it can be overriden for testing. return tempfile.NamedTemporaryFile(mode=mode, delete=False) diff --git a/tools/mb/mb_unittest.py b/tools/mb/mb_unittest.py index 942defb9f98c..e762b8d206d5 100755 --- a/tools/mb/mb_unittest.py +++ b/tools/mb/mb_unittest.py @@ -25,6 +25,7 @@ class FakeMBW(mb.MetaBuildWrapper): self.platform = 'linux2' self.chromium_src_dir = '/fake_src' self.default_config = '/fake_src/tools/mb/mb_config.pyl' + self.rmdirs = [] def ExpandUser(self, path): return '$HOME/%s' % path @@ -33,7 +34,7 @@ class FakeMBW(mb.MetaBuildWrapper): return self.files.get(path) is not None def MaybeMakeDirectory(self, path): - pass + self.files[path] = True def ReadFile(self, path): return self.files[path] @@ -64,6 +65,12 @@ class FakeMBW(mb.MetaBuildWrapper): def RemoveFile(self, path): del self.files[path] + def RemoveDirectory(self, path): + self.rmdirs.append(path) + files_to_delete = [f for f in self.files if f.startswith(path)] + for f in files_to_delete: + self.files[f] = None + class FakeFile(object): def __init__(self, files): @@ -84,6 +91,7 @@ TEST_CONFIG = """\ 'configs': { 'gyp_rel_bot': ['gyp', 'rel', 'goma'], 'gn_debug': ['gn', 'debug'], + 'gyp_debug': ['gyp', 'debug'], 'gn_rel_bot': ['gn', 'rel', 'goma'], 'private': ['gyp', 'rel', 'fake_feature1'], 'unsupported': ['gn', 'fake_feature2'], @@ -92,6 +100,7 @@ TEST_CONFIG = """\ 'fake_master': { 'fake_builder': 'gyp_rel_bot', 'fake_gn_builder': 'gn_rel_bot', + 'fake_gyp_builder': 'gyp_debug', }, }, 'mixins': { @@ -116,6 +125,7 @@ TEST_CONFIG = """\ }, 'debug': { 'gn_args': 'is_debug=true', + 'gyp_config': 'Debug', }, }, 'private_configs': ['private'], @@ -146,6 +156,37 @@ class UnitTest(unittest.TestCase): self.assertEqual(mbw.err, err) return mbw + def test_clobber(self): + files = { + '/fake_src/out/Debug': None, + '/fake_src/out/Debug/mb_type': None, + } + mbw = self.fake_mbw(files) + + # The first time we run this, the build dir doesn't exist, so no clobber. + self.check(['gen', '-c', 'gn_debug', '//out/Debug'], mbw=mbw, ret=0) + self.assertEqual(mbw.rmdirs, []) + self.assertTrue(mbw.files['/fake_src/out/Debug/mb_type'], 'gn') + + # The second time we run this, the build dir exists and matches, so no + # clobber. + self.check(['gen', '-c', 'gn_debug', '//out/Debug'], mbw=mbw, ret=0) + self.assertEqual(mbw.rmdirs, []) + self.assertEqual(mbw.files['/fake_src/out/Debug/mb_type'], 'gn') + + # Now we switch build types; this should result in a clobber. + self.check(['gen', '-c', 'gyp_debug', '//out/Debug'], mbw=mbw, ret=0) + self.assertEqual(mbw.rmdirs, ['/fake_src/out/Debug']) + self.assertEqual(mbw.files['/fake_src/out/Debug/mb_type'], 'gyp') + + # Now we delete mb_type; this checks the case where the build dir + # exists but wasn't populated by mb; this should also result in a clobber. + del mbw.files['/fake_src/out/Debug/mb_type'] + self.check(['gen', '-c', 'gyp_debug', '//out/Debug'], mbw=mbw, ret=0) + self.assertEqual(mbw.rmdirs, + ['/fake_src/out/Debug', '/fake_src/out/Debug']) + self.assertEqual(mbw.files['/fake_src/out/Debug/mb_type'], 'gyp') + def test_gn_analyze(self): files = {'/tmp/in.json': """{\ "files": ["foo/foo_unittest.cc"], -- 2.11.4.GIT