[Notes] [Git][BuildStream/buildstream][juerg/symlinks] 9 commits: filter

Jürg Billeter pushed to branch juerg/symlinks at BuildStream / buildstream

Commits:

03111d39

by Dor Askayo at 2019-01-30T10:35:06Z

filter.py: don't recurse when staging dependencies

Also bump the element's version so cached artifacts would be
invalidated.

Fixes #883

7256bb0c

by James Ennis at 2019-01-30T11:34:04Z

Merge branch 'doraskayo/filter-indirect-deps-fix' into 'master'

filter.py: don't recurse when staging dependencies

Closes #883

See merge request BuildStream/buildstream!1110

36746730

by Chandan Singh at 2019-01-31T10:50:05Z

tox.ini: Specify minimum version of click-man

`click-man` versions < 0.3.0 do not properly support multiple
entrypoints. Since this was added to `tox` after `0.3.0` was released,
`tox` should never be pulling older versions. But, let's add it here for
documentation purposes.

See
https://gitlab.com/BuildStream/buildstream/merge_requests/1107#note_135187046
for some background on this.

fa4a21ce

by Chandan Singh at 2019-01-31T12:15:43Z

Merge branch 'chandan/min-version-click-man' into 'master'

tox.ini: Specify minimum version of click-man

See merge request BuildStream/buildstream!1120

ebc31bd5

by Jürg Billeter at 2019-01-31T13:58:54Z

sandbox/sandbox.py: Do not follow symlinks in _has_command()

This is required to ensure symlinks are not resolved on the host.

aa83f158

by Jürg Billeter at 2019-01-31T13:58:54Z

utils.py: Change _ensure_real_directory() to not resolve symlinks

f778de6c

by Jürg Billeter at 2019-01-31T13:58:54Z

utils.py: Do not mangle symlinks

Copy symlinks as they are, absolute or relative.

3127e109
by Jürg Billeter at 2019-01-31T13:58:54Z
```
storage/_casbaseddirectory.py: Add TODO
```
07acea63
by Jürg Billeter at 2019-01-31T14:49:57Z
```
utils.py: Improve error message
```

8 changed files:

buildstream/plugins/elements/filter.py
buildstream/sandbox/sandbox.py
buildstream/storage/_casbaseddirectory.py
buildstream/utils.py
tests/elements/filter.py
+ tests/elements/filter/basic/elements/input-with-deps.bst
+ tests/elements/filter/basic/elements/output-include-with-indirect-deps.bst
tox.ini

Changes:

buildstream/plugins/elements/filter.py

@@ -47,6 +47,8 @@ from buildstream import Element, ElementError, Scope
  class FilterElement(Element):
      # pylint: disable=attribute-defined-outside-init
 +    BST_ARTIFACT_VERSION = 1
++
      # The filter element's output is its dependencies, so
      # we must rebuild if the dependencies change even when
      # not in strict build plans.
@@ -102,7 +104,7 @@ class FilterElement(Element):
      def assemble(self, sandbox):
          with self.timed_activity("Staging artifact", silent_nested=True):
 -            for dep in self.dependencies(Scope.BUILD):
 +            for dep in self.dependencies(Scope.BUILD, recurse=False):
                  dep.stage_artifact(sandbox, include=self.include,
                                     exclude=self.exclude, orphans=self.include_orphans)
          return ""

buildstream/sandbox/sandbox.py

@@ -525,11 +525,11 @@ class Sandbox():
      #         (bool): Whether a command exists inside the sandbox.
      def _has_command(self, command, env=None):
          if os.path.isabs(command):
 -            return os.path.exists(os.path.join(
 +            return os.path.lexists(os.path.join(
                  self._root, command.lstrip(os.sep)))
          for path in env.get('PATH').split(':'):
 -            if os.path.exists(os.path.join(
 +            if os.path.lexists(os.path.join(
                      self._root, path.lstrip(os.sep), command)):
                  return True

buildstream/storage/_casbaseddirectory.py

@@ -435,6 +435,7 @@ class CasBasedDirectory(Directory):
              return self
      def _resolve(self, name, absolute_symlinks_resolve=True, force_create=False):
 +        # TODO drop symlink resolution and mangling, matching changes in utils.py
          resolver = _Resolver(absolute_symlinks_resolve, force_create)
          return resolver.resolve(name, self)

buildstream/utils.py

@@ -774,37 +774,22 @@ def _copy_directories(srcdir, destdir, target):
                                  'directory expected: {}'.format(old_dir))
 -@functools.lru_cache(maxsize=64)
 -def _resolve_symlinks(path):
 -    return os.path.realpath(path)
+-
+-
 -def _ensure_real_directory(root, destpath):
 -    # The realpath in the sandbox may refer to a file outside of the
 -    # sandbox when any of the direcory branches are a symlink to an
 -    # absolute path.
 -    #
 -    # This should not happen as we rely on relative_symlink_target() below
 -    # when staging the actual symlinks which may lead up to this path.
 -    #
 -    destpath_resolved = _resolve_symlinks(destpath)
 -    if not destpath_resolved.startswith(_resolve_symlinks(root)):
 -        raise UtilError('Destination path resolves to a path outside ' +
 -                        'of the staging area\n\n' +
 -                        '  Destination path: {}\n'.format(destpath) +
 -                        '  Real path: {}'.format(destpath_resolved))
+-
 -    # Ensure the real destination path exists before trying to get the mode
 -    # of the real destination path.
 -    #
 -    # It is acceptable that chunks create symlinks inside artifacts which
 -    # refer to non-existing directories, they will be created on demand here
 -    # at staging time.
 -    #
 -    if not os.path.exists(destpath_resolved):
 -        os.makedirs(destpath_resolved)
+-
 -    return destpath_resolved
 +# _ensure_real_directory()
 +#
 +# Ensure `path` is a real directory and there are no symlink components.
 +#
 +# Symlink components are allowed in `root`.
 +#
 +def _ensure_real_directory(root, path):
 +    destpath = root
 +    for name in os.path.split(path):
 +        destpath = os.path.join(destpath, name)
 +        try:
 +            deststat = os.lstat(destpath)
 +            if not stat.S_ISDIR(deststat.st_mode):
 +                raise UtilError('Destination is not a directory {} for {}'.format(destpath, path))
 +        except FileNotFoundError:
 +            os.makedirs(destpath)
  # _process_list()
@@ -843,6 +828,10 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result,
          srcpath = os.path.join(srcdir, path)
          destpath = os.path.join(destdir, path)
 +        # Ensure that the parent of the destination path exists without symlink
 +        # components.
 +        _ensure_real_directory(destdir, os.path.dirname(path))
++
          # Add to the results the list of files written
          if report_written:
              result.files_written.append(path)
@@ -854,11 +843,6 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result,
          # The destination directory may not have been created separately
          permissions.extend(_copy_directories(srcdir, destdir, path))
 -        # Ensure that broken symlinks to directories have their targets
 -        # created before attempting to stage files across broken
 -        # symlink boundaries
 -        _ensure_real_directory(destdir, os.path.dirname(destpath))
+-
          try:
              file_stat = os.lstat(srcpath)
              mode = file_stat.st_mode
@@ -872,13 +856,7 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result,
          if stat.S_ISDIR(mode):
              # Ensure directory exists in destination
 -            if not os.path.exists(destpath):
 -                _ensure_real_directory(destdir, destpath)
+-
 -            dest_stat = os.lstat(_resolve_symlinks(destpath))
 -            if not stat.S_ISDIR(dest_stat.st_mode):
 -                raise UtilError('Destination not a directory. source has {}'
 -                                ' destination has {}'.format(srcpath, destpath))
 +            _ensure_real_directory(os.path.dirname(destpath), os.path.basename(path))
              permissions.append((destpath, os.stat(srcpath).st_mode))
          elif stat.S_ISLNK(mode):
@@ -887,7 +865,6 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result,
                  continue
              target = os.readlink(srcpath)
 -            target = _relative_symlink_target(destdir, destpath, target)
              os.symlink(target, destpath)
          elif stat.S_ISREG(mode):
@@ -925,51 +902,6 @@ def _process_list(srcdir, destdir, filelist, actionfunc, result,
          os.chmod(d, perms)
 -# _relative_symlink_target()
 -#
 -# Fetches a relative path for symlink with an absolute target
 -#
 -# @root:    The staging area root location
 -# @symlink: Location of the symlink in staging area (including the root path)
 -# @target:  The symbolic link target, which may be an absolute path
 -#
 -# If @target is an absolute path, a relative path from the symbolic link
 -# location will be returned, otherwise if @target is a relative path, it will
 -# be returned unchanged.
 -#
 -# Using relative symlinks helps to keep the target self contained when staging
 -# files onto the target.
 -#
 -def _relative_symlink_target(root, symlink, target):
+-
 -    if os.path.isabs(target):
 -        # First fix the input a little, the symlink itself must not have a
 -        # trailing slash, otherwise we fail to remove the symlink filename
 -        # from its directory components in os.path.split()
 -        #
 -        # The absolute target filename must have its leading separator
 -        # removed, otherwise os.path.join() will discard the prefix
 -        symlink = symlink.rstrip(os.path.sep)
 -        target = target.lstrip(os.path.sep)
+-
 -        # We want a relative path from the directory in which symlink
 -        # is located, not from the symlink itself.
 -        symlinkdir, _ = os.path.split(_resolve_symlinks(symlink))
+-
 -        # Create a full path to the target, including the leading staging
 -        # directory
 -        fulltarget = os.path.join(_resolve_symlinks(root), target)
+-
 -        # now get the relative path from the directory where the symlink
 -        # is located within the staging root, to the target within the same
 -        # staging root
 -        newtarget = os.path.relpath(fulltarget, symlinkdir)
+-
 -        return newtarget
 -    else:
 -        return target
+-
+-
  # _set_deterministic_user()
+ #
  # Set the uid/gid for every file in a directory tree to the process'

tests/elements/filter.py

@@ -464,3 +464,23 @@ def test_filter_track_multi_exclude(datafiles, cli, tmpdir):
      assert "ref" not in new_input["sources"][0]
      new_input2 = _yaml.load(input2_file)
      assert new_input2["sources"][0]["ref"] == ref
++
++
 +@pytest.mark.datafiles(os.path.join(DATA_DIR, 'basic'))
 +def test_filter_include_with_indirect_deps(datafiles, cli, tmpdir):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    result = cli.run(project=project, args=[
 +        'build', 'output-include-with-indirect-deps.bst'])
 +    result.assert_success()
++
 +    checkout = os.path.join(tmpdir.dirname, tmpdir.basename, 'checkout')
 +    result = cli.run(project=project, args=[
 +        'artifact', 'checkout', 'output-include-with-indirect-deps.bst', '--directory', checkout])
 +    result.assert_success()
++
 +    # direct dependencies should be staged and filtered
 +    assert os.path.exists(os.path.join(checkout, "baz"))
++
 +    # indirect dependencies shouldn't be staged and filtered
 +    assert not os.path.exists(os.path.join(checkout, "foo"))
 +    assert not os.path.exists(os.path.join(checkout, "bar"))

tests/elements/filter/basic/elements/input-with-deps.bst

 +kind: import
++
 +depends:
 +- filename: input.bst
++
 +sources:
 +- kind: local
 +  path: files
++
 +public:
 +  bst:
 +    split-rules:
 +      baz:
 +      - /baz

tests/elements/filter/basic/elements/output-include-with-indirect-deps.bst

 +kind: filter
++
 +depends:
 +- filename: input-with-deps.bst
 +  type: build

tox.ini

@@ -88,5 +88,5 @@ whitelist_externals =
  commands =
      python3 setup.py --command-packages=click_man.commands man_pages
  deps =
 -    click-man
 +    click-man >= 0.3.0
      -rrequirements/requirements.txt

[Notes] [Git][BuildStream/buildstream][juerg/symlinks] 9 commits: filter.py: don't recurse when staging dependencies

Jürg Billeter pushed to branch juerg/symlinks at BuildStream / buildstream

Commits:

8 changed files:

Changes: