From f08625b6a7b247665453b6a7d4872d6cb870bb30 Mon Sep 17 00:00:00 2001 From: Benjamin Schubert Date: Wed, 20 Mar 2019 22:54:17 +0000 Subject: lint: Fix or silence 'cyclic-import' errors and enable pylint for it Cyclic imports can be confusing because the order in which we import dependencies can make the import fail or not. We should not rely on ordering of imports for our code. This fixes everywhere possible the imports and silence explicitely some which are not convenient or would require big refactors --- .pylintrc | 5 ----- buildstream/_artifact.py | 3 +-- buildstream/_frontend/app.py | 6 ++++-- buildstream/_frontend/widget.py | 2 +- buildstream/_gitsourcebase.py | 7 ++++--- buildstream/_loader/loader.py | 6 +++--- buildstream/_platform/darwin.py | 2 +- buildstream/_platform/linux.py | 2 +- buildstream/_platform/platform.py | 6 +++--- buildstream/_platform/unix.py | 2 +- buildstream/buildelement.py | 6 ++++-- buildstream/element.py | 6 +++--- buildstream/sandbox/_sandboxdummy.py | 2 +- buildstream/sandbox/_sandboxremote.py | 3 +-- buildstream/scriptelement.py | 4 +++- buildstream/source.py | 3 ++- buildstream/storage/_filebaseddirectory.py | 2 +- buildstream/utils.py | 2 +- tests/testutils/element_generators.py | 2 +- tests/testutils/junction.py | 3 ++- 20 files changed, 38 insertions(+), 36 deletions(-) diff --git a/.pylintrc b/.pylintrc index 0cdb7586e..c47ef92cf 100644 --- a/.pylintrc +++ b/.pylintrc @@ -105,11 +105,6 @@ disable=##################################### unused-argument, - ########################################################### - # Messages that report warnings which should be addressed # - ########################################################### - - cyclic-import, # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option diff --git a/buildstream/_artifact.py b/buildstream/_artifact.py index 71b3c6f5c..6d26eb392 100644 --- a/buildstream/_artifact.py +++ b/buildstream/_artifact.py @@ -32,9 +32,8 @@ import os import shutil from . import _yaml -from . import Scope from ._exceptions import ArtifactError -from .types import _KeyStrength +from .types import Scope, _KeyStrength from .storage._casbaseddirectory import CasBasedDirectory diff --git a/buildstream/_frontend/app.py b/buildstream/_frontend/app.py index a659b202a..c506db221 100644 --- a/buildstream/_frontend/app.py +++ b/buildstream/_frontend/app.py @@ -41,7 +41,9 @@ from .. import _yaml from .._scheduler import ElementJob, JobStatus # Import frontend assets -from . import Profile, LogLine, Status +from .profile import Profile +from .status import Status +from .widget import LogLine # Intendation for all logging INDENT = 4 @@ -127,7 +129,7 @@ class App(): def create(cls, *args, **kwargs): if sys.platform.startswith('linux'): # Use an App with linux specific features - from .linuxapp import LinuxApp + from .linuxapp import LinuxApp # pylint: disable=cyclic-import return LinuxApp(*args, **kwargs) else: # The base App() class is default diff --git a/buildstream/_frontend/widget.py b/buildstream/_frontend/widget.py index f092cb5ec..cfe3a06e9 100644 --- a/buildstream/_frontend/widget.py +++ b/buildstream/_frontend/widget.py @@ -26,7 +26,7 @@ import textwrap from ruamel import yaml import click -from . import Profile +from .profile import Profile from .. import Element, Consistency, Scope from .. import _yaml from .. import __version__ as bst_version diff --git a/buildstream/_gitsourcebase.py b/buildstream/_gitsourcebase.py index d4c54fd89..5777d4aed 100644 --- a/buildstream/_gitsourcebase.py +++ b/buildstream/_gitsourcebase.py @@ -30,9 +30,10 @@ from tempfile import TemporaryFile from configparser import RawConfigParser -from buildstream import Source, SourceError, Consistency, SourceFetcher, CoreWarnings -from buildstream import utils -from buildstream.utils import move_atomic, DirectoryExistsError +from .source import Source, SourceError, SourceFetcher +from .types import Consistency, CoreWarnings +from . import utils +from .utils import move_atomic, DirectoryExistsError GIT_MODULES = '.gitmodules' diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py index 4c2bfec77..86ecf878e 100644 --- a/buildstream/_loader/loader.py +++ b/buildstream/_loader/loader.py @@ -30,8 +30,8 @@ from .._includes import Includes from .types import Symbol from .loadelement import LoadElement, _extract_depends_from_node -from . import MetaElement -from . import MetaSource +from .metaelement import MetaElement +from .metasource import MetaSource from ..types import CoreWarnings from .._message import Message, MessageType @@ -565,7 +565,7 @@ class Loader(): # Load the project project_dir = os.path.join(basedir, element.path) try: - from .._project import Project + from .._project import Project # pylint: disable=cyclic-import project = Project(project_dir, self._context, junction=element, parent_loader=self, search_for_project=False) except LoadError as e: diff --git a/buildstream/_platform/darwin.py b/buildstream/_platform/darwin.py index 8df8cda24..8e08685ec 100644 --- a/buildstream/_platform/darwin.py +++ b/buildstream/_platform/darwin.py @@ -19,7 +19,7 @@ import os from ..sandbox import SandboxDummy -from . import Platform +from .platform import Platform class Darwin(Platform): diff --git a/buildstream/_platform/linux.py b/buildstream/_platform/linux.py index 85e810c26..e4ce02572 100644 --- a/buildstream/_platform/linux.py +++ b/buildstream/_platform/linux.py @@ -24,7 +24,7 @@ from .. import _site from .. import utils from ..sandbox import SandboxDummy -from . import Platform +from .platform import Platform from .._exceptions import PlatformError diff --git a/buildstream/_platform/platform.py b/buildstream/_platform/platform.py index 73ed571fe..eef07812a 100644 --- a/buildstream/_platform/platform.py +++ b/buildstream/_platform/platform.py @@ -50,11 +50,11 @@ class Platform(): backend = 'unix' if backend == 'linux': - from .linux import Linux as PlatformImpl + from .linux import Linux as PlatformImpl # pylint: disable=cyclic-import elif backend == 'darwin': - from .darwin import Darwin as PlatformImpl + from .darwin import Darwin as PlatformImpl # pylint: disable=cyclic-import elif backend == 'unix': - from .unix import Unix as PlatformImpl + from .unix import Unix as PlatformImpl # pylint: disable=cyclic-import else: raise PlatformError("No such platform: '{}'".format(backend)) diff --git a/buildstream/_platform/unix.py b/buildstream/_platform/unix.py index bbc55c3af..d04b0712c 100644 --- a/buildstream/_platform/unix.py +++ b/buildstream/_platform/unix.py @@ -21,7 +21,7 @@ import os from .._exceptions import PlatformError -from . import Platform +from .platform import Platform class Unix(Platform): diff --git a/buildstream/buildelement.py b/buildstream/buildelement.py index eb30d9d31..158f5fc11 100644 --- a/buildstream/buildelement.py +++ b/buildstream/buildelement.py @@ -135,8 +135,10 @@ artifact collection purposes. """ import os -from . import Element, Scope -from . import SandboxFlags + +from .element import Element +from .sandbox import SandboxFlags +from .types import Scope # This list is preserved because of an unfortunate situation, we diff --git a/buildstream/element.py b/buildstream/element.py index 23127d125..2bb492cb3 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -93,16 +93,16 @@ from ._versions import BST_CORE_ARTIFACT_VERSION from ._exceptions import BstError, LoadError, LoadErrorReason, ImplError, \ ErrorDomain, SourceCacheError from .utils import UtilError -from . import Plugin, Consistency, Scope -from . import SandboxFlags, SandboxCommandError from . import utils from . import _cachekey from . import _signals from . import _site from ._platform import Platform +from .plugin import Plugin +from .sandbox import SandboxFlags, SandboxCommandError from .sandbox._config import SandboxConfig from .sandbox._sandboxremote import SandboxRemote -from .types import _KeyStrength, CoreWarnings, _UniquePriorityQueue +from .types import Consistency, CoreWarnings, Scope, _KeyStrength, _UniquePriorityQueue from ._artifact import Artifact from .storage.directory import Directory diff --git a/buildstream/sandbox/_sandboxdummy.py b/buildstream/sandbox/_sandboxdummy.py index 4cc3aae9c..750ddb05d 100644 --- a/buildstream/sandbox/_sandboxdummy.py +++ b/buildstream/sandbox/_sandboxdummy.py @@ -17,7 +17,7 @@ # Authors: from .._exceptions import SandboxError -from . import Sandbox +from .sandbox import Sandbox class SandboxDummy(Sandbox): diff --git a/buildstream/sandbox/_sandboxremote.py b/buildstream/sandbox/_sandboxremote.py index f7ee6471f..14e160dc5 100644 --- a/buildstream/sandbox/_sandboxremote.py +++ b/buildstream/sandbox/_sandboxremote.py @@ -28,8 +28,7 @@ import grpc from .. import utils from .._message import Message, MessageType -from . import Sandbox, SandboxCommandError -from .sandbox import _SandboxBatch +from .sandbox import Sandbox, SandboxCommandError, _SandboxBatch from ..storage._casbaseddirectory import CasBasedDirectory from .. import _signals from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2, remote_execution_pb2_grpc diff --git a/buildstream/scriptelement.py b/buildstream/scriptelement.py index 3327f818e..dfdbb45c0 100644 --- a/buildstream/scriptelement.py +++ b/buildstream/scriptelement.py @@ -35,7 +35,9 @@ implementations. import os from collections import OrderedDict -from . import Element, ElementError, Scope, SandboxFlags +from .element import Element, ElementError +from .sandbox import SandboxFlags +from .types import Scope class ScriptElement(Element): diff --git a/buildstream/source.py b/buildstream/source.py index 36885ee2a..6f4ff575b 100644 --- a/buildstream/source.py +++ b/buildstream/source.py @@ -165,8 +165,9 @@ import os from collections.abc import Mapping from contextlib import contextmanager -from . import Plugin, Consistency from . import _yaml, utils +from .plugin import Plugin +from .types import Consistency from ._exceptions import BstError, ImplError, ErrorDomain from ._loader.metasource import MetaSource from ._projectrefs import ProjectRefStorage diff --git a/buildstream/storage/_filebaseddirectory.py b/buildstream/storage/_filebaseddirectory.py index 742200379..9a746f731 100644 --- a/buildstream/storage/_filebaseddirectory.py +++ b/buildstream/storage/_filebaseddirectory.py @@ -79,7 +79,7 @@ class FileBasedDirectory(Directory): can_link=False): """ See superclass Directory for arguments """ - from ._casbaseddirectory import CasBasedDirectory + from ._casbaseddirectory import CasBasedDirectory # pylint: disable=cyclic-import if isinstance(external_pathspec, CasBasedDirectory): if can_link and not update_mtime: diff --git a/buildstream/utils.py b/buildstream/utils.py index f4a329210..e730dc2a9 100644 --- a/buildstream/utils.py +++ b/buildstream/utils.py @@ -485,7 +485,7 @@ def get_bst_version(): (int): The minor version """ # Import this only conditionally, it's not resolved at bash complete time - from . import __version__ + from . import __version__ # pylint: disable=cyclic-import versions = __version__.split('.')[:2] if versions[0] == '0+untagged': diff --git a/tests/testutils/element_generators.py b/tests/testutils/element_generators.py index 448c8571a..4461e4b7f 100644 --- a/tests/testutils/element_generators.py +++ b/tests/testutils/element_generators.py @@ -3,7 +3,7 @@ import os from buildstream import _yaml from buildstream import utils -from . import create_repo +from .repo import create_repo # create_element_size() diff --git a/tests/testutils/junction.py b/tests/testutils/junction.py index 01c76d14a..e0db8fcfb 100644 --- a/tests/testutils/junction.py +++ b/tests/testutils/junction.py @@ -1,6 +1,7 @@ -from tests.testutils import create_repo from buildstream import _yaml +from .repo import create_repo + # generate_junction() # -- cgit v1.2.1