[Notes] [Git][BuildStream/buildstream][danielsilverstone-ct/fix-lint-iss

Daniel pushed to branch danielsilverstone-ct/fix-lint-issues at BuildStream / buildstream

Commits:

37e32bbe

by Daniel Silverstone at 2018-10-24T08:27:58Z

_yaml.py: Quieten a lint in Python < 3.6

For Python before 3.6, `path.resolve()` could not take the `strict`
keyword argument.  Linting on such Python versions will raise an unnecessary
issue given the check present.  As such, quieten that lint.  Unfortunately
this ends up making the line quite long (though not too long).

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

a96175cd

by Daniel Silverstone at 2018-10-24T08:28:04Z

_yamlcache.py: Correct bug in `YamlCache._get_filepath()`

The correct file path was computed but never returned.  This was
never caught because of a test missing consumption of one of its
parameters.  This commit resolves the bug mentioned in #722 leaving
the test to be amended by a future commit.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

bdc99787

by Daniel Silverstone at 2018-10-24T08:28:04Z

_yamlcache.py: Correct braino in variable name

This braino would mean that we'd fail to raise the right exception
if we failed to find the right project object when unpickling the
yaml cache.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

a5cf9781

by Daniel Silverstone at 2018-10-24T08:28:04Z

_yamlcache.py: Remove unused imports

The imports of _cachekey and utils were unused.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

460bde90

by Daniel Silverstone at 2018-10-24T08:28:31Z

buildelement.py: Quieten a lint about `__commands`

Unfortunately elements don't override `__init__` in general, so
we have to quieten pylint here.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

1de42941

by Daniel Silverstone at 2018-10-24T08:28:35Z

element.py: Simplify some conditions with `in (foo, bar)`

Where we have conditions of the form `var == foo or var == bar` it
can be simplified to `var in (foo, bar)` which pylint prefers.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

5249fcea

by Daniel Silverstone at 2018-10-24T08:28:59Z

element.py: Silence pylint warning about abstract method

Unfortunately pylint can't understand that we'll always use subclasses
of Element and as such complains of a no-return function (`assemble()`)
having its return value used.  This quietens that warning.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

8e6aa97c

by Daniel Silverstone at 2018-10-24T08:29:03Z

plugin.py: use dict.get() instead of an if statement

Pylint recommends the use of dict.get() rather than a multi-line if
statement for handling optional keyword arguments.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

d67252ae

by Daniel Silverstone at 2018-10-24T08:30:05Z

source.py: Quiet several pylint issues with abstract methods

A number of times, abstract methods are called which pylint cannot
possibly understand will only ever happen in subclasses which implement
the abstract methods.  This silences those specific warnings

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

ca589f81

by Daniel Silverstone at 2018-10-24T08:30:08Z

source.py: Remove unused local variable

Remove `source_kind` as it was unused.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

d839b6e9

by Daniel Silverstone at 2018-10-24T08:30:08Z

source.py: Remove unused variable

Remove the `context` variable which was unused.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

04e45f22

by Daniel Silverstone at 2018-10-24T08:30:29Z

utils.py: Disable lint warning about popen

We use `preexec_fn` in our use of Popen.  Since there are threads
in the codebase, this is not necessarily safe.  However we go to
great lengths to ensure that the main process doesn't spawn threads
(they're used in gRPC a lot) and so it should be safe for our use.
As such, we disable the lint here.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

45a66c11

by Daniel Silverstone at 2018-10-24T08:30:33Z

utils.py: Group stat imports

The imports of `stat` and `S_ISDIR` from `stat` should be grouped to
satisfy pylint.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

de1e0a52

by Daniel Silverstone at 2018-10-24T08:30:33Z

_artifactcache/artifactcache.py: Remove superfluous parens

The pylint report dislikes the superfluous parens which used to
be here in this assert.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

a3271709

by Daniel Silverstone at 2018-10-24T08:31:00Z

_artifactcache/artifactcache.py: Silence lint issues

The pylint tool is unable to understand that the abstract methods
in the artifact cache will only be called when there's concrete
implementations behind them.  Silence the lint errors for these
specific calls.

Signed-off-by: Daniel Silverstone <daniel silverstone codethink co uk>

8 changed files:

buildstream/_artifactcache/artifactcache.py
buildstream/_yaml.py
buildstream/_yamlcache.py
buildstream/buildelement.py
buildstream/element.py
buildstream/plugin.py
buildstream/source.py
buildstream/utils.py

Changes:

buildstream/_artifactcache/artifactcache.py

@@ -156,7 +156,7 @@ class ArtifactCache():
      def setup_remotes(self, *, use_config=False, remote_url=None):
          # Ensure we do not double-initialise since this can be expensive
 -        assert(not self._remotes_setup)
 +        assert not self._remotes_setup
          self._remotes_setup = True
          # Initialize remote artifact caches. We allow the commandline to override
@@ -252,7 +252,7 @@ class ArtifactCache():
      #    (int): The size of the cache after having cleaned up
+     #
      def clean(self):
 -        artifacts = self.list_artifacts()
 +        artifacts = self.list_artifacts()  # pylint: disable=assignment-from-no-return
          # Build a set of the cache keys which are required
          # based on the required elements at cleanup time
@@ -294,7 +294,7 @@ class ArtifactCache():
              if key not in required_artifacts:
                  # Remove the actual artifact, if it's not required.
 -                size = self.remove(to_remove)
 +                size = self.remove(to_remove)  # pylint: disable=assignment-from-no-return
                  # Remove the size from the removed size
                  self.set_cache_size(self._cache_size - size)
@@ -311,7 +311,7 @@ class ArtifactCache():
      #    (int): The size of the artifact cache.
+     #
      def compute_cache_size(self):
 -        self._cache_size = self.calculate_cache_size()
 +        self._cache_size = self.calculate_cache_size()  # pylint: disable=assignment-from-no-return
          return self._cache_size

buildstream/_yaml.py

@@ -473,7 +473,7 @@ def node_get_project_path(node, key, project_dir, *,
          if sys.version_info[0] == 3 and sys.version_info[1] < 6:
              full_resolved_path = (project_dir_path / path).resolve()
          else:
 -            full_resolved_path = (project_dir_path / path).resolve(strict=True)
 +            full_resolved_path = (project_dir_path / path).resolve(strict=True)  # pylint: disable=unexpected-keyword-arg
      except FileNotFoundError:
          raise LoadError(LoadErrorReason.MISSING_FILE,
                          "{}: Specified path '{}' does not exist"

buildstream/_yamlcache.py

@@ -27,9 +27,8 @@ import sys
  from contextlib import contextmanager
  from collections import namedtuple
 -from ._cachekey import generate_key
  from ._context import Context
 -from . import utils, _yaml
 +from . import _yaml
  YAML_CACHE_FILENAME = "yaml_cache.pickle"
@@ -207,7 +206,7 @@ class YamlCache():
              filepath = os.path.relpath(full_path, project.directory)
          else:
              filepath = full_path
 -        return full_path
 +        return filepath
      # _calculate_key():
+     #
@@ -329,7 +328,7 @@ class BstUnpickler(pickle.Unpickler):
                  if not project:
                      projects = [p.name for p in self._context.get_projects()]
                      raise pickle.UnpicklingError("No project with name {} found in {}"
 -                                                 .format(key_id, projects))
 +                                                 .format(project_tag, projects))
              else:
                  project = None
                  name = tagged_name

buildstream/buildelement.py

@@ -152,7 +152,7 @@ class BuildElement(Element):
      #############################################################
      def configure(self, node):
 -        self.__commands = {}
 +        self.__commands = {}  # pylint: disable=attribute-defined-outside-init
          # FIXME: Currently this forcefully validates configurations
          #        for all BuildElement subclasses so they are unable to

buildstream/element.py

@@ -432,7 +432,7 @@ class Element(Plugin):
                                                  visited=visited, recursed=True)
          # Yeild self only at the end, after anything needed has been traversed
 -        if should_yield and (recurse or recursed) and (scope == Scope.ALL or scope == Scope.RUN):
 +        if should_yield and (recurse or recursed) and (scope in (Scope.ALL, Scope.RUN)):
              yield self
      def search(self, scope, name):
@@ -1563,7 +1563,7 @@ class Element(Plugin):
                      # Step 3 - Prepare
                      self.__prepare(sandbox)
                      # Step 4 - Assemble
 -                    collect = self.assemble(sandbox)
 +                    collect = self.assemble(sandbox)  # pylint: disable=assignment-from-no-return
                      self.__set_build_result(success=True, description="succeeded")
                  except BstError as e:
                      # If an error occurred assembling an element in a sandbox,
@@ -2521,7 +2521,7 @@ class Element(Plugin):
          strong_key = meta['strong']
          weak_key = meta['weak']
 -        assert key == strong_key or key == weak_key
 +        assert key in (strong_key, weak_key)
          self.__metadata_keys[strong_key] = meta
          self.__metadata_keys[weak_key] = meta

buildstream/plugin.py

@@ -751,9 +751,7 @@ class Plugin():
          self.__context.message(message)
      def __note_command(self, output, *popenargs, **kwargs):
 -        workdir = os.getcwd()
 -        if 'cwd' in kwargs:
 -            workdir = kwargs['cwd']
 +        workdir = kwargs.get('cwd', os.getcwd())
          command = " ".join(popenargs[0])
          output.write('Running host command {}: {}\n'.format(workdir, command))
          output.flush()

buildstream/source.py

@@ -637,7 +637,7 @@ class Source(Plugin):
              # Source consistency interrogations are silent.
              context = self._get_context()
              with context.silence():
 -                self.__consistency = self.get_consistency()
 +                self.__consistency = self.get_consistency()  # pylint: disable=assignment-from-no-return
      # Return cached consistency
+     #
@@ -687,14 +687,14 @@ class Source(Plugin):
          key['directory'] = self.__directory
          if include_source:
 -            key['unique'] = self.get_unique_key()
 +            key['unique'] = self.get_unique_key()  # pylint: disable=assignment-from-no-return
          return key
      # Wrapper for set_ref(), also returns whether it changed.
+     #
      def _set_ref(self, ref, node):
 -        current_ref = self.get_ref()
 +        current_ref = self.get_ref()  # pylint: disable=assignment-from-no-return
          changed = False
          # This comparison should work even for tuples and lists,
@@ -773,7 +773,7 @@ class Source(Plugin):
          elif project.ref_storage == ProjectRefStorage.PROJECT_REFS:
              # First warn if there is a ref already loaded, and reset it
 -            redundant_ref = self.get_ref()
 +            redundant_ref = self.get_ref()  # pylint: disable=assignment-from-no-return
              if redundant_ref is not None:
                  self.set_ref(None, {})
@@ -883,7 +883,7 @@ class Source(Plugin):
          else:
              new_ref = self.__do_track()
 -        current_ref = self.get_ref()
 +        current_ref = self.get_ref()  # pylint: disable=assignment-from-no-return
          if new_ref is None:
              # No tracking, keep current ref
@@ -1038,15 +1038,12 @@ class Source(Plugin):
          if not mirrors or not alias:
              return self.track(**kwargs)
 -        context = self._get_context()
 -        source_kind = type(self)
+-
          # NOTE: We are assuming here that tracking only requires substituting the
          #       first alias used
          for uri in reversed(project.get_alias_uris(alias, first_pass=self.__first_pass)):
              new_source = self.__clone_for_uri(uri)
              try:
 -                ref = new_source.track(**kwargs)
 +                ref = new_source.track(**kwargs)  # pylint: disable=assignment-from-none
              # FIXME: Need to consider temporary vs. permanent failures,
              #        and how this works with retries.
              except BstError as e:

buildstream/utils.py

@@ -29,13 +29,13 @@ import re
  import shutil
  import signal
  import stat
 +from stat import S_ISDIR
  import string
  import subprocess
  import tempfile
  import itertools
  import functools
  from contextlib import contextmanager
 -from stat import S_ISDIR
  import psutil
@@ -1088,7 +1088,7 @@ def _call(*popenargs, terminate=False, **kwargs):
              os.killpg(group_id, signal.SIGCONT)
      with _signals.suspendable(suspend_proc, resume_proc), _signals.terminator(kill_proc):
 -        process = subprocess.Popen(*popenargs, preexec_fn=preexec_fn, **kwargs)
 +        process = subprocess.Popen(*popenargs, preexec_fn=preexec_fn, **kwargs)  # pylint: disable=subprocess-popen-preexec-fn
          output, _ = process.communicate()
          exit_code = process.poll()

[Notes] [Git][BuildStream/buildstream][danielsilverstone-ct/fix-lint-issues] 15 commits: _yaml.py: Quieten a lint in Python < 3.6

Daniel pushed to branch danielsilverstone-ct/fix-lint-issues at BuildStream / buildstream

Commits:

8 changed files:

Changes: