diff options
author | Sam Thursfield <sam.thursfield@codethink.co.uk> | 2013-02-22 19:07:13 +0000 |
---|---|---|
committer | Sam Thursfield <sam.thursfield@codethink.co.uk> | 2013-03-13 15:20:02 +0000 |
commit | ece7f823de6bd61a0676edf71a9525697848824e (patch) | |
tree | 98c408d5960915f0cb5b98a941e2dc6508047f8b | |
parent | 6a61dd9cc1fe8a3ccd2128fb628ed929fd496ad0 (diff) | |
download | morph-ece7f823de6bd61a0676edf71a9525697848824e.tar.gz |
Refactor build process
Reorganise the build_artifact() and build_artifacts() functions to
allow more complex work when setting up chunk builds in
build_artifact().
The staging area now holds the BuildEnvironment object (the
environment variables that should be set during build). This makes sense
because all build commands should be run inside the staging area and
therefore through the StagingArea object.
The BuildEnvironment object is now considered immutable after it is
created. The environment is used in cache key computation when
computing what artifacts are required; if it changes after that point
we risk either computing different artifact keys for the same artifact
or missing data in the cache key that should be included in the hash.
Better to force changes into a separate 'extra_env' variable.
-rw-r--r-- | morphlib/artifact.py | 1 | ||||
-rw-r--r-- | morphlib/buildcommand.py | 92 | ||||
-rw-r--r-- | morphlib/buildenvironment.py | 20 | ||||
-rw-r--r-- | morphlib/buildenvironment_tests.py | 33 | ||||
-rw-r--r-- | morphlib/builder2.py | 21 | ||||
-rw-r--r-- | morphlib/builder2_tests.py | 9 | ||||
-rw-r--r-- | morphlib/cachekeycomputer.py | 7 | ||||
-rw-r--r-- | morphlib/cachekeycomputer_tests.py | 11 | ||||
-rw-r--r-- | morphlib/stagingarea.py | 52 | ||||
-rw-r--r-- | morphlib/stagingarea_tests.py | 17 | ||||
-rw-r--r-- | morphlib/util.py | 1 |
11 files changed, 130 insertions, 134 deletions
diff --git a/morphlib/artifact.py b/morphlib/artifact.py index aef48d76..5f084384 100644 --- a/morphlib/artifact.py +++ b/morphlib/artifact.py @@ -83,4 +83,3 @@ class Artifact(object): yield a return list(depth_first(self)) - diff --git a/morphlib/buildcommand.py b/morphlib/buildcommand.py index c7c650d3..56c7fae8 100644 --- a/morphlib/buildcommand.py +++ b/morphlib/buildcommand.py @@ -181,19 +181,38 @@ class BuildCommand(object): assert len(maybe) == 1 return maybe.pop() - def build_in_order(self, artifact): + def build_in_order(self, root_artifact): '''Build everything specified in a build order.''' - self.app.status(msg='Building according to build ordering', - chatty=True) - artifacts = artifact.walk() + self.app.status(msg='Building a set of artifacts', chatty=True) + artifacts = root_artifact.walk() old_prefix = self.app.status_prefix for i, a in enumerate(artifacts): self.app.status_prefix = ( old_prefix + '[Build %d/%d] ' % ((i+1), len(artifacts))) - self.build_artifact(a) + + self.app.status(msg='Checking if %(kind)s %(name)s needs building', + kind=a.source.morphology['kind'], name=a.name) + + if self.is_built(a): + self.app.status(msg='The %(kind)s %(name)s is already built', + kind=a.source.morphology['kind'], name=a.name) + self.cache_artifacts_locally([a]) + else: + self.app.status(msg='Building %(kind)s %(name)s', + kind=a.source.morphology['kind'], name=a.name) + self.build_artifact(a) + + self.app.status(msg='%(kind)s %(name)s is cached at %(cachepath)s', + kind=a.source.morphology['kind'], name=a.name, + cachepath=self.lac.artifact_filename(a), + chatty=(a.source.morphology['kind'] != "system")) self.app.status_prefix = old_prefix + def is_built(self, artifact): + '''Does either cache already have the artifact?''' + return self.lac.has(artifact) or (self.rac and self.rac.has(artifact)) + def build_artifact(self, artifact): '''Build one artifact. @@ -201,52 +220,20 @@ class BuildCommand(object): in either the local or remote cache already. ''' - - self.app.status(msg='Checking if %(kind)s %(name)s needs building', - kind=artifact.source.morphology['kind'], - name=artifact.name) - - if self.is_built(artifact): - self.app.status(msg='The %(kind)s %(name)s is already built', - kind=artifact.source.morphology['kind'], - name=artifact.name) - self.cache_artifacts_locally([artifact]) - else: - self.app.status(msg='Building %(kind)s %(name)s', - kind=artifact.source.morphology['kind'], - name=artifact.name) - self.get_sources(artifact) - deps = self.get_recursive_deps(artifact) - self.cache_artifacts_locally(deps) - staging_area = self.create_staging_area(artifact) - if artifact.source.morphology.needs_staging_area: - self.install_fillers(staging_area) - self.install_chunk_artifacts(staging_area, - deps, artifact) - morphlib.builder2.ldconfig(self.app.runcmd, - staging_area.dirname) - self.build_and_cache(staging_area, artifact) - self.remove_staging_area(staging_area) - self.app.status(msg='%(kind)s %(name)s is cached at %(cachepath)s', - kind=artifact.source.morphology['kind'], - name=artifact.name, - cachepath=self.lac.artifact_filename(artifact), - chatty=(artifact.source.morphology['kind'] != - "system")) - - def is_built(self, artifact): - '''Does either cache already have the artifact?''' - return self.lac.has(artifact) or (self.rac and self.rac.has(artifact)) + self.get_sources(artifact) + deps = self.get_recursive_deps(artifact) + self.cache_artifacts_locally(deps) + staging_area = self.create_staging_area() + if artifact.source.morphology.needs_staging_area: + self.install_fillers(staging_area) + self.install_chunk_artifacts(staging_area, deps, artifact) + morphlib.builder2.ldconfig(self.app.runcmd, + staging_area.dirname) + self.build_and_cache(staging_area, artifact) + self.remove_staging_area(staging_area) def get_recursive_deps(self, artifact): - done = set() - todo = set((artifact,)) - while todo: - for a in todo.pop().dependencies: - if a not in done: - done.add(a) - todo.add(a) - return done + return artifact.walk()[:-1] def get_sources(self, artifact): '''Update the local git repository cache with the sources.''' @@ -311,12 +298,13 @@ class BuildCommand(object): copy(self.rac.get_artifact_metadata(artifact, 'meta'), self.lac.put_artifact_metadata(artifact, 'meta')) - def create_staging_area(self, artifact): + def create_staging_area(self): '''Create the staging area for building a single artifact.''' self.app.status(msg='Creating staging area') staging_dir = tempfile.mkdtemp(dir=self.app.settings['tempdir']) - staging_area = morphlib.stagingarea.StagingArea(self.app, staging_dir) + staging_area = morphlib.stagingarea.StagingArea( + self.app, staging_dir, self.build_env, False, {}) return staging_area def remove_staging_area(self, staging_area): @@ -365,5 +353,5 @@ class BuildCommand(object): setup_mounts = self.app.settings['staging-chroot'] builder = morphlib.builder2.Builder( self.app, staging_area, self.lac, self.rac, self.lrc, - self.build_env, self.app.settings['max-jobs'], True) + self.app.settings['max-jobs'], True) return builder.build_and_cache(artifact) diff --git a/morphlib/buildenvironment.py b/morphlib/buildenvironment.py index 57270414..fce98da4 100644 --- a/morphlib/buildenvironment.py +++ b/morphlib/buildenvironment.py @@ -21,17 +21,18 @@ import morphlib class BuildEnvironment(): def __init__(self, settings, arch=None): + self.extra_path = [] + self.arch = morphlib.util.arch() if arch is None else arch self.env = self._clean_env(settings) _osenv = os.environ - _default_path = '/sbin:/usr/sbin:/bin:/usr/bin' - _override_term = 'dumb' + _ccache_path = '/usr/lib/ccache' + _override_home = '/tmp' + _override_locale = 'C' _override_shell = '/bin/sh' + _override_term = 'dumb' _override_username = 'tomjon' - _override_locale = 'C' - _override_home = '/tmp' - _ccache_path = '/usr/lib/ccache' def _clean_env(self, settings): '''Create a fresh set of environment variables for a clean build. @@ -40,8 +41,6 @@ class BuildEnvironment(): ''' - path = self._osenv['PATH'] - # copy a set of white-listed variables from the original env copied_vars = dict.fromkeys([ 'DISTCC_HOSTS', @@ -69,14 +68,11 @@ class BuildEnvironment(): env['LC_ALL'] = self._override_locale env['HOME'] = self._override_home - env['PATH'] = self._default_path - - env['TOOLCHAIN_TARGET'] = settings['toolchain-target'] - env['CFLAGS'] = settings['target-cflags'] env['PREFIX'] = settings['prefix'] env['BOOTSTRAP'] = 'false' if not settings['no-ccache']: - env['PATH'] = ('%s:%s' % (self._ccache_path, env['PATH'])) + self.extra_path.append(self._ccache_path) + # FIXME: we should set CCACHE_BASEDIR so any objects that refer to their # current directory get corrected. This improve the cache hit rate # env['CCACHE_BASEDIR'] = self.tempdir.dirname diff --git a/morphlib/buildenvironment_tests.py b/morphlib/buildenvironment_tests.py index a55cd5ac..2af3f605 100644 --- a/morphlib/buildenvironment_tests.py +++ b/morphlib/buildenvironment_tests.py @@ -14,6 +14,7 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +import copy import unittest import morphlib @@ -24,8 +25,6 @@ class BuildEnvironmentTests(unittest.TestCase): def setUp(self): self.settings = { - 'toolchain-target': '%s-baserock-linux-gnu' % morphlib.util.arch(), - 'target-cflags': '', 'prefix': '/usr', 'no-ccache': True, 'no-distcc': True @@ -44,13 +43,6 @@ class BuildEnvironmentTests(unittest.TestCase): arch='noarch') self.assertEqual(buildenv.arch, 'noarch') - def test_sets_default_path(self): - olddefaultpath = buildenvironment.BuildEnvironment._default_path - buildenvironment.BuildEnvironment._default_path = self.default_path - buildenv = buildenvironment.BuildEnvironment(self.settings) - buildenvironment.BuildEnvironment._default_path = olddefaultpath - self.assertTrue(self.default_path in buildenv.env['PATH']) - def test_copies_whitelist_vars(self): env = self.fake_env safe = { @@ -62,15 +54,15 @@ class BuildEnvironmentTests(unittest.TestCase): 'FAKEROOT_FD_BASE': '-1', } env.update(safe) - old_osenv = buildenvironment.BuildEnvironment._osenv buildenvironment.BuildEnvironment._osenv = env - buildenv = buildenvironment.BuildEnvironment(self.settings) - buildenvironment.BuildEnvironment._osenv = old_osenv + buildenv = buildenvironment.BuildEnvironment(self.settings) self.assertEqual(sorted(safe.items()), sorted([(k, buildenv.env[k]) for k in safe.keys()])) + buildenvironment.BuildEnvironment._osenv = old_osenv + def test_user_spellings_equal(self): buildenv = buildenvironment.BuildEnvironment(self.settings) self.assertTrue(buildenv.env['USER'] == buildenv.env['USERNAME'] == @@ -88,16 +80,13 @@ class BuildEnvironmentTests(unittest.TestCase): def test_environment_settings_set(self): buildenv = buildenvironment.BuildEnvironment(self.settings) - self.assertEqual(buildenv.env['TOOLCHAIN_TARGET'], - self.settings['toolchain-target']) - self.assertEqual(buildenv.env['CFLAGS'], - self.settings['target-cflags']) - self.assertEqual(buildenv.env['PREFIX'], - self.settings['prefix']) + #self.assertEqual(buildenv.env['TOOLCHAIN_TARGET'], + # self.settings['toolchain-target']) def test_ccache_vars_set(self): - self.settings['no-ccache'] = False - self.settings['no-distcc'] = False - buildenv = buildenvironment.BuildEnvironment(self.settings) - self.assertTrue(buildenv._ccache_path in buildenv.env['PATH']) + new_settings = copy.copy(self.settings) + new_settings['no-ccache'] = False + new_settings['no-distcc'] = False + buildenv = buildenvironment.BuildEnvironment(new_settings) + self.assertTrue(buildenv._ccache_path in buildenv.extra_path) self.assertEqual(buildenv.env['CCACHE_PREFIX'], 'distcc') diff --git a/morphlib/builder2.py b/morphlib/builder2.py index 6f7836e3..f6a1bafa 100644 --- a/morphlib/builder2.py +++ b/morphlib/builder2.py @@ -155,15 +155,14 @@ class BuilderBase(object): '''Base class for building artifacts.''' def __init__(self, app, staging_area, local_artifact_cache, - remote_artifact_cache, artifact, repo_cache, - build_env, max_jobs, setup_mounts): + remote_artifact_cache, artifact, repo_cache, max_jobs, + setup_mounts): self.app = app self.staging_area = staging_area self.local_artifact_cache = local_artifact_cache self.remote_artifact_cache = remote_artifact_cache self.artifact = artifact self.repo_cache = repo_cache - self.build_env = build_env self.max_jobs = max_jobs self.build_watch = morphlib.stopwatch.Stopwatch() self.setup_mounts = setup_mounts @@ -253,7 +252,6 @@ class BuilderBase(object): return a def runcmd(self, *args, **kwargs): - kwargs['env'] = self.build_env.env return self.staging_area.runcmd(*args, **kwargs) @@ -378,7 +376,7 @@ class ChunkBuilder(BuilderBase): relative_builddir = self.staging_area.relative(builddir) relative_destdir = self.staging_area.relative(destdir) - self.build_env.env['DESTDIR'] = relative_destdir + extra_env = { 'DESTDIR': relative_destdir } steps = [ ('pre-configure', False), @@ -406,9 +404,9 @@ class ChunkBuilder(BuilderBase): max_jobs = self.artifact.source.morphology['max-jobs'] if max_jobs is None: max_jobs = self.max_jobs - self.build_env.env['MAKEFLAGS'] = '-j%s' % max_jobs + extra_env['MAKEFLAGS'] = '-j%s' % max_jobs else: - self.build_env.env['MAKEFLAGS'] = '-j1' + extra_env['MAKEFLAGS'] = '-j1' try: # flushing is needed because writes from python and # writes from being the output in Popen have different @@ -416,6 +414,7 @@ class ChunkBuilder(BuilderBase): logfile.write('# # %s\n' % cmd) logfile.flush() self.runcmd(['sh', '-c', cmd], + extra_env=extra_env, cwd=relative_builddir, stdout=logfile, stderr=subprocess.STDOUT) @@ -670,14 +669,12 @@ class Builder(object): # pragma: no cover } def __init__(self, app, staging_area, local_artifact_cache, - remote_artifact_cache, repo_cache, build_env, max_jobs, - setup_mounts): + remote_artifact_cache, repo_cache, max_jobs, setup_mounts): self.app = app self.staging_area = staging_area self.local_artifact_cache = local_artifact_cache self.remote_artifact_cache = remote_artifact_cache self.repo_cache = repo_cache - self.build_env = build_env self.max_jobs = max_jobs self.setup_mounts = setup_mounts @@ -686,8 +683,8 @@ class Builder(object): # pragma: no cover o = self.classes[kind](self.app, self.staging_area, self.local_artifact_cache, self.remote_artifact_cache, artifact, - self.repo_cache, self.build_env, - self.max_jobs, self.setup_mounts) + self.repo_cache, self.max_jobs, + self.setup_mounts) logging.debug('Builder.build: artifact %s with %s' % (artifact.name, repr(o))) built_artifacts = o.build_and_cache() diff --git a/morphlib/builder2_tests.py b/morphlib/builder2_tests.py index df07a59f..c0da3cd9 100644 --- a/morphlib/builder2_tests.py +++ b/morphlib/builder2_tests.py @@ -35,8 +35,9 @@ class FakeApp(object): class FakeStagingArea(object): - def __init__(self, runcmd): + def __init__(self, runcmd, build_env): self.runcmd = runcmd + self.env = build_env.env class FakeSource(object): @@ -146,7 +147,7 @@ class BuilderBaseTests(unittest.TestCase): def setUp(self): self.commands_run = [] self.app = FakeApp(self.fake_runcmd) - self.staging_area = FakeStagingArea(self.fake_runcmd) + self.staging_area = FakeStagingArea(self.fake_runcmd, FakeBuildEnv()) self.artifact_cache = FakeArtifactCache() self.artifact = FakeArtifact('le-artifact') self.repo_cache = None @@ -158,7 +159,6 @@ class BuilderBaseTests(unittest.TestCase): None, self.artifact, self.repo_cache, - self.build_env, self.max_jobs, False) @@ -252,8 +252,7 @@ class ChunkBuilderTests(unittest.TestCase): def setUp(self): self.app = FakeApp() self.build = morphlib.builder2.ChunkBuilder(self.app, None, None, - None, None, None, None, 1, - False) + None, None, None, 1, False) def test_uses_morphology_commands_when_given(self): m = {'build-commands': ['build-it']} diff --git a/morphlib/cachekeycomputer.py b/morphlib/cachekeycomputer.py index a4ea10ed..15dc5ae9 100644 --- a/morphlib/cachekeycomputer.py +++ b/morphlib/cachekeycomputer.py @@ -1,4 +1,4 @@ -# Copyright (C) 2012 Codethink Limited +# Copyright (C) 2012-2013 Codethink Limited # # 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 @@ -27,9 +27,8 @@ class CacheKeyComputer(object): self._calculated = {} def _filterenv(self, env): - return dict([(k, env[k]) for k in ("USER", "USERNAME", "LOGNAME", - "TOOLCHAIN_TARGET", "PREFIX", - "BOOTSTRAP", "CFLAGS")]) + keys = ["BOOTSTRAP", "LOGNAME", "PREFIX", "USER", "USERNAME"] + return dict([(k, env[k]) for k in keys]) def compute_key(self, artifact): logging.debug('computing cache key for artifact %s from source ' diff --git a/morphlib/cachekeycomputer_tests.py b/morphlib/cachekeycomputer_tests.py index 411ad3f5..cc0b3ab8 100644 --- a/morphlib/cachekeycomputer_tests.py +++ b/morphlib/cachekeycomputer_tests.py @@ -14,6 +14,7 @@ # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +import copy import unittest import morphlib @@ -155,14 +156,8 @@ class CacheKeyComputerTests(unittest.TestCase): def test_different_env_gives_different_key(self): artifact = self._find_artifact('system-rootfs') oldsha = self.ckc.compute_key(artifact) - build_env = DummyBuildEnvironment({ - "USER": "foouser", - "USERNAME": "foouser", - "LOGNAME": "foouser", - "TOOLCHAIN_TARGET": "dummy-baserock-linux-gnu", - "PREFIX": "/baserock", - "BOOTSTRAP": "false", - "CFLAGS": "-Os"}) + build_env = copy.deepcopy(self.build_env) + build_env.env["USER"] = "brian" ckc = morphlib.cachekeycomputer.CacheKeyComputer(build_env) self.assertNotEqual(oldsha, ckc.compute_key(artifact)) diff --git a/morphlib/stagingarea.py b/morphlib/stagingarea.py index 24d4ebf9..ee3e444f 100644 --- a/morphlib/stagingarea.py +++ b/morphlib/stagingarea.py @@ -35,7 +35,9 @@ class StagingArea(object): ''' - def __init__(self, app, dirname): + _base_path = ['/sbin', '/usr/sbin', '/bin', '/usr/bin'] + + def __init__(self, app, dirname, build_env, use_chroot=True, extra_env={}): self._app = app self.dirname = dirname self.builddirname = None @@ -43,6 +45,17 @@ class StagingArea(object): self.mounted = None self._bind_readonly_mount = None + self.use_chroot = use_chroot + self.env = build_env.env + self.env.update(extra_env) + + if use_chroot: + path = build_env.extra_path + self._base_path + else: + full_path = [self.relative(p) for p in build_env.extra_path] + path = full_path + os.environ['PATH'].split(':') + self.env['PATH'] = ':'.join(path) + # Wrapper to be overridden by unit tests. def _mkdir(self, dirname): # pragma: no cover os.mkdir(dirname) @@ -75,6 +88,9 @@ class StagingArea(object): def relative(self, filename): '''Return a filename relative to the staging area.''' + if not self.use_chroot: + return filename + dirname = self.dirname if not dirname.endswith('/'): dirname += '/' @@ -190,8 +206,7 @@ class StagingArea(object): if not os.path.isdir(ccache_repodir): os.mkdir(ccache_repodir) # Get the destination path - ccache_destdir= os.path.join(self.tempdir, - 'tmp', 'ccache') + ccache_destdir= os.path.join(self.dirname, 'tmp', 'ccache') # Make sure that the destination exists. We'll create /tmp if necessary # to avoid breaking when faced with an empty staging area. if not os.path.isdir(ccache_destdir): @@ -247,14 +262,20 @@ class StagingArea(object): def runcmd(self, argv, **kwargs): # pragma: no cover '''Run a command in a chroot in the staging area.''' + assert 'env' not in kwargs + kwargs['env'] = self.env + if 'extra_env' in kwargs: + kwargs['env'].update(kwargs['extra_env']) + del kwargs['extra_env'] + + if self.use_chroot: + cwd = kwargs.get('cwd') or '/' + if 'cwd' in kwargs: + cwd = kwargs['cwd'] + del kwargs['cwd'] + else: + cwd = '/' - cwd = kwargs.get('cwd') or '/' - if 'cwd' in kwargs: - cwd = kwargs['cwd'] - del kwargs['cwd'] - else: - cwd = '/' - if self._app.settings['staging-chroot']: not_readonly_dirs = [self.builddirname, self.destdirname, 'dev', 'proc', 'tmp'] dirs = os.listdir(self.dirname) @@ -265,12 +286,11 @@ class StagingArea(object): for entry in dirs: real_argv += ['--mount-readonly', '/'+entry] - real_argv += [self.dirname] - else: - real_argv = ['chroot', '/'] - real_argv += ['sh', '-c', 'cd "$1" && shift && exec "$@"', '--', cwd] - real_argv += argv + real_argv += ['sh', '-c', 'cd "$1" && shift && exec "$@"', '--', cwd] + real_argv += argv - return self._app.runcmd(real_argv, **kwargs) + return self._app.runcmd(real_argv, **kwargs) + else: + return self._app.runcmd(argv, **kwargs) diff --git a/morphlib/stagingarea_tests.py b/morphlib/stagingarea_tests.py index 5c547f6e..35174f3b 100644 --- a/morphlib/stagingarea_tests.py +++ b/morphlib/stagingarea_tests.py @@ -24,6 +24,13 @@ import unittest import morphlib +class FakeBuildEnvironment(object): + + def __init__(self): + self.env = { + } + self.extra_path = ['/extra-path'] + class FakeSource(object): def __init__(self): @@ -56,8 +63,10 @@ class StagingAreaTests(unittest.TestCase): os.mkdir(os.path.join(self.cachedir, 'artifacts')) self.staging = os.path.join(self.tempdir, 'staging') self.created_dirs = [] + self.build_env = FakeBuildEnvironment() self.sa = morphlib.stagingarea.StagingArea( - FakeApplication(self.cachedir, self.tempdir), self.staging) + FakeApplication(self.cachedir, self.tempdir), self.staging, + self.build_env) def tearDown(self): shutil.rmtree(self.tempdir) @@ -118,3 +127,9 @@ class StagingAreaTests(unittest.TestCase): self.sa.install_artifact(f) self.sa.remove() self.assertFalse(os.path.exists(self.staging)) + + def test_supports_non_isolated_mode(self): + sa = morphlib.stagingarea.StagingArea( + object(), self.staging, self.build_env, use_chroot=False) + filename = os.path.join(self.staging, 'foobar') + self.assertEqual(sa.relative(filename), filename) diff --git a/morphlib/util.py b/morphlib/util.py index b4e06092..16063f45 100644 --- a/morphlib/util.py +++ b/morphlib/util.py @@ -42,7 +42,6 @@ def arch(): '''Return the CPU architecture of the current host.''' return os.uname()[4] - def indent(string, spaces=4): '''Return ``string`` indented by ``spaces`` spaces. |