From df92e8e08ea4ddfd58d6cfbf63d758cda5d3d44f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Tue, 16 Jul 2019 15:51:39 +0200 Subject: Store Platform reference in Context instance variable This allows us to remove the platform reset helpers in tests/conftest.py. --- src/buildstream/_context.py | 9 +++++++++ src/buildstream/_frontend/app.py | 3 +-- src/buildstream/_platform/platform.py | 12 ++---------- src/buildstream/_project.py | 3 +-- src/buildstream/element.py | 10 +++++----- tests/conftest.py | 17 ----------------- tests/integration/cachedfail.py | 6 ------ tests/sandboxes/fallback.py | 4 ---- tests/sandboxes/selection.py | 6 ------ 9 files changed, 18 insertions(+), 52 deletions(-) diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py index bb9825f44..3c20834b0 100644 --- a/src/buildstream/_context.py +++ b/src/buildstream/_context.py @@ -25,6 +25,7 @@ from . import _yaml from ._exceptions import LoadError, LoadErrorReason from ._messenger import Messenger from ._profile import Topics, PROFILER +from ._platform import Platform from ._artifactcache import ArtifactCache from ._sourcecache import SourceCache from ._cas import CASCache, CASQuota, CASCacheUsage @@ -153,6 +154,7 @@ class Context(): self.messenger = Messenger() # Private variables + self._platform = None self._artifactcache = None self._sourcecache = None self._projects = [] @@ -342,6 +344,13 @@ class Context(): 'strict', 'default-mirror', 'remote-execution']) + @property + def platform(self): + if not self._platform: + self._platform = Platform.create_instance() + + return self._platform + @property def artifactcache(self): if not self._artifactcache: diff --git a/src/buildstream/_frontend/app.py b/src/buildstream/_frontend/app.py index 372ade191..a1ddf8bb7 100644 --- a/src/buildstream/_frontend/app.py +++ b/src/buildstream/_frontend/app.py @@ -32,7 +32,6 @@ from .. import Scope # Import various buildstream internals from .._context import Context from ..plugin import Plugin -from .._platform import Platform from .._project import Project from .._exceptions import BstError, StreamError, LoadError, LoadErrorReason, AppError from .._message import Message, MessageType, unconditional_messages @@ -204,7 +203,7 @@ class App(): if option_value is not None: setattr(self.context, context_attr, option_value) try: - Platform.get_platform() + self.context.platform except BstError as e: self._error_exit(e, "Error instantiating platform") diff --git a/src/buildstream/_platform/platform.py b/src/buildstream/_platform/platform.py index dab6049ea..d6e0755f2 100644 --- a/src/buildstream/_platform/platform.py +++ b/src/buildstream/_platform/platform.py @@ -29,8 +29,6 @@ from .. import utils class Platform(): - _instance = None - # Platform() # # A class to manage platform-specific details. Currently holds the @@ -84,7 +82,7 @@ class Platform(): raise Error @classmethod - def _create_instance(cls): + def create_instance(cls): # Meant for testing purposes and therefore hidden in the # deepest corners of the source code. Try not to abuse this, # please? @@ -111,13 +109,7 @@ class Platform(): else: raise PlatformError("No such platform: '{}'".format(backend)) - cls._instance = PlatformImpl(force_sandbox=force_sandbox) - - @classmethod - def get_platform(cls): - if not cls._instance: - cls._create_instance() - return cls._instance + return PlatformImpl(force_sandbox=force_sandbox) def get_cpu_count(self, cap=None): cpu_count = len(psutil.Process().cpu_affinity()) diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py index 95afc78b5..fa02143e1 100644 --- a/src/buildstream/_project.py +++ b/src/buildstream/_project.py @@ -43,7 +43,6 @@ from ._loader import Loader from .element import Element from ._message import Message, MessageType from ._includes import Includes -from ._platform import Platform from ._workspaces import WORKSPACE_PROJECT_FILE @@ -789,7 +788,7 @@ class Project(): # users should set values based on workload and build infrastructure if self._context.build_max_jobs == 0: # User requested automatic max-jobs - platform = Platform.get_platform() + platform = self._context.platform output.base_variables['max-jobs'] = str(platform.get_cpu_count(8)) else: # User requested explicit max-jobs setting diff --git a/src/buildstream/element.py b/src/buildstream/element.py index af171621c..dffc33ba0 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -290,11 +290,11 @@ class Element(Plugin): self.__remote_execution_specs = project.remote_execution_specs # Extract Sandbox config - self.__sandbox_config = self.__extract_sandbox_config(project, meta) + self.__sandbox_config = self.__extract_sandbox_config(context, project, meta) self.__sandbox_config_supported = True if not self.__use_remote_execution(): - platform = Platform.get_platform() + platform = context.platform if not platform.check_sandbox_config(self.__sandbox_config): # Local sandbox does not fully support specified sandbox config. # This will taint the artifact, disable pushing. @@ -2487,7 +2487,7 @@ class Element(Plugin): def __sandbox(self, directory, stdout=None, stderr=None, config=None, allow_remote=True, bare_directory=False): context = self._get_context() project = self._get_project() - platform = Platform.get_platform() + platform = context.platform if directory is not None and allow_remote and self.__use_remote_execution(): @@ -2682,7 +2682,7 @@ class Element(Plugin): # Sandbox-specific configuration data, to be passed to the sandbox's constructor. # @classmethod - def __extract_sandbox_config(cls, project, meta): + def __extract_sandbox_config(cls, context, project, meta): if meta.is_junction: sandbox_config = Node.from_dict({ 'build-uid': 0, @@ -2692,7 +2692,7 @@ class Element(Plugin): sandbox_config = project._sandbox.clone() # Get the platform to ask for host architecture - platform = Platform.get_platform() + platform = context.platform host_arch = platform.get_host_arch() host_os = platform.get_host_os() diff --git a/tests/conftest.py b/tests/conftest.py index 138ae0745..77f8666a6 100755 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,7 +21,6 @@ # import os import pytest -from buildstream._platform.platform import Platform from buildstream.testing import register_repo_kind, sourcetests_collection_hook from buildstream.testing.integration import integration_cache # pylint: disable=unused-import @@ -102,22 +101,6 @@ def remote_services(request): return RemoteServices(**kwargs) -################################################# -# Automatically reset the platform # -################################################# -# -# This might need some refactor, maybe buildstream -# needs to cleanup more gracefully and we could remove this. -# -def clean_platform_cache(): - Platform._instance = None - - -@pytest.fixture(autouse=True) -def ensure_platform_cache_is_clean(): - clean_platform_cache() - - ################################################# # Setup for templated source tests # ################################################# diff --git a/tests/integration/cachedfail.py b/tests/integration/cachedfail.py index 467d3968f..a7509ab3b 100644 --- a/tests/integration/cachedfail.py +++ b/tests/integration/cachedfail.py @@ -25,7 +25,6 @@ from buildstream._exceptions import ErrorDomain from buildstream.testing import cli_integration as cli # pylint: disable=unused-import from buildstream.testing._utils.site import HAVE_SANDBOX -from tests.conftest import clean_platform_cache from tests.testutils import create_artifact_share @@ -206,16 +205,11 @@ def test_host_tools_errors_are_not_cached(cli, datafiles): } _yaml.roundtrip_dump(element, element_path) - clean_platform_cache() - # Build without access to host tools, this will fail result1 = cli.run(project=project, args=['build', 'element.bst'], env={'PATH': '', 'BST_FORCE_SANDBOX': None}) result1.assert_task_error(ErrorDomain.SANDBOX, 'unavailable-local-sandbox') assert cli.get_element_state(project, 'element.bst') == 'buildable' - # clean the cache before running again - clean_platform_cache() - # When rebuilding, this should work result2 = cli.run(project=project, args=['build', 'element.bst']) result2.assert_success() diff --git a/tests/sandboxes/fallback.py b/tests/sandboxes/fallback.py index d8170c61d..f2f585e70 100644 --- a/tests/sandboxes/fallback.py +++ b/tests/sandboxes/fallback.py @@ -23,8 +23,6 @@ from buildstream import _yaml from buildstream._exceptions import ErrorDomain from buildstream.testing import cli # pylint: disable=unused-import -from tests.conftest import clean_platform_cache - pytestmark = pytest.mark.integration @@ -56,8 +54,6 @@ def test_fallback_platform_fails(cli, datafiles): } _yaml.roundtrip_dump(element, element_path) - clean_platform_cache() - result = cli.run(project=project, args=['build', 'element.bst'], env={'BST_FORCE_BACKEND': 'fallback', 'BST_FORCE_SANDBOX': None}) diff --git a/tests/sandboxes/selection.py b/tests/sandboxes/selection.py index 50406b4cb..eff247a95 100644 --- a/tests/sandboxes/selection.py +++ b/tests/sandboxes/selection.py @@ -23,8 +23,6 @@ from buildstream import _yaml from buildstream._exceptions import ErrorDomain from buildstream.testing import cli # pylint: disable=unused-import -from tests.conftest import clean_platform_cache - pytestmark = pytest.mark.integration @@ -56,8 +54,6 @@ def test_force_sandbox(cli, datafiles): } _yaml.roundtrip_dump(element, element_path) - clean_platform_cache() - # Build without access to host tools, this will fail result = cli.run(project=project, args=['build', 'element.bst'], env={'PATH': '', 'BST_FORCE_SANDBOX': 'bwrap'}) result.assert_main_error(ErrorDomain.PLATFORM, None) @@ -89,8 +85,6 @@ def test_dummy_sandbox_fallback(cli, datafiles): } _yaml.roundtrip_dump(element, element_path) - clean_platform_cache() - # Build without access to host tools, this will fail result = cli.run(project=project, args=['build', 'element.bst'], env={'PATH': '', 'BST_FORCE_SANDBOX': None}) # But if we dont spesify a sandbox then we fall back to dummy, we still -- cgit v1.2.1