[Notes] [Git][BuildStream/buildstream][abderrahim/cmake-variable-types]

Abderrahim Kitouni pushed to branch abderrahim/cmake-variable-types at BuildStream / buildstream

Commits:

6f4351ec

by Benjamin Schubert at 2018-11-09T16:53:42Z

source.py: don't let StopIteration propagate to silence() contextmanager

As per PEP 0479 (https://www.python.org/dev/peps/pep-0479/),
StopIteration thrown in context managers are not valid starting from
Python 3.7.

e6e03451

by richardmaw-codethink at 2018-11-12T11:05:15Z

Merge branch 'bschubert/fix-silence-stopiteration-pep-0479' into 'master'

source.py: don't let StopIteration propagate to silence() contextmanager

See merge request BuildStream/buildstream!945

ae5ccd76

by Phillip Smyth at 2018-11-12T14:57:51Z

_project.py: Validate nodes early in Project._load

_project.py: Added validate_nodes() helper function to prevent duplicate lists

element-path was being used before node validation resulting in uncaught errors

56a54161

by Phillip Smyth at 2018-11-12T14:57:51Z

tests/frontend: Add test for invalid element-path

e55a9703

by Jürg Billeter at 2018-11-12T15:44:45Z

Merge branch 'element-path_not_validated' into 'master'

Element path not validated before use

See merge request BuildStream/buildstream!937

e8bc5515

by Abderrahim Kitouni at 2018-11-13T09:37:07Z

plugins/elements/cmake.yaml: always specify variable types

cmake sometimes misinterprets relative paths as relative to the current directory
if this is not specified. See freedesktop-sdk/freedesktop-sdk#431

adjust tests/format/variables.py accordingly.

6 changed files:

buildstream/_project.py
buildstream/plugins/elements/cmake.yaml
buildstream/source.py
tests/format/variables.py
+ tests/frontend/invalid_element_path/project.conf
tests/frontend/show.py

Changes:

buildstream/_project.py

@@ -219,6 +219,19 @@ class Project():
          return self._cache_key
 +    def _validate_node(self, node):
 +        _yaml.node_validate(node, [
 +            'format-version',
 +            'element-path', 'variables',
 +            'environment', 'environment-nocache',
 +            'split-rules', 'elements', 'plugins',
 +            'aliases', 'name',
 +            'artifacts', 'options',
 +            'fail-on-overlap', 'shell', 'fatal-warnings',
 +            'ref-storage', 'sandbox', 'mirrors', 'remote-execution',
 +            'sources', '(@)'
 +        ])
++
      # create_element()
+     #
      # Instantiate and return an element
@@ -402,6 +415,8 @@ class Project():
                  "Project requested format version {}, but BuildStream {}.{} only supports up until format version {}"
                  .format(format_version, major, minor, BST_FORMAT_VERSION))
 +        self._validate_node(pre_config_node)
++
          # FIXME:
+         #
          #   Performing this check manually in the absense
@@ -467,16 +482,7 @@ class Project():
          self._load_pass(config, self.config)
 -        _yaml.node_validate(config, [
 -            'format-version',
 -            'element-path', 'variables',
 -            'environment', 'environment-nocache',
 -            'split-rules', 'elements', 'plugins',
 -            'aliases', 'name',
 -            'artifacts', 'options',
 -            'fail-on-overlap', 'shell', 'fatal-warnings',
 -            'ref-storage', 'sandbox', 'mirrors', 'remote-execution'
 -        ])
 +        self._validate_node(config)
+         #
          # Now all YAML composition is done, from here on we just load

buildstream/plugins/elements/cmake.yaml

@@ -19,7 +19,7 @@ variables:
    cmake-args: |
      -DCMAKE_INSTALL_PREFIX:PATH="%{prefix}" \
 -    -DCMAKE_INSTALL_LIBDIR=%{lib} %{cmake-extra} %{cmake-global} %{cmake-local}
 +    -DCMAKE_INSTALL_LIBDIR:PATH="%{lib}" %{cmake-extra} %{cmake-global} %{cmake-local}
    cmake: |

buildstream/source.py

@@ -973,32 +973,34 @@ class Source(Plugin):
              # the items of source_fetchers, if it happens to be a generator.
+             #
              source_fetchers = iter(source_fetchers)
 -            try:
 -                while True:
 +            while True:
 -                    with context.silence():
 +                with context.silence():
 +                    try:
                          fetcher = next(source_fetchers)
+-
 -                    alias = fetcher._get_alias()
 -                    for uri in project.get_alias_uris(alias, first_pass=self.__first_pass):
 -                        try:
 -                            fetcher.fetch(uri)
 -                        # FIXME: Need to consider temporary vs. permanent failures,
 -                        #        and how this works with retries.
 -                        except BstError as e:
 -                            last_error = e
 -                            continue
+-
 -                        # No error, we're done with this fetcher
 +                    except StopIteration:
 +                        # as per PEP479, we are not allowed to let StopIteration
 +                        # thrown from a context manager.
 +                        # Catching it here and breaking instead.
                          break
 -                    else:
 -                        # No break occurred, raise the last detected error
 -                        raise last_error
 +                alias = fetcher._get_alias()
 +                for uri in project.get_alias_uris(alias, first_pass=self.__first_pass):
 +                    try:
 +                        fetcher.fetch(uri)
 +                    # FIXME: Need to consider temporary vs. permanent failures,
 +                    #        and how this works with retries.
 +                    except BstError as e:
 +                        last_error = e
 +                        continue
 -            except StopIteration:
 -                pass
 +                    # No error, we're done with this fetcher
 +                    break
++
 +                else:
 +                    # No break occurred, raise the last detected error
 +                    raise last_error
          # Default codepath is to reinstantiate the Source
+         #

tests/format/variables.py

@@ -20,7 +20,7 @@ DATA_DIR = os.path.join(
      ('autotools.bst', 'make-install', "make -j1 DESTDIR=\"/buildstream-install\" install"),
      ('cmake.bst', 'cmake',
       "cmake -B_builddir -H\".\" -G\"Unix Makefiles\" " + "-DCMAKE_INSTALL_PREFIX:PATH=\"/usr\" \\\n" +
 -     "-DCMAKE_INSTALL_LIBDIR=lib   "),
 +     "-DCMAKE_INSTALL_LIBDIR:PATH=\"lib\"   "),
      ('distutils.bst', 'python-install',
       "python3 ./setup.py install --prefix \"/usr\" \\\n" +
       "--root \"/buildstream-install\""),
@@ -46,7 +46,7 @@ def test_defaults(cli, datafiles, tmpdir, target, varname, expected):
      ('autotools.bst', 'make-install', "make -j1 DESTDIR=\"/custom/install/root\" install"),
      ('cmake.bst', 'cmake',
       "cmake -B_builddir -H\".\" -G\"Ninja\" " + "-DCMAKE_INSTALL_PREFIX:PATH=\"/opt\" \\\n" +
 -     "-DCMAKE_INSTALL_LIBDIR=lib   "),
 +     "-DCMAKE_INSTALL_LIBDIR:PATH=\"lib\"   "),
      ('distutils.bst', 'python-install',
       "python3 ./setup.py install --prefix \"/opt\" \\\n" +
       "--root \"/custom/install/root\""),

tests/frontend/invalid_element_path/project.conf

 +# Project config for frontend build test
 +name: test
++
 +elephant-path: elements

tests/frontend/show.py

@@ -36,6 +36,19 @@ def test_show(cli, datafiles, target, format, expected):
                               .format(expected, result.output))
 +@pytest.mark.datafiles(os.path.join(
 +    os.path.dirname(os.path.realpath(__file__)),
 +    "invalid_element_path",
 +))
 +def test_show_invalid_element_path(cli, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    result = cli.run(project=project, silent=True, args=[
 +        'show',
 +        "foo.bst"])
++
 +    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)
++
++
  @pytest.mark.datafiles(DATA_DIR)
  @pytest.mark.parametrize("target,except_,expected", [
      ('target.bst', 'import-bin.bst', ['import-dev.bst', 'compose-all.bst', 'target.bst']),

[Notes] [Git][BuildStream/buildstream][abderrahim/cmake-variable-types] 6 commits: source.py: don't let StopIteration propagate to silence() contextmanager

Abderrahim Kitouni pushed to branch abderrahim/cmake-variable-types at BuildStream / buildstream

Commits:

6 changed files:

Changes: