[Notes] [Git][BuildStream/buildstream][chandan/fix-bad-filename-disaster

Chandan Singh pushed to branch chandan/fix-bad-filename-disaster at BuildStream / buildstream

Commits:

3295f9b5

by Chandan Singh at 2019-01-03T00:32:45Z

tests/buildcheckout.py: Add tests for invalid chars in filename

This commit is the first one in the chain to rewrite history since
one of the test files was commit, in error, with a name that is invalid
on Windows. Hence, it must be skipped on Windows.

Please see https://gitlab.com/BuildStream/buildstream/issues/842 for
more details.

142fafbf
by Chandan Singh at 2019-01-03T00:32:45Z
```
doc: Add note about valid element names
```

0e8ff35b

by Chandan Singh at 2019-01-03T00:32:45Z

doc/Makefile: Update comment about sphinx entrypoint

https://github.com/sphinx-doc/sphinx/issues/4375 has been closed as
`wontfix`. Update the comment in doc/Makefile that mentions it, based on
the discussion on the issue.

298cf7f6

by Tristan Maat at 2019-01-03T00:32:45Z

.gitlab-ci.yml: Use new testsuite images with pycodestyle 1.4.0

We need to bump the testsuite image versions to use the new
pycodestyle update that allows excluding file paths, so that we can
exclude generated python code.

07f8692b

by Javier Jardón at 2019-01-03T00:32:45Z

Use pycodestyle instead of pep8

pep8 package was renamed to pycodestyle to reduce confusion.

Disabled warnings:
W504 - Line break after binary operator (seems people like this)
W605 - Invalid escape sequence (some of our regexes use these)

13 changed files:

.gitlab-ci.yml
buildstream/_ostree.py
buildstream/_signals.py
buildstream/element.py
dev-requirements.txt
doc/Makefile
doc/source/format_declaring.rst
setup.cfg
tests/frontend/buildcheckout.py
+ tests/frontend/project/elements/invalid-chars-in-dep.bst
tests/frontend/project/project.conf
tests/testutils/http_server.py
tests/testutils/site.py

Changes:

.gitlab-ci.yml

 -image: buildstream/testsuite-debian:9-master-123-7ce6581b
 +image: buildstream/testsuite-debian:9-06bab030-b848172c
  cache:
    key: "$CI_JOB_NAME-"
@@ -91,23 +91,23 @@ source_dist:
      - ${COVERAGE_DIR}
  tests-debian-9:
 -  image: buildstream/testsuite-debian:9-master-123-7ce6581b
 +  image: buildstream/testsuite-debian:9-06bab030-b848172c
    <<: *tests
  tests-fedora-27:
 -  image: buildstream/testsuite-fedora:27-master-123-7ce6581b
 +  image: buildstream/testsuite-fedora:27-06bab030-b848172c
    <<: *tests
  tests-fedora-28:
 -  image: buildstream/testsuite-fedora:28-master-123-7ce6581b
 +  image: buildstream/testsuite-fedora:28-06bab030-b848172c
    <<: *tests
  tests-ubuntu-18.04:
 -  image: buildstream/testsuite-ubuntu:18.04-master-123-7ce6581b
 +  image: buildstream/testsuite-ubuntu:18.04-06bab030-b848172c
    <<: *tests
  overnight-fedora-28-aarch64:
 -  image: buildstream/testsuite-fedora:aarch64-28-master-123-7ce6581b
 +  image: buildstream/testsuite-fedora:aarch64-28-06bab030-32a101f6
    tags:
      - aarch64
    <<: *tests
@@ -120,7 +120,7 @@ overnight-fedora-28-aarch64:
  tests-unix:
    # Use fedora here, to a) run a test on fedora and b) ensure that we
    # can get rid of ostree - this is not possible with debian-8
 -  image: buildstream/testsuite-fedora:27-master-123-7ce6581b
 +  image: buildstream/testsuite-fedora:27-06bab030-b848172c
    <<: *tests
    variables:
      BST_FORCE_BACKEND: "unix"
@@ -140,7 +140,7 @@ tests-unix:
  tests-fedora-missing-deps:
    # Ensure that tests behave nicely while missing bwrap and ostree
 -  image: buildstream/testsuite-fedora:28-master-123-7ce6581b
 +  image: buildstream/testsuite-fedora:28-06bab030-b848172c
    <<: *tests
    script:

buildstream/_ostree.py

@@ -34,7 +34,7 @@ from ._exceptions import BstError, ErrorDomain
  # pylint: disable=wrong-import-position,wrong-import-order
  gi.require_version('OSTree', '1.0')
 -from gi.repository import GLib, Gio, OSTree  # nopep8
 +from gi.repository import GLib, Gio, OSTree  # noqa
  # For users of this file, they must expect (except) it.

buildstream/_signals.py

@@ -38,7 +38,7 @@ def terminator_handler(signal_, frame):
          terminator_ = terminator_stack.pop()
          try:
              terminator_()
 -        except:                                                 # pylint: disable=bare-except
 +        except:                               # noqa pylint: disable=bare-except
              # Ensure we print something if there's an exception raised when
              # processing the handlers. Note that the default exception
              # handler won't be called because we os._exit next, so we must

buildstream/element.py

@@ -1568,7 +1568,7 @@ class Element(Plugin):
                  utils._force_rmtree(rootdir)
              with _signals.terminator(cleanup_rootdir), \
 -                self.__sandbox(rootdir, output_file, output_file, self.__sandbox_config) as sandbox:  # nopep8
 +                self.__sandbox(rootdir, output_file, output_file, self.__sandbox_config) as sandbox:  # noqa
                  # By default, the dynamic public data is the same as the static public data.
                  # The plugin's assemble() method may modify this, though.

dev-requirements.txt

  coverage == 4.4.0
 -pep8
  pylint
  pytest >= 3.9
 +pytest-codestyle >= 1.4.0
  pytest-cov >= 2.5.0
  pytest-datafiles >= 2.0
  pytest-env
 -pytest-pep8
  pytest-pylint
  pytest-xdist
  pytest-timeout

doc/Makefile

@@ -6,10 +6,10 @@
  # we dont use the standard `sphinx-build` and `sphinx-apidoc` entry points.
+ #
  # The following technique works as long as sphinx is installed for python3,
 -# regardless of the entry point which might be in /usr/bin or PATH, but
 -# will stop working in sphinx >= 2.0. Hopefully by then, the mentioned
 -# bug will be fixed and we can use a standard python3 specific script to
 -# invoke sphnix.
 +# regardless of the entry point which might be in /usr/bin or PATH.
 +#
 +# Since Sphinx 2.0 is planned to be Python 3-only, this workaround should not
 +# be needed once Spinx 2.0 is released, and we upgrade to it
+ #
  SPHINXOPTS    =
  SPHINXBUILD   = python3 -m sphinx

doc/source/format_declaring.rst

@@ -526,3 +526,27 @@ read-only variables are also dynamically declared by BuildStream:
    build, support for this is conditional on the element type
    and the build system used (any element using 'make' can
    implement this).
++
++
 +Naming elements
 +---------------
 +When naming the element files, use the following rules:
++
 +* The name of the file must have ``.bst`` extension.
++
 +* All characters in the name must be printable 7-bit ASCII characters.
++
 +* Following characters are reserved and must not be part of the name:
++
 +  - ``<`` (less than)
 +  - ``>`` (greater than)
 +  - ``:`` (colon)
 +  - ``"`` (double quote)
 +  - ``/`` (forward slash)
 +  - ``\`` (backslash)
 +  - ``|`` (vertical bar)
 +  - ``?`` (question mark)
 +  - ``*`` (asterisk)
++
 +BuildStream will attempt to raise warnings when any of these rules are violated
 +but that may not always be possible.

setup.cfg

@@ -11,20 +11,11 @@ parentdir_prefix = BuildStream-
  test=pytest
  [tool:pytest]
 -addopts = --verbose --basetemp ./tmp --pep8 --pylint --pylint-rcfile=.pylintrc --cov=buildstream --cov-config .coveragerc --durations=20
 +addopts = --verbose --basetemp ./tmp --codestyle --pylint --pylint-rcfile=.pylintrc --cov=buildstream --cov-config .coveragerc --durations=20
  norecursedirs = tests/integration/project integration-cache tmp __pycache__ .eggs
  python_files = tests/*/*.py
 -pep8maxlinelength = 119
 -pep8ignore =
 -    * E129
 -    * E125
 -    doc/source/conf.py ALL
 -    tmp/* ALL
 -    */lib/python3* ALL
 -    */bin/* ALL
 -    buildstream/_fuse/fuse.py ALL
 -    .eggs/* ALL
 -    *_pb2.py ALL
 -    *_pb2_grpc.py ALL
 +codestyle_max_line_length = 119
 +codestyle_ignore = E129 E125 W504 W605
 +codestyle_exclude = doc/source/conf.py buildstream/_fuse/fuse.py buildstream/_protos/**/*py
  env =
      D:BST_TEST_SUITE=True

tests/frontend/buildcheckout.py

@@ -3,6 +3,7 @@ import tarfile
  import hashlib
  import pytest
  from tests.testutils import cli, create_repo, ALL_REPO_KINDS, generate_junction
 +from tests.testutils.site import IS_WINDOWS
  from buildstream import _yaml
  from buildstream._exceptions import ErrorDomain, LoadErrorReason
@@ -85,6 +86,41 @@ def test_build_invalid_suffix_dep(datafiles, cli, strict, hardlinks):
      result.assert_main_error(ErrorDomain.LOAD, "bad-element-suffix")
 +@pytest.mark.skipif(IS_WINDOWS, reason='Not available on Windows')
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_build_invalid_filename_chars(datafiles, cli):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    element_name = 'invalid-chars|<>-in-name.bst'
++
 +    # The name of this file contains characters that are not allowed by
 +    # BuildStream, using it should raise a warning.
 +    element = {
 +        'kind': 'stack',
 +    }
 +    _yaml.dump(element, os.path.join(project, 'elements', element_name))
++
 +    result = cli.run(project=project, args=strict_args(['build', element_name], 'non-strict'))
 +    result.assert_main_error(ErrorDomain.LOAD, "bad-characters-in-name")
++
++
 +@pytest.mark.skipif(IS_WINDOWS, reason='Not available on Windows')
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_build_invalid_filename_chars_dep(datafiles, cli):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    element_name = 'invalid-chars|<>-in-name.bst'
++
 +    # The name of this file contains characters that are not allowed by
 +    # BuildStream, and is listed as a dependency of 'invalid-chars-in-dep.bst'.
 +    # This should also raise a warning.
 +    element = {
 +        'kind': 'stack',
 +    }
 +    _yaml.dump(element, os.path.join(project, 'elements', element_name))
++
 +    result = cli.run(project=project, args=strict_args(['build', 'invalid-chars-in-dep.bst'], 'non-strict'))
 +    result.assert_main_error(ErrorDomain.LOAD, "bad-characters-in-name")
++
++
  @pytest.mark.datafiles(DATA_DIR)
  @pytest.mark.parametrize("deps", [("run"), ("none"), ("build")])
  def test_build_checkout_deps(datafiles, cli, deps):

tests/frontend/project/elements/invalid-chars-in-dep.bst

 +kind: stack
 +description: |
++
 +  This element itself has a valid name, but depends on elements that have
 +  invalid names. This should also result in a warning.
++
 +depends:
 +- invalid-chars|<>-in-name.bst

tests/frontend/project/project.conf

@@ -5,3 +5,4 @@ element-path: elements
  fatal-warnings:
  - bad-element-suffix
 +- bad-characters-in-name

tests/testutils/http_server.py

@@ -29,7 +29,7 @@ class RequestHandler(SimpleHTTPRequestHandler):
                  expected_password, directory = self.server.users[user]
                  if password == expected_password:
                      return directory
 -            except:
 +            except:                           # noqa
                  raise Unauthorized('unauthorized')
              return None

tests/testutils/site.py

@@ -52,5 +52,6 @@ except ImportError:
      HAVE_ARPY = False
  IS_LINUX = os.getenv('BST_FORCE_BACKEND', sys.platform).startswith('linux')
 +IS_WINDOWS = (os.name == 'nt')
  MACHINE_ARCH = Platform.get_host_arch()

[Notes] [Git][BuildStream/buildstream][chandan/fix-bad-filename-disaster] 5 commits: tests/buildcheckout.py: Add tests for invalid chars in filename

Chandan Singh pushed to branch chandan/fix-bad-filename-disaster at BuildStream / buildstream

Commits:

13 changed files:

Changes: