diff options
author | Tom Pollard <tom.pollard@codethink.co.uk> | 2019-05-08 17:32:00 +0100 |
---|---|---|
committer | bst-marge-bot <marge-bot@buildstream.build> | 2019-05-15 10:18:26 +0000 |
commit | 7c1bb299c891b7fe8b92e1d54a38eca0b55840ef (patch) | |
tree | aca810443b33de0172be97405dcfbdd0fd501a0d | |
parent | 108e30a44aded323c9b11ddf623bdcc6802d8193 (diff) | |
download | buildstream-7c1bb299c891b7fe8b92e1d54a38eca0b55840ef.tar.gz |
element.py: Name normalisation & artifact path constructer helpers
By extracting the functionality from the Element() it allows
the removal of code duplication for artifact assertion in
ArtifactShare(), via a new get_artifact_name() method in Cli().
-rw-r--r-- | buildstream/element.py | 51 | ||||
-rw-r--r-- | buildstream/testing/runcli.py | 12 | ||||
-rw-r--r-- | tests/artifactcache/junctions.py | 3 | ||||
-rw-r--r-- | tests/artifactcache/pull.py | 6 | ||||
-rw-r--r-- | tests/artifactcache/push.py | 2 | ||||
-rw-r--r-- | tests/frontend/artifact.py | 3 | ||||
-rw-r--r-- | tests/frontend/pull.py | 6 | ||||
-rw-r--r-- | tests/frontend/push.py | 8 | ||||
-rw-r--r-- | tests/integration/artifact.py | 11 | ||||
-rw-r--r-- | tests/integration/cachedfail.py | 2 | ||||
-rw-r--r-- | tests/integration/pullbuildtrees.py | 12 | ||||
-rw-r--r-- | tests/integration/shellbuildtrees.py | 2 | ||||
-rw-r--r-- | tests/testutils/artifactshare.py | 25 |
13 files changed, 79 insertions, 64 deletions
diff --git a/buildstream/element.py b/buildstream/element.py index 7f68af262..91b6ca63f 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -198,7 +198,7 @@ class Element(Plugin): if not self.__is_junction: project.ensure_fully_loaded() - self.normal_name = os.path.splitext(self.name.replace(os.sep, '-'))[0] + self.normal_name = _get_normal_name(self.name) """A normalized element name This is the original element without path separators or @@ -620,15 +620,7 @@ class Element(Plugin): assert key is not None - valid_chars = string.digits + string.ascii_letters + '-._' - element_name = ''.join([ - x if x in valid_chars else '_' - for x in self.normal_name - ]) - - # Note that project names are not allowed to contain slashes. Element names containing - # a '/' will have this replaced with a '-' upon Element object instantiation. - return '{0}/{1}/{2}'.format(project.name, element_name, key) + return _compose_artifact_name(project.name, self.normal_name, key) def stage_artifact(self, sandbox, *, path=None, include=None, exclude=None, orphans=True, update_mtimes=None): """Stage this element's output artifact in the sandbox @@ -3017,3 +3009,42 @@ def _overlap_error_detail(f, forbidden_overlap_elements, elements): " above ".join(reversed(elements)))) else: return "" + + +# _get_normal_name(): +# +# Get the element name without path separators or +# the extension. +# +# Args: +# element_name (str): The element's name +# +# Returns: +# (str): The normalised element name +# +def _get_normal_name(element_name): + return os.path.splitext(element_name.replace(os.sep, '-'))[0] + + +# _compose_artifact_name(): +# +# Compose the completely resolved 'artifact_name' as a filepath +# +# Args: +# project_name (str): The project's name +# normal_name (str): The element's normalised name +# cache_key (str): The relevant cache key +# +# Returns: +# (str): The constructed artifact name path +# +def _compose_artifact_name(project_name, normal_name, cache_key): + valid_chars = string.digits + string.ascii_letters + '-._' + normal_name = ''.join([ + x if x in valid_chars else '_' + for x in normal_name + ]) + + # Note that project names are not allowed to contain slashes. Element names containing + # a '/' will have this replaced with a '-' upon Element object instantiation. + return '{0}/{1}/{2}'.format(project_name, normal_name, cache_key) diff --git a/buildstream/testing/runcli.py b/buildstream/testing/runcli.py index 934c31236..82bab55b5 100644 --- a/buildstream/testing/runcli.py +++ b/buildstream/testing/runcli.py @@ -53,6 +53,7 @@ from _pytest.capture import MultiCapture, FDCapture, FDCaptureBinary from buildstream._frontend import cli as bst_cli from buildstream import _yaml from buildstream._cas import CASCache +from buildstream.element import _get_normal_name, _compose_artifact_name # Special private exception accessor, for test case purposes from buildstream._exceptions import BstError, get_last_exception, get_last_task_error @@ -495,6 +496,17 @@ class Cli(): result.assert_success() return result.output.splitlines() + # Fetch an element's complete artifact name, cache_key will be generated + # if not given. + # + def get_artifact_name(self, project, project_name, element_name, cache_key=None): + if not cache_key: + cache_key = self.get_element_key(project, element_name) + + # Replace path separator and chop off the .bst suffix for normal name + normal_name = _get_normal_name(element_name) + return _compose_artifact_name(project_name, normal_name, cache_key) + class CliIntegration(Cli): diff --git a/tests/artifactcache/junctions.py b/tests/artifactcache/junctions.py index 1eb67b659..d2eceb842 100644 --- a/tests/artifactcache/junctions.py +++ b/tests/artifactcache/junctions.py @@ -20,8 +20,7 @@ def assert_shared(cli, share, project_name, project, element_name): # NOTE: 'test' here is the name of the project # specified in the project.conf we are testing with. # - cache_key = cli.get_element_key(project, element_name) - if not share.has_artifact(project_name, element_name, cache_key): + if not share.has_artifact(cli.get_artifact_name(project, project_name, element_name)): raise AssertionError("Artifact share at {} does not contain the expected element {}" .format(share.repo, element_name)) diff --git a/tests/artifactcache/pull.py b/tests/artifactcache/pull.py index 40fed7637..4f34156d7 100644 --- a/tests/artifactcache/pull.py +++ b/tests/artifactcache/pull.py @@ -81,8 +81,7 @@ def test_pull(cli, tmpdir, datafiles): # Assert that we are now cached locally assert cli.get_element_state(project_dir, 'target.bst') == 'cached' # Assert that we shared/pushed the cached artifact - element_key = cli.get_element_key(project_dir, 'target.bst') - assert share.has_artifact('test', 'target.bst', element_key) + assert share.has_artifact(cli.get_artifact_name(project_dir, 'test', 'target.bst')) # Delete the artifact locally cli.remove_artifact_from_cache(project_dir, 'target.bst') @@ -191,8 +190,7 @@ def test_pull_tree(cli, tmpdir, datafiles): # Assert that we are now cached locally assert cli.get_element_state(project_dir, 'target.bst') == 'cached' # Assert that we shared/pushed the cached artifact - element_key = cli.get_element_key(project_dir, 'target.bst') - assert share.has_artifact('test', 'target.bst', element_key) + assert share.has_artifact(cli.get_artifact_name(project_dir, 'test', 'target.bst')) # Fake minimal context context = Context() diff --git a/tests/artifactcache/push.py b/tests/artifactcache/push.py index 4c1d2cd79..a099ad136 100644 --- a/tests/artifactcache/push.py +++ b/tests/artifactcache/push.py @@ -99,7 +99,7 @@ def test_push(cli, tmpdir, datafiles): raise assert not error - assert share.has_artifact('test', 'target.bst', element_key) + assert share.has_artifact(cli.get_artifact_name(project_dir, 'test', 'target.bst', cache_key=element_key)) def _test_push(user_config_file, project_dir, element_name, element_key, queue): diff --git a/tests/frontend/artifact.py b/tests/frontend/artifact.py index 10cb4f513..716c4b8a1 100644 --- a/tests/frontend/artifact.py +++ b/tests/frontend/artifact.py @@ -193,8 +193,7 @@ def test_artifact_delete_pulled_artifact_without_buildtree(cli, tmpdir, datafile result.assert_success() # Make sure it's in the share - cache_key = cli.get_element_key(project, element) - assert remote.has_artifact('test', element, cache_key) + assert remote.has_artifact(cli.get_artifact_name(project, 'test', element)) # Delete and then pull the artifact (without its buildtree) result = cli.run(project=project, args=['artifact', 'delete', element]) diff --git a/tests/frontend/pull.py b/tests/frontend/pull.py index cc62afe92..bf25ab1af 100644 --- a/tests/frontend/pull.py +++ b/tests/frontend/pull.py @@ -23,8 +23,7 @@ def assert_shared(cli, share, project, element_name): # NOTE: 'test' here is the name of the project # specified in the project.conf we are testing with. # - cache_key = cli.get_element_key(project, element_name) - if not share.has_artifact('test', element_name, cache_key): + if not share.has_artifact(cli.get_artifact_name(project, 'test', element_name)): raise AssertionError("Artifact share at {} does not contain the expected element {}" .format(share.repo, element_name)) @@ -35,8 +34,7 @@ def assert_not_shared(cli, share, project, element_name): # NOTE: 'test' here is the name of the project # specified in the project.conf we are testing with. # - cache_key = cli.get_element_key(project, element_name) - if share.has_artifact('test', element_name, cache_key): + if share.has_artifact(cli.get_artifact_name(project, 'test', element_name)): raise AssertionError("Artifact share at {} unexpectedly contains the element {}" .format(share.repo, element_name)) diff --git a/tests/frontend/push.py b/tests/frontend/push.py index 67c53f2bb..a9b072cf7 100644 --- a/tests/frontend/push.py +++ b/tests/frontend/push.py @@ -44,8 +44,7 @@ def assert_shared(cli, share, project, element_name): # NOTE: 'test' here is the name of the project # specified in the project.conf we are testing with. # - cache_key = cli.get_element_key(project, element_name) - if not share.has_artifact('test', element_name, cache_key): + if not share.has_artifact(cli.get_artifact_name(project, 'test', element_name)): raise AssertionError("Artifact share at {} does not contain the expected element {}" .format(share.repo, element_name)) @@ -56,8 +55,7 @@ def assert_not_shared(cli, share, project, element_name): # NOTE: 'test' here is the name of the project # specified in the project.conf we are testing with. # - cache_key = cli.get_element_key(project, element_name) - if share.has_artifact('test', element_name, cache_key): + if share.has_artifact(cli.get_artifact_name(project, 'test', element_name)): raise AssertionError("Artifact share at {} unexpectedly contains the element {}" .format(share.repo, element_name)) @@ -400,7 +398,7 @@ def test_push_cross_junction(cli, tmpdir, datafiles): cli.run(project=project, args=['artifact', 'push', 'junction.bst:import-etc.bst']) cache_key = cli.get_element_key(project, 'junction.bst:import-etc.bst') - assert share.has_artifact('subtest', 'import-etc.bst', cache_key) + assert share.has_artifact(cli.get_artifact_name(project, 'subtest', 'import-etc.bst', cache_key=cache_key)) @pytest.mark.datafiles(DATA_DIR) diff --git a/tests/integration/artifact.py b/tests/integration/artifact.py index a5e1f4d77..cb9f070d5 100644 --- a/tests/integration/artifact.py +++ b/tests/integration/artifact.py @@ -65,11 +65,11 @@ def test_cache_buildtrees(cli, tmpdir, datafiles): result = cli.run(project=project, args=['build', element_name]) assert result.exit_code == 0 assert cli.get_element_state(project, element_name) == 'cached' - assert share1.has_artifact('test', element_name, cli.get_element_key(project, element_name)) + assert share1.has_artifact(cli.get_artifact_name(project, 'test', element_name)) # The buildtree dir should not exist, as we set the config to not cache buildtrees. cache_key = cli.get_element_key(project, element_name) - elementdigest = share1.has_artifact('test', element_name, cache_key) + elementdigest = share1.has_artifact(cli.get_artifact_name(project, 'test', element_name, cache_key=cache_key)) with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir: assert not os.path.isdir(buildtreedir) @@ -102,10 +102,10 @@ def test_cache_buildtrees(cli, tmpdir, datafiles): result = cli.run(project=project, args=['--cache-buildtrees', 'always', 'build', element_name]) assert result.exit_code == 0 assert cli.get_element_state(project, element_name) == 'cached' - assert share2.has_artifact('test', element_name, cli.get_element_key(project, element_name)) + assert share2.has_artifact(cli.get_artifact_name(project, 'test', element_name)) # Cache key will be the same however the digest hash will have changed as expected, so reconstruct paths - elementdigest = share2.has_artifact('test', element_name, cache_key) + elementdigest = share2.has_artifact(cli.get_artifact_name(project, 'test', element_name, cache_key=cache_key)) with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir: assert os.path.isdir(buildtreedir) assert os.listdir(buildtreedir) @@ -132,8 +132,7 @@ def test_cache_buildtrees(cli, tmpdir, datafiles): result = cli.run(project=project, args=['build', element_name]) assert result.exit_code == 0 assert cli.get_element_state(project, element_name) == 'cached' - cache_key = cli.get_element_key(project, element_name) - elementdigest = share3.has_artifact('test', element_name, cache_key) + elementdigest = share3.has_artifact(cli.get_artifact_name(project, 'test', element_name)) with cli.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir: assert os.path.isdir(buildtreedir) assert os.listdir(buildtreedir) diff --git a/tests/integration/cachedfail.py b/tests/integration/cachedfail.py index 28174353d..489e48379 100644 --- a/tests/integration/cachedfail.py +++ b/tests/integration/cachedfail.py @@ -161,7 +161,7 @@ def test_push_cached_fail(cli, tmpdir, datafiles, on_error): # This element should have failed assert cli.get_element_state(project, 'element.bst') == 'failed' # This element should have been pushed to the remote - assert share.has_artifact('test', 'element.bst', cli.get_element_key(project, 'element.bst')) + assert share.has_artifact(cli.get_artifact_name(project, 'test', 'element.bst')) @pytest.mark.skipif(not (IS_LINUX and HAVE_BWRAP), reason='Only available with bubblewrap on Linux') diff --git a/tests/integration/pullbuildtrees.py b/tests/integration/pullbuildtrees.py index 91acff4a3..dfef18e7f 100644 --- a/tests/integration/pullbuildtrees.py +++ b/tests/integration/pullbuildtrees.py @@ -55,7 +55,7 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles): result = cli2.run(project=project, args=['build', element_name]) assert result.exit_code == 0 assert cli2.get_element_state(project, element_name) == 'cached' - assert share1.has_artifact('test', element_name, cli2.get_element_key(project, element_name)) + assert share1.has_artifact(cli2.get_artifact_name(project, 'test', element_name)) default_state(cli2, tmpdir, share1) # Pull artifact with default config, assert that pulling again @@ -75,7 +75,7 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles): # Also assert that the buildtree is added to the local CAS. result = cli2.run(project=project, args=['artifact', 'pull', element_name]) assert element_name in result.get_pulled_elements() - elementdigest = share1.has_artifact('test', element_name, cli2.get_element_key(project, element_name)) + elementdigest = share1.has_artifact(cli2.get_artifact_name(project, 'test', element_name)) with cli2.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir: assert not buildtreedir result = cli2.run(project=project, args=['--pull-buildtrees', 'artifact', 'pull', element_name]) @@ -115,7 +115,7 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles): cli2.configure({'artifacts': {'url': share2.repo, 'push': True}}) result = cli2.run(project=project, args=['artifact', 'push', element_name]) assert element_name not in result.get_pushed_elements() - assert not share2.has_artifact('test', element_name, cli2.get_element_key(project, element_name)) + assert not share2.has_artifact(cli2.get_artifact_name(project, 'test', element_name)) # Assert that after pulling the missing buildtree the element artifact can be # successfully pushed to the remote. This will attempt to pull the buildtree @@ -126,7 +126,7 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles): cli2.configure({'artifacts': {'url': share2.repo, 'push': True}}) result = cli2.run(project=project, args=['artifact', 'push', element_name]) assert element_name in result.get_pushed_elements() - assert share2.has_artifact('test', element_name, cli2.get_element_key(project, element_name)) + assert share2.has_artifact(cli2.get_artifact_name(project, 'test', element_name)) default_state(cli2, tmpdir, share1) # Assert that bst artifact push will automatically attempt to pull a missing buildtree @@ -142,7 +142,7 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles): with cli2.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir: assert not buildtreedir assert element_name not in result.get_pushed_elements() - assert not share3.has_artifact('test', element_name, cli2.get_element_key(project, element_name)) + assert not share3.has_artifact(cli2.get_artifact_name(project, 'test', element_name)) # Assert that if we add an extra remote that has the buildtree artfact cached, bst artifact push will # automatically attempt to pull it and will be successful, leading to the full artifact being pushed @@ -155,7 +155,7 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles): with cli2.artifact.extract_buildtree(tmpdir, elementdigest) as buildtreedir: assert os.path.isdir(buildtreedir) assert element_name in result.get_pushed_elements() - assert share3.has_artifact('test', element_name, cli2.get_element_key(project, element_name)) + assert share3.has_artifact(cli2.get_artifact_name(project, 'test', element_name)) # Ensure that only valid pull-buildtrees boolean options make it through the loading diff --git a/tests/integration/shellbuildtrees.py b/tests/integration/shellbuildtrees.py index 3d59c78b9..d03344992 100644 --- a/tests/integration/shellbuildtrees.py +++ b/tests/integration/shellbuildtrees.py @@ -225,7 +225,7 @@ def test_buildtree_options(cli, tmpdir, datafiles): result = cli.run(project=project, args=['--cache-buildtrees', 'always', 'build', element_name]) result.assert_success() assert cli.get_element_state(project, element_name) == 'cached' - assert share.has_artifact('test', element_name, cli.get_element_key(project, element_name)) + assert share.has_artifact(cli.get_artifact_name(project, 'test', element_name)) # Discard the cache shutil.rmtree(str(os.path.join(str(tmpdir), 'cache', 'cas'))) diff --git a/tests/testutils/artifactshare.py b/tests/testutils/artifactshare.py index 6c484ceb7..fca01497a 100644 --- a/tests/testutils/artifactshare.py +++ b/tests/testutils/artifactshare.py @@ -1,4 +1,3 @@ -import string import os import shutil import signal @@ -122,31 +121,13 @@ class ArtifactShare(): # Checks whether the artifact is present in the share # # Args: - # project_name (str): The project name - # element_name (str): The element name - # cache_key (str): The cache key + # artifact_name (str): The composed complete artifact name # # Returns: # (str): artifact digest if the artifact exists in the share, otherwise None. - def has_artifact(self, project_name, element_name, cache_key): - - # NOTE: This should be kept in line with our - # artifact cache code, the below is the - # same algo for creating an artifact reference - # - - # Replace path separator and chop off the .bst suffix - element_name = os.path.splitext(element_name.replace(os.sep, '-'))[0] - - valid_chars = string.digits + string.ascii_letters + '-._' - element_name = ''.join([ - x if x in valid_chars else '_' - for x in element_name - ]) - artifact_key = '{0}/{1}/{2}'.format(project_name, element_name, cache_key) - + def has_artifact(self, artifact_name): try: - tree = self.cas.resolve_ref(artifact_key) + tree = self.cas.resolve_ref(artifact_name) reachable = set() try: self.cas._reachable_refs_dir(reachable, tree, update_mtime=False, check_exists=True) |