From 0a457a720999ad6f3621020ef31f7fff9389fd5d Mon Sep 17 00:00:00 2001 From: Phillip Smyth Date: Mon, 18 Jun 2018 14:34:39 +0100 Subject: Implementing relative workspaces This fixes #191 A note has been added to NEWS explaining backwards compatibility issues --- NEWS | 6 +++- buildstream/_frontend/widget.py | 4 ++- buildstream/_stream.py | 32 +++++++++++---------- buildstream/_workspaces.py | 41 +++++++++++---------------- buildstream/element.py | 7 +++-- tests/frontend/workspace.py | 63 ++++++++++++++++++++++++++++++++++------- 6 files changed, 98 insertions(+), 55 deletions(-) diff --git a/NEWS b/NEWS index 49901731b..262ee1428 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,11 @@ buildstream 1.3.2 o BuildStream now depends on python3 ujson (for some internal serializations) + o Workspaces can now be opened as relative paths. + + Existing open workspaces will not be converted to relative paths, + (they need to be closed and opened again to get the new behavior). + ================= buildstream 1.3.1 ================= @@ -211,7 +216,6 @@ buildstream 1.1.5 o Add support for the new include '(@)' directive in project.conf and .bst files - ================= buildstream 1.1.4 ================= diff --git a/buildstream/_frontend/widget.py b/buildstream/_frontend/widget.py index a772f3248..80fffd201 100644 --- a/buildstream/_frontend/widget.py +++ b/buildstream/_frontend/widget.py @@ -416,7 +416,9 @@ class LogLine(Widget): if "%{workspace-dirs" in format_: workspace = element._get_workspace() if workspace is not None: - path = workspace.path.replace(os.getenv('HOME', '/root'), '~') + path = workspace.get_absolute_path() + if path.startswith("~/"): + path = os.path.join(os.getenv('HOME', '/root'), path[2:]) line = p.fmt_subst(line, 'workspace-dirs', "Workspace: {}".format(path)) else: line = p.fmt_subst( diff --git a/buildstream/_stream.py b/buildstream/_stream.py index 19c73b85e..dd33c6a62 100644 --- a/buildstream/_stream.py +++ b/buildstream/_stream.py @@ -476,7 +476,7 @@ class Stream(): selection=PipelineSelection.REDIRECT, track_selection=PipelineSelection.REDIRECT) target = elements[0] - workdir = os.path.abspath(directory) + directory = os.path.abspath(directory) if not list(target.sources()): build_depends = [x.name for x in target.dependencies(Scope.BUILD, recurse=False)] @@ -492,7 +492,7 @@ class Stream(): workspace = workspaces.get_workspace(target._get_full_name()) if workspace and not force: raise StreamError("Workspace '{}' is already defined at: {}" - .format(target.name, workspace.path)) + .format(target.name, workspace.get_absolute_path())) # If we're going to checkout, we need at least a fetch, # if we were asked to track first, we're going to fetch anyway. @@ -518,7 +518,7 @@ class Stream(): except OSError as e: raise StreamError("Failed to create workspace directory: {}".format(e)) from e - workspaces.create_workspace(target._get_full_name(), workdir) + workspaces.create_workspace(target._get_full_name(), directory) if not no_checkout: with target.timed_activity("Staging sources to {}".format(directory)): @@ -542,12 +542,12 @@ class Stream(): # Remove workspace directory if prompted if remove_dir: with self._context.timed_activity("Removing workspace directory {}" - .format(workspace.path)): + .format(workspace.get_absolute_path())): try: - shutil.rmtree(workspace.path) + shutil.rmtree(workspace.get_absolute_path()) except OSError as e: raise StreamError("Could not remove '{}': {}" - .format(workspace.path, e)) from e + .format(workspace.get_absolute_path(), e)) from e # Delete the workspace and save the configuration workspaces.delete_workspace(element_name) @@ -590,28 +590,30 @@ class Stream(): for element in elements: workspace = workspaces.get_workspace(element._get_full_name()) - + workspace_path = workspace.get_absolute_path() if soft: workspace.prepared = False self._message(MessageType.INFO, "Reset workspace state for {} at: {}" - .format(element.name, workspace.path)) + .format(element.name, workspace_path)) continue with element.timed_activity("Removing workspace directory {}" - .format(workspace.path)): + .format(workspace_path)): try: - shutil.rmtree(workspace.path) + shutil.rmtree(workspace_path) except OSError as e: raise StreamError("Could not remove '{}': {}" - .format(workspace.path, e)) from e + .format(workspace_path, e)) from e workspaces.delete_workspace(element._get_full_name()) - workspaces.create_workspace(element._get_full_name(), workspace.path) + workspaces.create_workspace(element._get_full_name(), workspace_path) - with element.timed_activity("Staging sources to {}".format(workspace.path)): + with element.timed_activity("Staging sources to {}".format(workspace_path)): element._open_workspace() - self._message(MessageType.INFO, "Reset workspace for {} at: {}".format(element.name, workspace.path)) + self._message(MessageType.INFO, + "Reset workspace for {} at: {}".format(element.name, + workspace_path)) workspaces.save_config() @@ -648,7 +650,7 @@ class Stream(): for element_name, workspace_ in self._context.get_workspaces().list(): workspace_detail = { 'element': element_name, - 'directory': workspace_.path, + 'directory': workspace_.get_absolute_path(), } workspaces.append(workspace_detail) diff --git a/buildstream/_workspaces.py b/buildstream/_workspaces.py index 3f474b8ca..adffaa694 100644 --- a/buildstream/_workspaces.py +++ b/buildstream/_workspaces.py @@ -26,14 +26,6 @@ from ._exceptions import LoadError, LoadErrorReason BST_WORKSPACE_FORMAT_VERSION = 3 -# Hold on to a list of members which get serialized -_WORKSPACE_MEMBERS = [ - 'prepared', - 'path', - 'last_successful', - 'running_files' -] - # Workspace() # @@ -56,7 +48,7 @@ class Workspace(): def __init__(self, toplevel_project, *, last_successful=None, path=None, prepared=False, running_files=None): self.prepared = prepared self.last_successful = last_successful - self.path = path + self._path = path self.running_files = running_files if running_files is not None else {} self._toplevel_project = toplevel_project @@ -64,14 +56,20 @@ class Workspace(): # to_dict() # - # Convert this object to a dict for serialization purposes + # Convert a list of members which get serialized to a dict for serialization purposes # # Returns: # (dict) A dict representation of the workspace # def to_dict(self): - return {key: val for key, val in self.__dict__.items() - if key in _WORKSPACE_MEMBERS and val is not None} + ret = { + 'prepared': self.prepared, + 'path': self._path, + 'running_files': self.running_files + } + if self.last_successful is not None: + ret["last_successful"] = self.last_successful + return ret # from_dict(): # @@ -103,15 +101,7 @@ class Workspace(): # True if the workspace differs from 'other', otherwise False # def differs(self, other): - - for member in _WORKSPACE_MEMBERS: - member_a = getattr(self, member) - member_b = getattr(other, member) - - if member_a != member_b: - return True - - return False + return self.to_dict() != other.to_dict() # invalidate_key() # @@ -133,7 +123,7 @@ class Workspace(): if os.path.isdir(fullpath): utils.copy_files(fullpath, directory) else: - destfile = os.path.join(directory, os.path.basename(self.path)) + destfile = os.path.join(directory, os.path.basename(self.get_absolute_path())) utils.safe_copy(fullpath, destfile) # add_running_files() @@ -189,7 +179,7 @@ class Workspace(): filelist = utils.list_relative_paths(fullpath) filelist = [(relpath, os.path.join(fullpath, relpath)) for relpath in filelist] else: - filelist = [(self.path, fullpath)] + filelist = [(self.get_absolute_path(), fullpath)] self._key = [(relpath, unique_key(fullpath)) for relpath, fullpath in filelist] @@ -200,7 +190,7 @@ class Workspace(): # Returns: The absolute path of the element's workspace. # def get_absolute_path(self): - return os.path.join(self._toplevel_project.directory, self.path) + return os.path.join(self._toplevel_project.directory, self._path) # Workspaces() @@ -236,6 +226,9 @@ class Workspaces(): # path (str) - The path in which the workspace should be kept # def create_workspace(self, element_name, path): + if path.startswith(self._toplevel_project.directory): + path = os.path.relpath(path, self._toplevel_project.directory) + self._workspaces[element_name] = Workspace(self._toplevel_project, path=path) return self._workspaces[element_name] diff --git a/buildstream/element.py b/buildstream/element.py index 370eb7547..0ef7f09be 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -1385,7 +1385,8 @@ class Element(Plugin): # If mount_workspaces is set and we're doing incremental builds, # the workspace is already mounted into the sandbox. if not (mount_workspaces and self.__can_build_incrementally()): - with self.timed_activity("Staging local files at {}".format(workspace.path)): + with self.timed_activity("Staging local files at {}" + .format(workspace.get_absolute_path())): workspace.stage(directory) else: # No workspace, stage directly @@ -1559,7 +1560,7 @@ class Element(Plugin): sandbox_path = os.path.join(sandbox_root, self.__staged_sources_directory.lstrip(os.sep)) try: - utils.copy_files(workspace.path, sandbox_path) + utils.copy_files(workspace.get_absolute_path(), sandbox_path) except UtilError as e: self.warn("Failed to preserve workspace state for failed build sysroot: {}" .format(e)) @@ -1864,7 +1865,7 @@ class Element(Plugin): source._init_workspace(temp) # Now hardlink the files into the workspace target. - utils.link_files(temp, workspace.path) + utils.link_files(temp, workspace.get_absolute_path()) # _get_workspace(): # diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py index 1009ad3d8..aff3e15db 100644 --- a/tests/frontend/workspace.py +++ b/tests/frontend/workspace.py @@ -18,12 +18,13 @@ DATA_DIR = os.path.join( ) -def open_workspace(cli, tmpdir, datafiles, kind, track, suffix=''): - project = os.path.join(datafiles.dirname, datafiles.basename) - bin_files_path = os.path.join(project, 'files', 'bin-files') - element_path = os.path.join(project, 'elements') +def open_workspace(cli, tmpdir, datafiles, kind, track, suffix='', workspace_dir=None): + if not workspace_dir: + workspace_dir = os.path.join(str(tmpdir), 'workspace{}'.format(suffix)) + project_path = os.path.join(datafiles.dirname, datafiles.basename) + bin_files_path = os.path.join(project_path, 'files', 'bin-files') + element_path = os.path.join(project_path, 'elements') element_name = 'workspace-test-{}{}.bst'.format(kind, suffix) - workspace = os.path.join(str(tmpdir), 'workspace{}'.format(suffix)) # Create our repo object of the given source type with # the bin files, and then collect the initial ref. @@ -45,7 +46,7 @@ def open_workspace(cli, tmpdir, datafiles, kind, track, suffix=''): element_name)) # Assert that there is no reference, a track & fetch is needed - state = cli.get_element_state(project, element_name) + state = cli.get_element_state(project_path, element_name) if track: assert state == 'no reference' else: @@ -56,20 +57,20 @@ def open_workspace(cli, tmpdir, datafiles, kind, track, suffix=''): args = ['workspace', 'open'] if track: args.append('--track') - args.extend([element_name, workspace]) + args.extend([element_name, workspace_dir]) + result = cli.run(project=project_path, args=args) - result = cli.run(project=project, args=args) result.assert_success() # Assert that we are now buildable because the source is # now cached. - assert cli.get_element_state(project, element_name) == 'buildable' + assert cli.get_element_state(project_path, element_name) == 'buildable' # Check that the executable hello file is found in the workspace - filename = os.path.join(workspace, 'usr', 'bin', 'hello') + filename = os.path.join(workspace_dir, 'usr', 'bin', 'hello') assert os.path.exists(filename) - return (element_name, project, workspace) + return (element_name, project_path, workspace_dir) @pytest.mark.datafiles(DATA_DIR) @@ -190,6 +191,46 @@ def test_close(cli, tmpdir, datafiles, kind): assert not os.path.exists(workspace) +@pytest.mark.datafiles(DATA_DIR) +def test_close_external_after_move_project(cli, tmpdir, datafiles): + tmp_parent = os.path.dirname(str(tmpdir)) + workspace_dir = os.path.join(tmp_parent, "workspace") + element_name, project_path, _ = open_workspace(cli, tmpdir, datafiles, 'git', False, "", workspace_dir) + assert os.path.exists(workspace_dir) + tmp_dir = os.path.join(tmp_parent, 'external_project') + shutil.move(project_path, tmp_dir) + assert os.path.exists(tmp_dir) + + # Close the workspace + result = cli.run(configure=False, project=tmp_dir, args=[ + 'workspace', 'close', '--remove-dir', element_name + ]) + result.assert_success() + + # Assert the workspace dir has been deleted + assert not os.path.exists(workspace_dir) + # Move directory back inside tmp directory so it can be recognised + shutil.move(tmp_dir, project_path) + + +@pytest.mark.datafiles(DATA_DIR) +def test_close_internal_after_move_project(cli, tmpdir, datafiles): + element_name, project, _ = open_workspace(cli, tmpdir, datafiles, 'git', False) + tmp_dir = os.path.join(os.path.dirname(str(tmpdir)), 'external_project') + shutil.move(str(tmpdir), tmp_dir) + assert os.path.exists(tmp_dir) + + # Close the workspace + result = cli.run(configure=False, project=tmp_dir, args=[ + 'workspace', 'close', '--remove-dir', element_name + ]) + result.assert_success() + + # Assert the workspace dir has been deleted + workspace = os.path.join(tmp_dir, 'workspace') + assert not os.path.exists(workspace) + + @pytest.mark.datafiles(DATA_DIR) def test_close_removed(cli, tmpdir, datafiles): element_name, project, workspace = open_workspace(cli, tmpdir, datafiles, 'git', False) -- cgit v1.2.1