From 1aaa11a10d03202063e15b6dd0a3b5217de51fc2 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 8 Aug 2014 23:05:10 +0100 Subject: Turn BuildBranch methods into regular functions Previously they were generator functions, which yielded interesting context at interesting times so that the caller could respond by printing status messages. The only benefits this had over callbacks were: 1. 1 fewer function scope to worry about. I don't have data on the amount of memory used for a function scope vs a generator, but it could be less cognitive load for determining which variables are defined in the callback's body. 2. It is possible to yield in the caller, so you could make that into a coroutine too, however this wasn't required in this case, as the yielded value was intended to be informational. The downsides to this are: 1. It's a rather peculiar construct, so can be difficult to understand what's going on, and the implications, which led to 2. If you call the function, but don't use the iterator it returned, then it won't do anything, which is very confusing indeed, if you're not used to how generator functions work. --- morphlib/buildbranch.py | 18 ++++++++++-------- morphlib/plugins/build_plugin.py | 14 +++++++++----- morphlib/plugins/deploy_plugin.py | 14 +++++++++----- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/morphlib/buildbranch.py b/morphlib/buildbranch.py index d415e7e1..7aa75dc1 100644 --- a/morphlib/buildbranch.py +++ b/morphlib/buildbranch.py @@ -84,14 +84,14 @@ class BuildBranch(object): def _register_cleanup(self, func, *args, **kwargs): self._cleanup.append((func, args, kwargs)) - def add_uncommitted_changes(self): + def add_uncommitted_changes(self, add_cb=lambda **kwargs: None): '''Add any uncommitted changes to temporary build GitIndexes''' for gd, (build_ref, index) in self._to_push.iteritems(): changed = [to_path for code, to_path, from_path in index.get_uncommitted_changes()] if not changed: continue - yield gd, build_ref + add_cb(gd=gd, build_ref=gd, changed=changed) index.add_files_from_working_tree(changed) @staticmethod @@ -102,7 +102,7 @@ class BuildBranch(object): sha1 = gd.store_blob(loader.save_to_string(morphology)) yield 0100644, sha1, morphology.filename - def inject_build_refs(self, loader): + def inject_build_refs(self, loader, inject_cb=lambda **kwargs: None): '''Update system and stratum morphologies to point to our branch. For all edited repositories, this alter the temporary GitIndex @@ -141,12 +141,13 @@ class BuildBranch(object): morphs.traverse_specs(process, filter) if any(m.dirty for m in morphs.morphologies): - yield self._root + inject_cb(gd=self._root) self._root_index.add_files_from_index_info( self._hash_morphologies(self._root, morphs.morphologies, loader)) - def update_build_refs(self, name, email, uuid): + def update_build_refs(self, name, email, uuid, + commit_cb=lambda **kwargs: None): '''Commit changes in temporary GitIndexes to temporary branches. `name` and `email` are required to construct the commit author info. @@ -176,7 +177,7 @@ class BuildBranch(object): with morphlib.branchmanager.LocalRefManager() as lrm: for gd, (build_ref, index) in self._to_push.iteritems(): - yield gd, build_ref + commit_cb(gd=gd, build_ref=build_ref) tree = index.write_tree() try: parent = gd.resolve_ref_to_commit(build_ref) @@ -201,7 +202,7 @@ class BuildBranch(object): # a problem. lrm.update(gd, build_ref, commit, old_commit) - def push_build_branches(self): + def push_build_branches(self, push_cb=lambda **kwargs: None): '''Push all temporary build branches to the remote repositories. This is a no-op if the BuildBranch was constructed with @@ -219,8 +220,9 @@ class BuildBranch(object): with morphlib.branchmanager.RemoteRefManager(False) as rrm: for gd, (build_ref, index) in self._to_push.iteritems(): remote = gd.get_remote('origin') - yield gd, build_ref, remote refspec = morphlib.gitdir.RefSpec(build_ref) + push_cb(gd=gd, build_ref=build_ref, + remote=remote, refspec=refspec) rrm.push(remote, refspec) self._register_cleanup(rrm.close) diff --git a/morphlib/plugins/build_plugin.py b/morphlib/plugins/build_plugin.py index 1a4fb573..3a489a61 100644 --- a/morphlib/plugins/build_plugin.py +++ b/morphlib/plugins/build_plugin.py @@ -187,27 +187,31 @@ class BuildPlugin(cliapp.Plugin): bb = morphlib.buildbranch.BuildBranch(sb, build_ref_prefix, push_temporary=push) with contextlib.closing(bb) as bb: - - for gd, build_ref in bb.add_uncommitted_changes(): + def report_add(gd, build_ref, changed): self.app.status(msg='Adding uncommitted changes '\ 'in %(dirname)s to %(ref)s', dirname=gd.dirname, ref=build_ref, chatty=True) + bb.add_uncommitted_changes(add_cb=report_add) - for gd in bb.inject_build_refs(loader): + def report_inject(gd): self.app.status(msg='Injecting temporary build refs '\ 'into morphologies in %(dirname)s', dirname=gd.dirname, chatty=True) + bb.inject_build_refs(loader, inject_cb=report_inject) - for gd, build_ref in bb.update_build_refs(name, email, build_uuid): + def report_commit(gd, build_ref): self.app.status(msg='Committing changes in %(dirname)s '\ 'to %(ref)s', dirname=gd.dirname, ref=build_ref, chatty=True) + bb.update_build_refs(name, email, build_uuid, + commit_cb=report_commit) - for gd, build_ref, remote in bb.push_build_branches(): + def report_push(gd, build_ref, remote, refspec): self.app.status(msg='Pushing %(ref)s in %(dirname)s '\ 'to %(remote)s', ref=build_ref, dirname=gd.dirname, remote=remote.get_push_url(), chatty=True) + bb.push_build_branches(push_cb=report_push) build_command.build([bb.root_repo_url, bb.root_ref, diff --git a/morphlib/plugins/deploy_plugin.py b/morphlib/plugins/deploy_plugin.py index 38c17bc2..99ca6fc3 100644 --- a/morphlib/plugins/deploy_plugin.py +++ b/morphlib/plugins/deploy_plugin.py @@ -322,27 +322,31 @@ class DeployPlugin(cliapp.Plugin): bb = morphlib.buildbranch.BuildBranch(sb, build_ref_prefix, push_temporary=False) with contextlib.closing(bb) as bb: - - for gd, build_ref in bb.add_uncommitted_changes(): + def report_add(gd, build_ref, changed): self.app.status(msg='Adding uncommitted changes '\ 'in %(dirname)s to %(ref)s', dirname=gd.dirname, ref=build_ref, chatty=True) + bb.add_uncommitted_changes(add_cb=report_add) - for gd in bb.inject_build_refs(loader): + def report_inject(gd): self.app.status(msg='Injecting temporary build refs '\ 'into morphologies in %(dirname)s', dirname=gd.dirname, chatty=True) + bb.inject_build_refs(loader, inject_cb=report_inject) - for gd, build_ref in bb.update_build_refs(name, email, build_uuid): + def report_commit(gd, build_ref): self.app.status(msg='Committing changes in %(dirname)s '\ 'to %(ref)s', dirname=gd.dirname, ref=build_ref, chatty=True) + bb.update_build_refs(name, email, build_uuid, + commit_cb=report_commit) - for gd, build_ref, remote in bb.push_build_branches(): + def report_push(gd, build_ref, remote, refspec): self.app.status(msg='Pushing %(ref)s in %(dirname)s '\ 'to %(remote)s', ref=build_ref, dirname=gd.dirname, remote=remote.get_push_url(), chatty=True) + bb.push_build_branches(push_cb=report_push) # Create a tempdir for this deployment to work in deploy_tempdir = tempfile.mkdtemp( -- cgit v1.2.1 From 4f14465e968be2ff479ddfeaa6eb568881287bed Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 8 Aug 2014 23:05:11 +0100 Subject: Cut BuildBranch out of morphlib.extensions This was previously used just so it could get the right repo and ref to read the file out of. However, there was a subtle bug in this behaviour, as if we had not previously used morph build in that branch, it would attempt to read the extensions from a branch which didn't exist. So now it reads it from the working tree, which always exists. --- morphlib/app.py | 5 ++--- morphlib/extensions.py | 42 ++++++++++++++++----------------------- morphlib/plugins/deploy_plugin.py | 3 +-- 3 files changed, 20 insertions(+), 30 deletions(-) diff --git a/morphlib/app.py b/morphlib/app.py index a543443e..88eb58a4 100644 --- a/morphlib/app.py +++ b/morphlib/app.py @@ -518,16 +518,15 @@ class Morph(cliapp.Application): self.output.write(text) def _help_topic(self, topic): - build_ref_prefix = self.settings['build-ref-prefix'] if topic in self.subcommands: usage = self._format_usage_for(topic) description = self._format_subcommand_help(topic) text = '%s\n\n%s' % (usage, description) self.output.write(text) - elif topic in extensions.list_extensions(build_ref_prefix): + elif topic in extensions.list_extensions(): name, kind = os.path.splitext(topic) try: - with extensions.get_extension_filename(build_ref_prefix, + with extensions.get_extension_filename( name, kind + '.help', executable=False) as fname: with open(fname, 'r') as f: diff --git a/morphlib/extensions.py b/morphlib/extensions.py index be551fdd..55478418 100644 --- a/morphlib/extensions.py +++ b/morphlib/extensions.py @@ -30,26 +30,20 @@ class ExtensionNotFoundError(ExtensionError): class ExtensionNotExecutableError(ExtensionError): pass -def _get_root_repo(build_ref_prefix): +def _get_root_repo(): system_branch = morphlib.sysbranchdir.open_from_within('.') root_repo_dir = morphlib.gitdir.GitDirectory( system_branch.get_git_directory_name( system_branch.root_repository_url)) - build_branch = morphlib.buildbranch.BuildBranch(system_branch, - build_ref_prefix, - push_temporary=False) - ref = build_branch.root_ref - - return (build_branch.root_ref, root_repo_dir) + return root_repo_dir def _get_morph_extension_directory(): code_dir = os.path.dirname(morphlib.__file__) return os.path.join(code_dir, 'exts') -def _list_repo_extension_filenames(build_ref_prefix, - kind): #pragma: no cover - (ref, repo_dir) = _get_root_repo(build_ref_prefix) - files = repo_dir.list_files(ref) +def _list_repo_extension_filenames(kind): #pragma: no cover + repo_dir = _get_root_repo() + files = repo_dir.list_files() return (f for f in files if os.path.splitext(f)[1] == kind) def _list_morph_extension_filenames(kind): @@ -59,9 +53,9 @@ def _list_morph_extension_filenames(kind): def _get_extension_name(filename): return os.path.basename(filename) -def _get_repo_extension_contents(build_ref_prefix, name, kind): - (ref, repo_dir) = _get_root_repo(build_ref_prefix) - return repo_dir.get_file_from_ref(ref, name + kind) +def _get_repo_extension_contents(name, kind): + repo_dir = _get_root_repo() + return repo_dir.read_file(name + kind) def _get_morph_extension_filename(name, kind): return os.path.join(_get_morph_extension_directory(), name + kind) @@ -71,11 +65,11 @@ def _is_executable(filename): mask = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH return (stat.S_IMODE(st.st_mode) & mask) != 0 -def _list_extensions(build_ref_prefix, kind): +def _list_extensions(kind): repo_extension_filenames = [] try: repo_extension_filenames = \ - _list_repo_extension_filenames(build_ref_prefix, kind) + _list_repo_extension_filenames(kind) except (sysbranchdir.NotInSystemBranch): # Squash this and just return no system branch extensions pass @@ -90,7 +84,7 @@ def _list_extensions(build_ref_prefix, kind): extension_names.update(set(morph_extension_names)) return list(extension_names) -def list_extensions(build_ref_prefix, kind=None): +def list_extensions(kind=None): """ List all available extensions by 'kind'. @@ -102,10 +96,10 @@ def list_extensions(build_ref_prefix, kind=None): be associated with a '.write' extension of the same name. """ if kind: - return _list_extensions(build_ref_prefix, kind) + return _list_extensions(kind) else: - configure_extensions = _list_extensions(build_ref_prefix, '.configure') - write_extensions = _list_extensions(build_ref_prefix, '.write') + configure_extensions = _list_extensions('.configure') + write_extensions = _list_extensions('.write') return configure_extensions + write_extensions @@ -121,8 +115,7 @@ class get_extension_filename(): If the extension is in the build repository then a temporary file will be created, which will be deleted on exting the with block. """ - def __init__(self, build_ref_prefix, name, kind, executable=True): - self.build_ref_prefix = build_ref_prefix + def __init__(self, name, kind, executable=True): self.name = name self.kind = kind self.executable = executable @@ -131,10 +124,9 @@ class get_extension_filename(): def __enter__(self): ext_filename = None try: - ext_contents = _get_repo_extension_contents(self.build_ref_prefix, - self.name, + ext_contents = _get_repo_extension_contents(self.name, self.kind) - except cliapp.AppException, sysbranchdir.NotInSystemBranch: + except (IOError, cliapp.AppException, sysbranchdir.NotInSystemBranch): # Not found: look for it in the Morph code. ext_filename = _get_morph_extension_filename(self.name, self.kind) if not os.path.exists(ext_filename): diff --git a/morphlib/plugins/deploy_plugin.py b/morphlib/plugins/deploy_plugin.py index 99ca6fc3..9f2d8eba 100644 --- a/morphlib/plugins/deploy_plugin.py +++ b/morphlib/plugins/deploy_plugin.py @@ -551,9 +551,8 @@ class DeployPlugin(cliapp.Plugin): system morphology (repo, ref), or with the Morph code. ''' - build_ref_prefix = self.app.settings['build-ref-prefix'] with morphlib.extensions.get_extension_filename( - build_ref_prefix, name, kind) as ext_filename: + name, kind) as ext_filename: self.app.status(msg='Running extension %(name)s%(kind)s', name=name, kind=kind) self.app.runcmd( -- cgit v1.2.1 From 54e03f2e5005775c8e8c434094256feccd91c488 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 8 Aug 2014 23:05:12 +0100 Subject: Treat untracked morphologies as uncommitted changes We used to coincidentally include morphologies because we later include every morphology in the build graph into our temporary build branches. Since we're now checking whether there's any uncommitted changes before attempting to create a temporary build branch, this means that we can no longer build uncommitted morphologies if they aren't reported as changes. So this patch makes an exception to the untracked changes rule for anything that ends with .morph. It's still confusing that some files aren't included in temporary build branches, but that would cause performance regressions, so we'll limit it to just morphologies for now, until we make a decision on what uncomitted content we care about. --- morphlib/gitindex.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/morphlib/gitindex.py b/morphlib/gitindex.py index 978ea0e2..6be4aacb 100644 --- a/morphlib/gitindex.py +++ b/morphlib/gitindex.py @@ -1,4 +1,4 @@ -# Copyright (C) 2013 Codethink Limited +# Copyright (C) 2013-2014 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 @@ -97,7 +97,8 @@ class GitIndex(object): def get_uncommitted_changes(self): for code, to_path, from_path in self._get_status(): - if code not in (STATUS_UNTRACKED, STATUS_IGNORED): + if (code not in (STATUS_UNTRACKED, STATUS_IGNORED) + or code == (STATUS_UNTRACKED) and to_path.endswith('.morph')): yield code, to_path, from_path def set_to_tree(self, treeish): -- cgit v1.2.1 From 6c9a5d65d615abf75e640f5ceda332ef42854d57 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Fri, 8 Aug 2014 23:05:13 +0100 Subject: Avoid creating and pushing temporary build branches when they aren't necessary. Sorry about the big lump, I can split it into a nicer set of changes, but they didn't naturally emerge in a nice series. This creates a pushed_build_branch context manager, to eliminate code duplication between build and deploy. Rather than the build branch being constructed knowing whether it needs to push the branch, it infers that from the state of the repositories, and whether a local build would be possible. If there are no uncommitted changes and all local branches are pushed, then it doesn't create temporary branches or push them, and instead uses what it already has. It will currently create and use temporary build branches even for chunks that have no local changes, but it's pretty cheap, and doesn't require re-working the build-ref injection code to check whether there are local changes. --- morphlib/buildbranch.py | 134 ++++++++++++++++++++++++++++---------- morphlib/gitdir.py | 14 ++++ morphlib/plugins/build_plugin.py | 39 ++--------- morphlib/plugins/deploy_plugin.py | 41 +++--------- 4 files changed, 129 insertions(+), 99 deletions(-) diff --git a/morphlib/buildbranch.py b/morphlib/buildbranch.py index 7aa75dc1..885f5cf8 100644 --- a/morphlib/buildbranch.py +++ b/morphlib/buildbranch.py @@ -15,6 +15,7 @@ import collections +import contextlib import os import urlparse @@ -48,10 +49,9 @@ class BuildBranch(object): # would be better to not use local repositories and temporary refs, # so building from a workspace appears to be identical to using # `morph build-morphology` - def __init__(self, sb, build_ref_prefix, push_temporary): + def __init__(self, sb, build_ref_prefix): self._sb = sb - self._push_temporary = push_temporary self._cleanup = collections.deque() self._to_push = {} @@ -86,13 +86,16 @@ class BuildBranch(object): def add_uncommitted_changes(self, add_cb=lambda **kwargs: None): '''Add any uncommitted changes to temporary build GitIndexes''' + changes_made = False for gd, (build_ref, index) in self._to_push.iteritems(): changed = [to_path for code, to_path, from_path in index.get_uncommitted_changes()] if not changed: continue add_cb(gd=gd, build_ref=gd, changed=changed) + changes_made = True index.add_files_from_working_tree(changed) + return changes_made @staticmethod def _hash_morphologies(gd, morphologies, loader): @@ -102,7 +105,8 @@ class BuildBranch(object): sha1 = gd.store_blob(loader.save_to_string(morphology)) yield 0100644, sha1, morphology.filename - def inject_build_refs(self, loader, inject_cb=lambda **kwargs: None): + def inject_build_refs(self, loader, use_local_repos, + inject_cb=lambda **kwargs: None): '''Update system and stratum morphologies to point to our branch. For all edited repositories, this alter the temporary GitIndex @@ -133,7 +137,7 @@ class BuildBranch(object): spec['repo'] = None spec['ref'] = None return True - if not self._push_temporary: + if use_local_repos: spec['repo'] = urlparse.urljoin('file://', gd.dirname) spec['ref'] = build_ref return True @@ -143,6 +147,8 @@ class BuildBranch(object): if any(m.dirty for m in morphs.morphologies): inject_cb(gd=self._root) + # TODO: Prevent it hashing unchanged morphologies, while still + # hashing uncommitted ones. self._root_index.add_files_from_index_info( self._hash_morphologies(self._root, morphs.morphologies, loader)) @@ -177,12 +183,18 @@ class BuildBranch(object): with morphlib.branchmanager.LocalRefManager() as lrm: for gd, (build_ref, index) in self._to_push.iteritems(): - commit_cb(gd=gd, build_ref=build_ref) tree = index.write_tree() try: parent = gd.resolve_ref_to_commit(build_ref) except morphlib.gitdir.InvalidRefError: parent = gd.resolve_ref_to_commit(gd.HEAD) + else: + # Skip updating ref if we already have a temporary + # build branch and have this tree on the branch + if tree == gd.resolve_ref_to_tree(build_ref): + continue + + commit_cb(gd=gd, build_ref=build_ref) commit = gd.commit_tree(tree, parent=parent, committer_name=committer_name, @@ -202,47 +214,55 @@ class BuildBranch(object): # a problem. lrm.update(gd, build_ref, commit, old_commit) - def push_build_branches(self, push_cb=lambda **kwargs: None): - '''Push all temporary build branches to the remote repositories. + def get_unpushed_branches(self): + '''Work out which, if any, local branches need to be pushed to build + + NOTE: This assumes that the refs in the morphologies and the + refs in the local checkouts match. - This is a no-op if the BuildBranch was constructed with - `push_temporary` as False, so that the code flow for the user of - the BuildBranch can be the same when it can be pushed as when - it can't. + ''' + for gd, (build_ref, index) in self._to_push.iteritems(): + remote = gd.get_remote('origin') + head_ref = gd.disambiguate_ref(gd.HEAD) + head_sha1 = gd.resolve_ref_to_commit(head_ref) + pushed_refs = sorted( + (remote_ref + for remote_sha1, remote_ref in remote.ls() + # substring match of refs, since ref may be a tag, + # in which case it would end with ^{} + if remote_sha1 == head_sha1 and head_ref in remote_ref), + key=len) + if not pushed_refs: + yield gd + def push_build_branches(self, push_cb=lambda **kwargs: None): + '''Push all temporary build branches to the remote repositories. ''' - # TODO: When BuildBranches become more context aware, if there - # are no uncommitted changes and the local versions are pushed - # we can skip pushing even if push_temporary is set. - # No uncommitted changes isn't sufficient reason to push the - # current HEAD - if self._push_temporary: - with morphlib.branchmanager.RemoteRefManager(False) as rrm: - for gd, (build_ref, index) in self._to_push.iteritems(): - remote = gd.get_remote('origin') - refspec = morphlib.gitdir.RefSpec(build_ref) - push_cb(gd=gd, build_ref=build_ref, - remote=remote, refspec=refspec) - rrm.push(remote, refspec) - self._register_cleanup(rrm.close) + with morphlib.branchmanager.RemoteRefManager(False) as rrm: + for gd, (build_ref, index) in self._to_push.iteritems(): + remote = gd.get_remote('origin') + refspec = morphlib.gitdir.RefSpec(build_ref) + push_cb(gd=gd, build_ref=build_ref, + remote=remote, refspec=refspec) + rrm.push(remote, refspec) + self._register_cleanup(rrm.close) @property def root_repo_url(self): '''URI of the repository that systems may be found in.''' - # TODO: When BuildBranches become more context aware, we only - # have to use the file:// URI when there's uncommitted changes - # and we can't push; or HEAD is not pushed and we can't push. - # All other times we can use the pushed branch - return (self._sb.get_config('branch.root') if self._push_temporary - else urlparse.urljoin('file://', self._root.dirname)) + return self._sb.get_config('branch.root') @property def root_ref(self): + return self._sb.get_config('branch.name') + + @property + def root_local_repo_url(self): + return urlparse.urljoin('file://', self._root.dirname) + + @property + def root_build_ref(self): '''Name of the ref of the repository that systems may be found in.''' - # TODO: When BuildBranches become more context aware, this can be - # HEAD when there's no uncommitted changes and we're not pushing; - # or we are pushing and there's no uncommitted changes and HEAD - # has been pushed. build_ref, index = self._to_push[self._root] return build_ref @@ -260,3 +280,47 @@ class BuildBranch(object): exceptions.append(e) if exceptions: raise BuildBranchCleanupError(self, exceptions) + + +@contextlib.contextmanager +def pushed_build_branch(bb, loader, changes_need_pushing, name, email, + build_uuid, status): + with contextlib.closing(bb) as bb: + def report_add(gd, build_ref, changed): + status(msg='Adding uncommitted changes '\ + 'in %(dirname)s to %(ref)s', + dirname=gd.dirname, ref=build_ref, chatty=True) + changes_made = bb.add_uncommitted_changes(add_cb=report_add) + unpushed = any(bb.get_unpushed_branches()) + + if not changes_made and not unpushed: + yield bb.root_repo_url, bb.root_ref + return + + def report_inject(gd): + status(msg='Injecting temporary build refs '\ + 'into morphologies in %(dirname)s', + dirname=gd.dirname, chatty=True) + bb.inject_build_refs(loader=loader, + use_local_repos=not changes_need_pushing, + inject_cb=report_inject) + + def report_commit(gd, build_ref): + status(msg='Committing changes in %(dirname)s '\ + 'to %(ref)s', + dirname=gd.dirname, ref=build_ref, + chatty=True) + bb.update_build_refs(name, email, build_uuid, + commit_cb=report_commit) + + if changes_need_pushing: + def report_push(gd, build_ref, remote, refspec): + status(msg='Pushing %(ref)s in %(dirname)s '\ + 'to %(remote)s', + ref=build_ref, dirname=gd.dirname, + remote=remote.get_push_url(), chatty=True) + bb.push_build_branches(push_cb=report_push) + + yield bb.root_repo_url, bb.root_build_ref + else: + yield bb.root_local_repo_url, bb.root_build_ref diff --git a/morphlib/gitdir.py b/morphlib/gitdir.py index 5b0693cb..3966a0f0 100644 --- a/morphlib/gitdir.py +++ b/morphlib/gitdir.py @@ -281,6 +281,16 @@ class Remote(object): return self.push_url or self.get_fetch_url() return self._get_remote_url(self.name, 'push') + @staticmethod + def _parse_ls_remote_output(output): # pragma: no cover + for line in output.splitlines(): + sha1, refname = line.split(None, 1) + yield sha1, refname + + def ls(self): # pragma: no cover + out = self.gd._runcmd(['git', 'ls-remote', self.get_fetch_url()]) + return self._parse_ls_remote_output(out) + @staticmethod def _parse_push_output(output): for line in output.splitlines(): @@ -484,6 +494,10 @@ class GitDirectory(object): except cliapp.AppException as e: raise InvalidRefError(self, ref) + def disambiguate_ref(self, ref): # pragma: no cover + out = self._runcmd(['git', 'rev-parse', '--symbolic-full-name', ref]) + return out.strip() + def resolve_ref_to_commit(self, ref): return self._rev_parse('%s^{commit}' % ref) diff --git a/morphlib/plugins/build_plugin.py b/morphlib/plugins/build_plugin.py index 3a489a61..64630c2b 100644 --- a/morphlib/plugins/build_plugin.py +++ b/morphlib/plugins/build_plugin.py @@ -184,35 +184,10 @@ class BuildPlugin(cliapp.Plugin): system=system_filename, branch=sb.system_branch_name) - bb = morphlib.buildbranch.BuildBranch(sb, build_ref_prefix, - push_temporary=push) - with contextlib.closing(bb) as bb: - def report_add(gd, build_ref, changed): - self.app.status(msg='Adding uncommitted changes '\ - 'in %(dirname)s to %(ref)s', - dirname=gd.dirname, ref=build_ref, chatty=True) - bb.add_uncommitted_changes(add_cb=report_add) - - def report_inject(gd): - self.app.status(msg='Injecting temporary build refs '\ - 'into morphologies in %(dirname)s', - dirname=gd.dirname, chatty=True) - bb.inject_build_refs(loader, inject_cb=report_inject) - - def report_commit(gd, build_ref): - self.app.status(msg='Committing changes in %(dirname)s '\ - 'to %(ref)s', - dirname=gd.dirname, ref=build_ref, chatty=True) - bb.update_build_refs(name, email, build_uuid, - commit_cb=report_commit) - - def report_push(gd, build_ref, remote, refspec): - self.app.status(msg='Pushing %(ref)s in %(dirname)s '\ - 'to %(remote)s', - ref=build_ref, dirname=gd.dirname, - remote=remote.get_push_url(), chatty=True) - bb.push_build_branches(push_cb=report_push) - - build_command.build([bb.root_repo_url, - bb.root_ref, - system_filename]) + bb = morphlib.buildbranch.BuildBranch(sb, build_ref_prefix) + pbb = morphlib.buildbranch.pushed_build_branch( + bb, loader=loader, changes_need_pushing=push, + name=name, email=email, build_uuid=build_uuid, + status=self.app.status) + with pbb as (repo, ref): + build_command.build([repo, ref, system_filename]) diff --git a/morphlib/plugins/deploy_plugin.py b/morphlib/plugins/deploy_plugin.py index 9f2d8eba..61b8145e 100644 --- a/morphlib/plugins/deploy_plugin.py +++ b/morphlib/plugins/deploy_plugin.py @@ -319,44 +319,21 @@ class DeployPlugin(cliapp.Plugin): self.validate_deployment_options( env_vars, all_deployments, all_subsystems) - bb = morphlib.buildbranch.BuildBranch(sb, build_ref_prefix, - push_temporary=False) - with contextlib.closing(bb) as bb: - def report_add(gd, build_ref, changed): - self.app.status(msg='Adding uncommitted changes '\ - 'in %(dirname)s to %(ref)s', - dirname=gd.dirname, ref=build_ref, chatty=True) - bb.add_uncommitted_changes(add_cb=report_add) - - def report_inject(gd): - self.app.status(msg='Injecting temporary build refs '\ - 'into morphologies in %(dirname)s', - dirname=gd.dirname, chatty=True) - bb.inject_build_refs(loader, inject_cb=report_inject) - - def report_commit(gd, build_ref): - self.app.status(msg='Committing changes in %(dirname)s '\ - 'to %(ref)s', - dirname=gd.dirname, ref=build_ref, chatty=True) - bb.update_build_refs(name, email, build_uuid, - commit_cb=report_commit) - - def report_push(gd, build_ref, remote, refspec): - self.app.status(msg='Pushing %(ref)s in %(dirname)s '\ - 'to %(remote)s', - ref=build_ref, dirname=gd.dirname, - remote=remote.get_push_url(), chatty=True) - bb.push_build_branches(push_cb=report_push) - + bb = morphlib.buildbranch.BuildBranch(sb, build_ref_prefix) + pbb = morphlib.buildbranch.pushed_build_branch( + bb, loader=loader, changes_need_pushing=False, + name=name, email=email, build_uuid=build_uuid, + status=self.app.status) + with pbb as (repo, ref): # Create a tempdir for this deployment to work in deploy_tempdir = tempfile.mkdtemp( dir=os.path.join(self.app.settings['tempdir'], 'deployments')) try: for system in cluster_morphology['systems']: self.deploy_system(build_command, deploy_tempdir, - root_repo_dir, bb.root_repo_url, - bb.root_ref, system, env_vars, - deployments, parent_location='') + root_repo_dir, repo, ref, system, + env_vars, deployments, + parent_location='') finally: shutil.rmtree(deploy_tempdir) -- cgit v1.2.1 From 05d8377e3131af101f444e079347953587a2b0d6 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Tue, 12 Aug 2014 16:28:22 +0000 Subject: Yarns: misc fixes --- yarns/branches-workspaces.yarn | 4 ++-- yarns/implementations.yarn | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/yarns/branches-workspaces.yarn b/yarns/branches-workspaces.yarn index 96fbbd12..0753296b 100644 --- a/yarns/branches-workspaces.yarn +++ b/yarns/branches-workspaces.yarn @@ -168,8 +168,8 @@ branches checked out. Editing components ------------------ -`morph edit` can edit refs for a stratum only, or it can do that for -a chunk, and check out the chunk's repository. +`morph edit` can edit refs for a chunk, and check out the chunk's +repository. First of all, we verify that that when we create a system branch, all the refs are unchanged. diff --git a/yarns/implementations.yarn b/yarns/implementations.yarn index 5b5b1724..f01432b7 100644 --- a/yarns/implementations.yarn +++ b/yarns/implementations.yarn @@ -444,7 +444,7 @@ Editing morphologies with `morph edit`. ls -l "$DATADIR/workspace/$MATCH_2" chunkdir="$(slashify_colons "$MATCH_1")" cd "$DATADIR/workspace/$MATCH_2/$chunkdir" - git branch | awk '$1 == "*" { print $2 }' > "$DATADIR/git-branch.actual" + git rev-parse --abbrev-ref HEAD > "$DATADIR/git-branch.actual" echo "$MATCH_2" > "$DATADIR/git-branch.wanted" diff -u "$DATADIR/git-branch.wanted" "$DATADIR/git-branch.actual" -- cgit v1.2.1 From 2fd432ffe1ba37bbe0101f903653df2c5ad3b4a6 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Tue, 12 Aug 2014 16:27:30 +0000 Subject: Yarns: implement pushing a whole system branch --- yarns/implementations.yarn | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/yarns/implementations.yarn b/yarns/implementations.yarn index f01432b7..8ed007b6 100644 --- a/yarns/implementations.yarn +++ b/yarns/implementations.yarn @@ -380,8 +380,7 @@ Attempt to branch a system branch from a root that had no systems. Pushing all changes in a system branch checkout to the git server. IMPLEMENTS WHEN the user pushes the system branch called (\S+) to the git server - # FIXME: For now, this is just the morphs checkout. - run_in "$DATADIR/workspace/$MATCH_1/test/morphs" git push origin HEAD + run_in "$DATADIR/workspace/$MATCH_1/" morph foreach -- sh -c 'git push -u origin HEAD 2>&1' Report workspace path. -- cgit v1.2.1 From a0ff2aad3a0919baef37489dbd121fa0ef6d19c4 Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Tue, 12 Aug 2014 16:28:33 +0000 Subject: Yarns: Preserve the output of morph foo in out-foo --- yarns/morph.shell-lib | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/yarns/morph.shell-lib b/yarns/morph.shell-lib index 05c11bcc..9d67f2ab 100644 --- a/yarns/morph.shell-lib +++ b/yarns/morph.shell-lib @@ -37,11 +37,12 @@ run_morph() { { set +e - "$SRCDIR"/morph \ + "$SRCDIR"/morph --verbose \ --cachedir-min-space=0 --tempdir-min-space=0 \ --no-default-config --config "$DATADIR/morph.conf" "$@" \ - 2> "$DATADIR/result-$1" + 2> "$DATADIR/result-$1" > "$DATADIR/out-$1" local exit_code="$?" + cat "$DATADIR/out-$1" cat "$DATADIR/result-$1" >&2 return "$exit_code" } -- cgit v1.2.1 From a74e2caafbeb49a49f542514590f720f6b215a6d Mon Sep 17 00:00:00 2001 From: Richard Maw Date: Tue, 12 Aug 2014 16:26:42 +0000 Subject: Add yarns for checking the state of temporary build branches --- yarns/branches-workspaces.yarn | 169 +++++++++++++++++++++++++++++++++++++++++ yarns/implementations.yarn | 70 +++++++++++++++++ 2 files changed, 239 insertions(+) diff --git a/yarns/branches-workspaces.yarn b/yarns/branches-workspaces.yarn index 0753296b..648055b0 100644 --- a/yarns/branches-workspaces.yarn +++ b/yarns/branches-workspaces.yarn @@ -194,6 +194,175 @@ fields when referring to strata, when it didn't before. AND in branch foo, system systems/test-system.morph refers to test-stratum without repo AND in branch foo, system systems/test-system.morph refers to test-stratum without ref +Temporary Build Branch behaviour +-------------------------------- + +Morph always builds from committed changes, but it's not always convenient +to commit and push changes, so `morph build` can create temporary build +branches when necessary. + + SCENARIO morph makes temporary build branches for uncommitted changes when necessary + GIVEN a workspace + AND a git server + WHEN the user checks out the system branch called master + +The user hasn't made any changes yet, so attempts to build require no +temporary build branches. + + GIVEN the workspace contains no temporary build branches + AND we can build with local branches + WHEN the user builds systems/test-system.morph of the master branch + THEN the morphs repository in the workspace for master has no temporary build branches + +Similarly, if we need to build from pushed branches, such as when we're +distbuilding, we don't need temporary build branches yet, since we have +no local changes. + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we must build from pushed branches + WHEN the user builds systems/test-system.morph of the master branch + THEN the morphs repository in the workspace for master has no temporary build branches + AND no temporary build branches were pushed to the morphs repository + +If we actually want to be able to push our changes for review, we need to +use a different branch from master, since we require code to be reviewed +then merged, rather than pushing directly to master. + + WHEN the user creates a system branch called baserock/test + +When we start making changes we do need temporary build branches, since +the chunk specifiers in the strata now need to refer to the local changes +to the repository. + + WHEN the user edits the chunk test-chunk in branch baserock/test + +If we don't need to build from pushed branches then we have temporary +build branches only in the local clones of the repositories. + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we can build with local branches + WHEN the user builds systems/test-system.morph of the baserock/test branch + THEN the morphs repository in the workspace for baserock/test has temporary build branches + AND no temporary build branches were pushed to the morphs repository + +If we do need to build from pushed changes, then the temporary build +branch needs to be pushed. + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we must build from pushed branches + WHEN the user builds systems/test-system.morph of the baserock/test branch + THEN the morphs repository in the workspace for baserock/test has temporary build branches + AND temporary build branches were pushed to the morphs repository + +NOTE: We're not checking whether the test-chunk repo has changes since +it's currently an implementation detail that it does, but it would +be possible to build without a temporary build branch for the chunk +repository. + +Now that we have the chunk repository available, we can make our changes. + + WHEN the user makes changes to test-chunk in branch baserock/test + +When we have uncommitted changes to chunk repositories, we need +temporary build branches locally for local builds. + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we can build with local branches + WHEN the user builds systems/test-system.morph of the baserock/test branch + THEN the morphs repository in the workspace for baserock/test has temporary build branches + AND the test-chunk repository in the workspace for baserock/test has temporary build branches + AND no temporary build branches were pushed to the morphs repository + AND no temporary build branches were pushed to the test-chunk repository + +As before, we also need temporary build branches to have been pushed + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we must build from pushed branches + WHEN the user builds systems/test-system.morph of the baserock/test branch + THEN the morphs repository in the workspace for baserock/test has temporary build branches + AND the test-chunk repository in the workspace for baserock/test has temporary build branches + AND temporary build branches were pushed to the morphs repository + AND temporary build branches were pushed to the test-chunk repository + +Now that we've made our changes, we can commit them. + + WHEN the user commits changes to morphs in branch baserock/test + AND the user commits changes to test-chunk in branch baserock/test + +For local builds we should be able to use these committed changes, +provided the ref in the morphology matches the committed ref in the +chunk repository. + +However, since we do not currently do this integrity check, as it requires +extra tracking between edited morphologies and the local repositories, +it's easier to just require a temporary build branch. + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we can build with local branches + WHEN the user builds systems/test-system.morph of the baserock/test branch + THEN the morphs repository in the workspace for baserock/test has temporary build branches + AND the test-chunk repository in the workspace for baserock/test has temporary build branches + AND no temporary build branches were pushed to the morphs repository + AND no temporary build branches were pushed to the test-chunk repository + +For distributed building, it being committed locally is not sufficient, +as remote workers need to be able to access the changes, and dist-build +workers tunneling into the developer's machine and using those +repositories would be madness, so we require temporary build branches +to be pushed. + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we must build from pushed branches + WHEN the user builds systems/test-system.morph of the baserock/test branch + THEN the morphs repository in the workspace for baserock/test has temporary build branches + AND the test-chunk repository in the workspace for baserock/test has temporary build branches + AND temporary build branches were pushed to the morphs repository + AND temporary build branches were pushed to the test-chunk repository + +We can now push our committed changes. + + WHEN the user pushes the system branch called baserock/test to the git server + +We now don't need temporary build branches for local builds. + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we can build with local branches + WHEN the user builds systems/test-system.morph of the baserock/test branch + THEN the morphs repository in the workspace for baserock/test has no temporary build branches + AND the test-chunk repository in the workspace for baserock/test has no temporary build branches + AND no temporary build branches were pushed to the morphs repository + AND no temporary build branches were pushed to the test-chunk repository + +Nor do we need temporary build branches for distributed builds. + + GIVEN the workspace contains no temporary build branches + AND the git server contains no temporary build branches + AND we must build from pushed branches + WHEN the user builds systems/test-system.morph of the baserock/test branch + THEN the morphs repository in the workspace for baserock/test has no temporary build branches + AND the test-chunk repository in the workspace for baserock/test has no temporary build branches + AND no temporary build branches were pushed to the morphs repository + AND no temporary build branches were pushed to the test-chunk repository + + IMPLEMENTS WHEN the user makes changes to test-chunk in branch (\S+) + chunkdir="$(slashify_colons "test:test-chunk")" + cd "$DATADIR/workspace/$MATCH_1/$chunkdir" + sed -i -e 's/Hello/Goodbye/g' test-bin + + IMPLEMENTS WHEN the user commits changes to (\S+) in branch (\S+) + chunkdir="$(slashify_colons "test:$MATCH_1")" + cd "$DATADIR/workspace/$MATCH_2/$chunkdir" + git commit -a -m 'Commit local changes' + + Status of system branch checkout -------------------------------- diff --git a/yarns/implementations.yarn b/yarns/implementations.yarn index 8ed007b6..a28fda54 100644 --- a/yarns/implementations.yarn +++ b/yarns/implementations.yarn @@ -604,6 +604,72 @@ Generating a manifest. die "Output isn't what we expect" fi +Implementations for temporary build branch handling +--------------------------------------------------- + + IMPLEMENTS GIVEN the workspace contains no temporary build branches + build_ref_prefix=baserock/builds/ + cd "$DATADIR/workspace" + # Want to use -execdir here, but busybox find doesn't support it + find . -name .git -print | while read gitdir; do ( + cd "$(dirname "$gitdir")" + eval "$(git for-each-ref --shell \ + --format='git update-ref -d %(refname) %(objectname)' \ + "refs/heads/$build_ref_prefix")" + ); done + + IMPLEMENTS GIVEN the git server contains no temporary build branches + build_ref_prefix=refs/heads/baserock/builds/ + cd "$DATADIR/gits" + # Want to use -execdir here, but busybox find doesn't support it + find . -name .git -print | while read gitdir; do ( + cd "$(dirname "$gitdir")" + eval "$(git for-each-ref --shell \ + --format='git update-ref -d %(refname) %(objectname)' \ + "$build_ref_prefix")" + git config receive.denyCurrentBranch ignore + rm -f .git/morph-pushed-branches + mkdir -p .git/hooks + cat >.git/hooks/post-receive <<'EOF' + #!/bin/sh + touch "$GIT_DIR/hook-ever-run" + exec cat >>"$GIT_DIR/morph-pushed-branches" + EOF + chmod +x .git/hooks/post-receive + ); done + + IMPLEMENTS GIVEN we can build with local branches + sed -i -e '/push-build-branches/d' "$DATADIR/morph.conf" + + IMPLEMENTS GIVEN we must build from pushed branches + cat >>"$DATADIR/morph.conf" <<'EOF' + push-build-branches = True + EOF + + IMPLEMENTS THEN the (\S+) repository in the workspace for (\S+) has temporary build branches + build_ref_prefix=refs/heads/baserock/builds/ + cd "$DATADIR/workspace/$MATCH_2/$(slashify_colons "test:$MATCH_1")" + git for-each-ref | grep -F "$build_ref_prefix" + + IMPLEMENTS THEN the (\S+) repository in the workspace for (\S+) has no temporary build branches + build_ref_prefix=refs/heads/baserock/builds/ + cd "$DATADIR/workspace/$MATCH_2/$(slashify_colons "test:$MATCH_1")" + if git for-each-ref | grep -F "$build_ref_prefix"; then + die Did not expect repo to contain build branches + fi + + IMPLEMENTS THEN no temporary build branches were pushed to the (\S+) repository + build_ref_prefix=refs/heads/baserock/builds/ + cd "$DATADIR/gits/$MATCH_1/.git" + if test -e morph-pushed-branches && grep -F "$build_ref_prefix" morph-pushed-branches; then + die Did not expect any pushed build branches + fi + + IMPLEMENTS THEN temporary build branches were pushed to the (\S+) repository + build_ref_prefix=refs/heads/baserock/builds/ + cd "$DATADIR/gits/$MATCH_1/.git" + test -e morph-pushed-branches && grep -F "$build_ref_prefix" morph-pushed-branches + Implementation sections for building ==================================== @@ -800,6 +866,10 @@ Implementations for building systems cd "$DATADIR/workspace/$MATCH_3" run_morph build "$MATCH_1" + IMPLEMENTS WHEN the user builds (\S+) of the (\S+) (branch|tag) + cd "$DATADIR/workspace/$MATCH_2" + run_morph build "$MATCH_1" + Implementations for tarball inspection -------------------------------------- -- cgit v1.2.1