From ca1ddcbc1a24e754a2e3d1dc5f3c46c56d0a60d5 Mon Sep 17 00:00:00 2001 From: Tiago Gomes Date: Wed, 25 Nov 2015 12:15:02 +0000 Subject: Cleanup buildbranch.py module Now that we don't support system branches, we don't need to iterate over a list of repos to create temporary build branches, commit their local changes and push those branches. We only need to do this for the definitions repository itself. A bug is also fixed where the local build branch was not being deleted due a missing call to _register_cleanup() when commiting the changes. This commit also renames some functions to more clear names, and moves the logic on pushed_build_branch() to the definitions_repo module, the only place where it is used. Change-Id: Id86240d0c189245bed36bc46355be13d46498dbc --- morphlib/buildbranch.py | 263 ++++++++++++++++--------------------------- morphlib/definitions_repo.py | 54 ++++++--- 2 files changed, 139 insertions(+), 178 deletions(-) diff --git a/morphlib/buildbranch.py b/morphlib/buildbranch.py index 455438ed..9c866a7a 100644 --- a/morphlib/buildbranch.py +++ b/morphlib/buildbranch.py @@ -42,63 +42,45 @@ class BuildBranch(object): ''' - def __init__(self, build_ref_prefix, build_uuid, definitions_repo=None): - self._definitions_repo = definitions_repo - + def __init__(self, build_ref_prefix, build_uuid, definitions_repo): + self._build_uuid = build_uuid + self._repo = definitions_repo self._cleanup = collections.deque() - self._to_push = {} self._td = fs.tempfs.TempFS() self._register_cleanup(self._td.close) - # Temporary branch of only a definitions.git repo. - # - # Temporary ref will look like this: - # - # $build-ref-prefix/$HEAD/$build_uuid - # - # For example: - # - # baserock/builds/master/f0b21fe240b244edb7e4142b6e201658 - ref = definitions_repo.HEAD - build_ref = os.path.join( - 'refs/heads', build_ref_prefix, ref, build_uuid) - - index = definitions_repo.get_index(self._td.getsyspath('index')) - head_tree = definitions_repo.resolve_ref_to_tree(ref) - index.set_to_tree(head_tree) - - self._to_push[definitions_repo] = (build_ref, index) - self._root, self._root_index = definitions_repo, index + self._build_ref = os.path.join( + 'refs/heads', build_ref_prefix, self._repo.HEAD, build_uuid) + + self._index = self._repo.get_index(self._td.getsyspath('index')) + self._index.set_to_tree( + self._repo.resolve_ref_to_tree(self._repo.HEAD)) def _register_cleanup(self, func, *args, **kwargs): self._cleanup.append((func, args, kwargs)) - 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=build_ref, changed=changed) - changes_made = True - index.add_files_from_working_tree(changed) - return changes_made - - 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. - `uuid` is used to identify each build uniquely and is included - in the commit message. - - A new commit is added to the temporary build branch of each of - the repositories in the SystemBranch with: - 1. The tree of anything currently in the temporary GitIndex. + def stage_uncommited_changes(self, add_cb): + '''Add any uncommitted changes to a temporary GitIndex''' + + changed = [to_path for code, to_path, from_path + in self._index.get_uncommitted_changes()] + if changed: + add_cb(gd=self._repo, build_ref=self._build_ref, + changed=changed) + self._index.add_files_from_working_tree(changed) + return bool(changed) + + def commit_staged_changes(self, author_name, author_email, commit_cb): + '''Commit changes in a temporary GitIndex to a temporary branch. + + `author_name` and `author_email` are required to construct the + commit author info. + + A new commit is added to the temporary build branch of the + definitions repository with: + 1. the tree of anything currently in the temporary GitIndex. This is the same as the current commit on HEAD unless - `add_uncommitted_changes` has been called. + `stage_uncommited_changes` has been called. 2. the parent of the previous temporary commit, or the last commit of the working tree if there has been no previous commits @@ -108,106 +90,101 @@ class BuildBranch(object): 4. commit message describing the current build using `uuid` ''' - commit_message = 'Morph build %s\n' % uuid - author_name = name - committer_name = 'Morph (on behalf of %s)' % name - author_email = committer_email = email - + commit_message = 'Morph build %s\n' % self._build_uuid + committer_name = 'Morph (on behalf of %s)' % author_name + committer_email = author_email with morphlib.branchmanager.LocalRefManager() as lrm: - for gd, (build_ref, index) in self._to_push.iteritems(): - 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, - committer_email=committer_email, - author_name=author_name, - author_email=author_email, - message=commit_message) - try: - old_commit = gd.resolve_ref_to_commit(build_ref) - except morphlib.gitdir.InvalidRefError: - lrm.add(gd, build_ref, commit) - else: - # NOTE: This will fail if build_ref pointed to a tag, - # due to resolve_ref_to_commit returning the - # commit id of tags, but since it's only morph - # that touches those refs, it should not be - # a problem. - lrm.update(gd, build_ref, commit, old_commit) - - def get_unpushed_branches(self): - '''Work out which, if any, local branches need to be pushed to build + tree = self._index.write_tree() + try: + parent = self._repo.resolve_ref_to_commit(self._build_ref) + except morphlib.gitdir.InvalidRefError: + parent = self._repo.resolve_ref_to_commit(self._repo.HEAD) + else: + # Skip updating ref if we already have a temporary build + # branch and have this tree on the branch + if tree == self_.def_repo.resolve_ref_to_tree( + self._build_ref): + return + + commit_cb(gd=self._repo, build_ref=self._build_ref) + commit = self._repo.commit_tree( + tree, parent=parent, + committer_name=committer_name, + committer_email=committer_email, + author_name=author_name, + author_email=author_email, + message=commit_message) + try: + old_commit = self._repo.resolve_ref_to_commit(self._build_ref) + except morphlib.gitdir.InvalidRefError: + lrm.add(self._repo, self._build_ref, commit) + else: + # NOTE: This will fail if build_ref pointed to a tag, + # due to resolve_ref_to_commit returning the commit id + # of tags, but since it's only morph that touches those + # refs, it should not be a problem. + lrm.update(self._repo, self._build_ref, commit, old_commit) + self._register_cleanup(lrm.close) + + def needs_pushing(self): + '''Work out if the temporary branch needs to be pushed NOTE: This assumes that the refs in the morphologies and the refs in the local checkouts match. ''' - for gd, (build_ref, index) in self._to_push.iteritems(): - head_ref = gd.HEAD - upstream_ref = gd.get_upstream_of_branch(head_ref) - if upstream_ref is None: - yield gd - continue - head_sha1 = gd.resolve_ref_to_commit(head_ref) - upstream_sha1 = gd.resolve_ref_to_commit(upstream_ref) - if head_sha1 != upstream_sha1: - yield gd - - def push_build_branches(self, push_cb=lambda **kwargs: None): - '''Push all temporary build branches to the remote repositories. + + head_ref = self._repo.HEAD + upstream_ref = self._repo.get_upstream_of_branch(head_ref) + if upstream_ref is None: + return True + head_sha1 = self._repo.resolve_ref_to_commit(head_ref) + upstream_sha1 = self._repo.resolve_ref_to_commit(upstream_ref) + return head_sha1 != upstream_sha1 + + def push_build_branch(self, push_cb): + '''Push the temporary build branch to the remote repository. ''' + 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) + remote = self._repo.get_remote('origin') + refspec = morphlib.gitdir.RefSpec(self._build_ref) + push_cb(gd=self._repo, build_ref=self._build_ref, + remote=remote, refspec=refspec) + rrm.push(remote, refspec) self._register_cleanup(rrm.close) @property - def root_repo_url(self): + def repo_remote_url(self): '''URI of the repository that systems may be found in.''' - return self._definitions_repo.remote_url + return self._repo.remote_url @property - def root_ref(self): - return self._definitions_repo.HEAD + def repo_local_url(self): + return urlparse.urljoin('file://', self._repo.dirname) @property - def root_commit(self): - return self._root.resolve_ref_to_commit(self.root_ref) + def head_commit(self): + return self._repo.resolve_ref_to_commit(self._repo.HEAD) @property - def root_local_repo_url(self): - return urlparse.urljoin('file://', self._root.dirname) + def head_ref(self): + return self._repo.HEAD @property - def root_build_ref(self): - '''Name of the ref of the repository that systems may be found in.''' - build_ref, index = self._to_push[self._root] - return build_ref + def build_commit(self): + return self._repo.resolve_ref_to_commit(self.build_ref) @property - def root_build_commit(self): - return self._root.resolve_ref_to_commit(self.root_build_ref) + def build_ref(self): + '''Name of the ref of the repository that systems may be found in.''' + return self._build_ref def close(self): '''Clean up any resources acquired during operation.''' - # TODO: This is a common pattern for our context managers, - # we could do with a way to share the common code. I suggest the + # TODO: This is a common pattern for our context managers, we + # could do with a way to share the common code. I suggest the # ExitStack from python3.4 or the contextlib2 module. exceptions = [] while self._cleanup: @@ -218,45 +195,3 @@ class BuildBranch(object): exceptions.append(e) if exceptions: raise BuildBranchCleanupError(self, exceptions) - - -@contextlib.contextmanager -def pushed_build_branch(bb, changes_need_pushing, name, email, - build_uuid, status): - with contextlib.closing(bb) as bb: - def report_add(gd, build_ref, changed): - status(msg='Creating temporary branch in %(dirname)s '\ - 'named %(ref)s', dirname=gd.dirname, ref=build_ref) - changes_made = bb.add_uncommitted_changes(add_cb=report_add) - unpushed = any(bb.get_unpushed_branches()) - - if not changes_made and not unpushed: - # We resolve the system branch ref to the commit SHA1 here, so that - # the build uses whatever commit the user's copy of the root repo - # considers the head of that branch to be. If we returned a named - # ref, we risk building what the remote considers the head of that - # branch to be instead, and we also trigger a needless update in - # the cached copy of the root repo. - yield bb.root_repo_url, bb.root_commit, bb.root_ref - return - - 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_commit, bb.root_build_ref - else: - yield (bb.root_local_repo_url, bb.root_build_commit, - bb.root_build_ref) diff --git a/morphlib/definitions_repo.py b/morphlib/definitions_repo.py index e2bfd829..4c13abee 100644 --- a/morphlib/definitions_repo.py +++ b/morphlib/definitions_repo.py @@ -67,9 +67,10 @@ class DefinitionsRepo(gitdir.GitDirectory): return self.get_remote('origin').get_fetch_url() - def branch_with_local_changes(self, uuid, push=True, build_ref_prefix=None, + def branch_with_local_changes(self, build_uuid, push=True, + build_ref_prefix=None, git_user_name=None, git_user_email=None, - status_cb=None): + status_cb=lambda **kwargs: None): '''Yield a branch that includes the user's local changes to this repo. When operating on local repos, this isn't really necessary. But when @@ -87,18 +88,43 @@ class DefinitionsRepo(gitdir.GitDirectory): to only certain refs in some Git servers. ''' - if status_cb: - status_cb(msg='Looking for uncommitted changes (pass ' - '--local-changes=ignore to skip)') - - bb = morphlib.buildbranch.BuildBranch( - build_ref_prefix, uuid, definitions_repo=self) - - pbb = morphlib.buildbranch.pushed_build_branch( - bb, changes_need_pushing=push, name=git_user_name, - email=git_user_email, build_uuid=uuid, - status=status_cb) - return pbb # (repo_url, commit, original_ref) + + status_cb(msg='Looking for uncommitted changes (pass ' + '--local-changes=ignore to skip)') + + def report_add(gd, build_ref, changed): + status_cb(msg='Creating temporary branch in %(dirname)s named ' + '%(ref)s', + dirname=gd.dirname, ref=build_ref) + + def report_commit(gd, build_ref): + status_cb(msg='Committing changes in %(dirname)s to %(ref)s', + dirname=gd.dirname, ref=build_ref, chatty=True) + + def report_push(gd, build_ref, remote, refspec): + status_cb(msg='Pushing %(ref)s in %(dirname)s to %(remote)s', + ref=build_ref, dirname=gd.dirname, + remote=remote.get_push_url(), chatty=True) + + @contextlib.contextmanager + def bbcm(): + with contextlib.closing(morphlib.buildbranch.BuildBranch( + build_ref_prefix, build_uuid, definitions_repo=self)) \ + as bb: + changes_made = bb.stage_uncommited_changes(add_cb=report_add) + unpushed = bb.needs_pushing() + if not changes_made and not unpushed: + yield bb.repo_remote_url, bb.head_commit, bb.head_ref + return + bb.commit_staged_changes(git_user_name, git_user_email, + commit_cb=report_commit) + if push: + repo_url = bb.repo_remote_url + bb.push_build_branch(push_cb=report_push) + else: + repo_url = bb.repo_local_url + yield repo_url, bb.build_commit, bb.build_ref + return bbcm() @contextlib.contextmanager def source_pool(self, lrc, rrc, cachedir, ref, system_filename, -- cgit v1.2.1