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

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

Commits:

1cbd9a73

by Chandan Singh at 2019-01-02T19:39:58Z

Add requirements files for install, test and plugin dependencies

Add `.in` and `.txt` requirements files for BuildStream's pure python
dependencies. For each pair, the `.in` file is supposed to capture the
loose version requirements, and the corresponding `.txt` file is
supposed to have frozen requirements. We have 3 such sets:

* `requirements`: BuildStream's runtime dependencies
* `dev-requirements`: Dependencies for running tests
* `plugin-requirements`: Dependencies for core plugins

Note that the frozen requirements files will only be used for testing
purposes, and `setup.py` will continue to read loose requirements.

6b0cb5f3

by Chandan Singh at 2019-01-02T19:41:21Z

Add tox.ini to enable running tests using tox

Add `tox.ini` file that will enable us to use
[tox](https://tox.readthedocs.io/) as a frontend for running tests.
Since we share the config via `setup.cfg` and requirements via
requirements files, commands like `python3 setup.py test` will continue
to work.

9d2d1d4f

by Chandan Singh at 2019-01-03T03:31:52Z

.gitlab-ci.yml: Run tests using tox

Instead of invoking tests throung `setup.py`, use `tox` as a frontend in
the CI pipelines.

8ae04283

by Tristan Van Berkom at 2019-01-03T03:31:52Z

CONTRIBUTING.rst: Updated to reflect running tests using tox.

e7e2a5c1

by Chandan Singh at 2019-01-03T03:31:52Z

CONTRIBUTING.rst: Add steps for installing non-python build dependencies

Since we don't allow use of site packages by default in our `tox`
configuration, people will need to install non-python build dependencies
of some of our dependencies that do not provide pre-built wheels. We
have two such packages at the moment:

* psuitl: requires python C headers, compiler
* pygobject: requires python C headers, pkg-config, compiler, cairo
  headers, and gobject libraries

afa0a369

by Chandan Singh at 2019-01-03T03:31:52Z

Move all requirements files into "tools" directory

These new `.in` and `.txt` are making the repository look very
cluttered. Move them to a separate `tools` directory to make it look a
bit cleaner.

3fae3446

by Chandan Singh at 2019-01-03T03:44:17Z

Move sphinx build functionality to tox

Currently the CI and the docs both have to duplicate the same inforation
about how to gather dependencies etc, and have to use hacky ways to run
them.

Add a new `docs` environment to our tox setup so that building docs is
as simple as running `tox -e docs`.

1f7bc655

by Chandan Singh at 2019-01-03T03:44:17Z

CONTRIBUTING.rst: Add instructions to recreate tox environment

When we update our dependencies, developers will eventually need to
recreate their `tox` environments to get correct results. This happens
because `tox` isn't particularly good at recognizing changes in the
requirements files.

669b55b0

by Tristan Van Berkom at 2019-01-03T15:47:27Z

Merge branch 'chandan/toxify' into 'master'

Use tox as a frontend for running tests

See merge request BuildStream/buildstream!1027

ca547f19

by Chandan Singh at 2019-01-03T19:34:49Z

tests/frontend/buildcheckout.py: Fix bad filename issue for Windows

In BuildStream/buildstream!1028, we added a test specifically to test
that BuildStream correctly raises an warning when the name of an element
contains characters that are invalid on Windows. Unfortunately, we
didn't see it coming that it would make it impossible to checkout this
branch on Windows.

Fix it by generating this file, only if we are not running on Windows.

* tests/testutils/site.py: Add `IS_WINDOWS` check
* tests/frontend/buildcheckout.py: Generate file with invalid filename
  on the fly
* Remove tests/frontend/project/elements/invalid-chars.

Fixes #842.

Note that this may still cause issues on WSL when running tests on a
shared filesystem, but that seems to be a generic issue on WSL with
`os.rename`.

15 changed files:

.gitignore
.gitlab-ci.yml
CONTRIBUTING.rst
MANIFEST.in
setup.py
tests/frontend/buildcheckout.py
− tests/frontend/project/elements/invalid-chars|<>-in-name.bst
tests/testutils/site.py
dev-requirements.txt → tools/dev-requirements.in
+ tools/dev-requirements.txt
+ tools/plugin-requirements.in
+ tools/plugin-requirements.txt
+ tools/requirements.in
+ tools/requirements.txt
+ tox.ini

Changes:

.gitignore

@@ -17,6 +17,7 @@ tmp
  .cache
  .pytest_cache/
  *.bst/
 +.tox
  # Pycache, in case buildstream is ran directly from within the source
  # tree

.gitlab-ci.yml

 -image: buildstream/testsuite-debian:9-06bab030-b848172c
 +image: buildstream/testsuite-debian:9-5da27168-32c47d1c
  cache:
    key: "$CI_JOB_NAME-"
@@ -13,7 +13,7 @@ stages:
  variables:
    PYTEST_ADDOPTS: "--color=yes"
    INTEGRATION_CACHE: "${CI_PROJECT_DIR}/cache/integration-cache"
 -  TEST_COMMAND: 'python3 setup.py test --index-url invalid://uri --addopts --integration'
 +  TEST_COMMAND: "tox -- --color=yes --integration"
  #####################################################
  #                  Prepare stage                    #
@@ -68,10 +68,6 @@ source_dist:
    - mount
    - df -h
 -  # Unpack
 -  - cd dist && ./unpack.sh
 -  - cd buildstream
+-
    script:
    - useradd -Um buildstream
    - chown -R buildstream:buildstream .
@@ -83,7 +79,7 @@ source_dist:
    after_script:
    # Collect our reports
    - mkdir -p ${COVERAGE_DIR}
 -  - cp dist/buildstream/.coverage ${COVERAGE_DIR}/coverage."${CI_JOB_NAME}"
 +  - cp .coverage ${COVERAGE_DIR}/coverage."${CI_JOB_NAME}"
    except:
    - schedules
    artifacts:
@@ -91,19 +87,19 @@ source_dist:
      - ${COVERAGE_DIR}
  tests-debian-9:
 -  image: buildstream/testsuite-debian:9-06bab030-b848172c
 +  image: buildstream/testsuite-debian:9-5da27168-32c47d1c
    <<: *tests
  tests-fedora-27:
 -  image: buildstream/testsuite-fedora:27-06bab030-b848172c
 +  image: buildstream/testsuite-fedora:27-5da27168-32c47d1c
    <<: *tests
  tests-fedora-28:
 -  image: buildstream/testsuite-fedora:28-06bab030-b848172c
 +  image: buildstream/testsuite-fedora:28-5da27168-32c47d1c
    <<: *tests
  tests-ubuntu-18.04:
 -  image: buildstream/testsuite-ubuntu:18.04-06bab030-b848172c
 +  image: buildstream/testsuite-ubuntu:18.04-5da27168-32c47d1c
    <<: *tests
  overnight-fedora-28-aarch64:
@@ -120,7 +116,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-06bab030-b848172c
 +  image: buildstream/testsuite-fedora:27-5da27168-32c47d1c
    <<: *tests
    variables:
      BST_FORCE_BACKEND: "unix"
@@ -140,7 +136,7 @@ tests-unix:
  tests-fedora-missing-deps:
    # Ensure that tests behave nicely while missing bwrap and ostree
 -  image: buildstream/testsuite-fedora:28-06bab030-b848172c
 +  image: buildstream/testsuite-fedora:28-5da27168-32c47d1c
    <<: *tests
    script:
@@ -158,20 +154,13 @@ tests-fedora-missing-deps:
  # Automatically build documentation for every commit, we want to know
  # if building documentation fails even if we're not deploying it.
 -# Note: We still do not enforce a consistent installation of python3-sphinx,
 -#       as it will significantly grow the backing image.
  docs:
    stage: test
 +  variables:
 +    BST_FORCE_SESSION_REBUILD: 1
    script:
 -  - export BST_SOURCE_CACHE="$(pwd)/cache/integration-cache/sources"
 -  # Currently sphinx_rtd_theme does not support Sphinx >1.8, this breaks search functionality
 -  - pip3 install sphinx==1.7.9
 -  - pip3 install sphinx-click
 -  - pip3 install sphinx_rtd_theme
 -  - cd dist && ./unpack.sh && cd buildstream
 -  - make BST_FORCE_SESSION_REBUILD=1 -C doc
 -  - cd ../..
 -  - mv dist/buildstream/doc/build/html public
 +  - env BST_SOURCE_CACHE="$(pwd)/cache/integration-cache/sources" tox -e docs
 +  - mv doc/build/html public
    except:
    - schedules
    artifacts:
@@ -275,6 +264,7 @@ coverage:
    coverage: '/TOTAL +\d+ +\d+ +(\d+\.\d+)%/'
    script:
      - cd dist && ./unpack.sh && cd buildstream
 +    - pip3 install -r tools/requirements.txt -r tools/dev-requirements.txt
      - pip3 install --no-index .
      - mkdir report
      - cd report

CONTRIBUTING.rst

@@ -1222,27 +1222,13 @@ For further information about using the reStructuredText with sphinx, please see
  Building Docs
  ~~~~~~~~~~~~~
 -The documentation build is not integrated into the ``setup.py`` and is
 -difficult (or impossible) to do so, so there is a little bit of setup
 -you need to take care of first.
+-
 -Before you can build the BuildStream documentation yourself, you need
 -to first install ``sphinx`` along with some additional plugins and dependencies,
 -using pip or some other mechanism::
+-
 -  # Install sphinx
 -  pip3 install --user sphinx
+-
 -  # Install some sphinx extensions
 -  pip3 install --user sphinx-click
 -  pip3 install --user sphinx_rtd_theme
+-
 -  # Additional optional dependencies required
 -  pip3 install --user arpy
 +Before you can build the docs, you will end to ensure that you have installed
 +the required :ref:`buid dependencies <contributing_build_deps>` as mentioned
 +in the testing section above.
  To build the documentation, just run the following::
 -  make -C doc
 +  tox -e docs
  This will give you a ``doc/build/html`` directory with the html docs which
  you can view in your browser locally to test.
@@ -1260,9 +1246,10 @@ will make the docs build reuse already downloaded sources::
    export BST_SOURCE_CACHE=~/.cache/buildstream/sources
 -To force rebuild session html while building the doc, simply build the docs like this::
 +To force rebuild session html while building the doc, simply run `tox` with the
 +``BST_FORCE_SESSION_REBUILD`` environment variable set, like so::
 -  make BST_FORCE_SESSION_REBUILD=1 -C doc
 +  env BST_FORCE_SESSION_REBUILD=1 tox -e docs
  Man pages
@@ -1468,58 +1455,121 @@ regenerate them locally in order to build the docs.
  Testing
  -------
 -BuildStream uses pytest for regression tests and testing out
 -the behavior of newly added components.
 +BuildStream uses `tox <https://tox.readthedocs.org/>`_ as a frontend to run the
 +tests which are implemented using `pytest <https://pytest.org/>`_. We use
 +pytest for regression tests and testing out the behavior of newly added
 +components.
  The elaborate documentation for pytest can be found here: http://doc.pytest.org/en/latest/contents.html
  Don't get lost in the docs if you don't need to, follow existing examples instead.
 +.. _contributing_build_deps:
++
 +Installing build dependencies
 +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 +Some of BuildStream's dependencies have non-python build dependencies. When
 +running tests with ``tox``, you will first need to install these dependencies.
 +Exact steps to install these will depend on your oprtation systemm. Commands
 +for installing them for some common distributions are lised below.
++
 +For Fedora-based systems::
++
 +  dnf install gcc pkg-config python3-devel cairo-gobject-devel glib2-devel gobject-introspection-devel
++
++
 +For Debian-based systems::
++
 +  apt install gcc pkg-config python3-dev libcairo2-dev libgirepository1.0-dev
++
++
  Running tests
  ~~~~~~~~~~~~~
 -To run the tests, just type::
 +To run the tests, simply navigate to the toplevel directory of your BuildStream
 +checkout and run::
++
 +  tox
 -  ./setup.py test
 +By default, the test suite will be run against every supported python version
 +found on your host. If you have multiple python versions installed, you may
 +want to run tests against only one version and you can do that using the ``-e``
 +option when running tox::
 -At the toplevel.
 +  tox -e py37
 -When debugging a test, it can be desirable to see the stdout
 -and stderr generated by a test, to do this use the ``--addopts``
 -function to feed arguments to pytest as such::
 +The output of all failing tests will always be printed in the summary, but
 +if you want to observe the stdout and stderr generated by a passing test,
 +you can pass the ``-s`` option to pytest as such::
 -  ./setup.py test --addopts -s
 +  tox -- -s
++
 +.. tip::
++
 +   The ``-s`` option is `a pytest option <https://docs.pytest.org/latest/usage.html>`_.
++
 +   Any options specified before the ``--`` separator are consumed by ``tox``,
 +   and any options after the ``--`` separator will be passed along to pytest.
  You can always abort on the first failure by running::
 -  ./setup.py test --addopts -x
 +  tox -- -x
  If you want to run a specific test or a group of tests, you
  can specify a prefix to match. E.g. if you want to run all of
  the frontend tests you can do::
 -  ./setup.py test --addopts 'tests/frontend/'
 +  tox -- tests/frontend/
  Specific tests can be chosen by using the :: delimeter after the test module.
  If you wanted to run the test_build_track test within frontend/buildtrack.py you could do::
 -  ./setup.py test --addopts 'tests/frontend/buildtrack.py::test_build_track'
 +  tox -- tests/frontend/buildtrack.py::test_build_track
  We also have a set of slow integration tests that are disabled by
  default - you will notice most of them marked with SKIP in the pytest
  output. To run them, you can use::
 -  ./setup.py test --addopts '--integration'
 +  tox -- --integration
  By default, buildstream also runs pylint on all files. Should you want
  to run just pylint (these checks are a lot faster), you can do so
  with::
 -  ./setup.py test --addopts '-m pylint'
 +  tox -- -m pylint
  Alternatively, any IDE plugin that uses pytest should automatically
  detect the ``.pylintrc`` in the project's root directory.
 +In case BuildStream's dependencies were updated since you last ran the
 +tests, you might see some errors like
 +``pytest: error: unrecognized arguments: --codestyle``. If this happens, you
 +will need to force ``tox`` to recreate the test environment(s). To do so, you
 +can run ``tox`` with ``-r`` or  ``--recreate`` option.
++
 +.. note::
++
 +   By default, we do not allow use of site packages in our ``tox``
 +   confguration to enable running the tests in an isolated environment.
 +   If you need to enable use of site packages for whatever reason, you can
 +   do so by passing the ``--sitepackages`` option to ``tox``. Also, you will
 +   not need to install any of the build dependencies mentioned above if you
 +   use this approach.
++
 +.. note::
++
 +   While using ``tox`` is practical for developers running tests in
 +   more predictable execution environments, it is still possible to
 +   execute the test suite against a specific installation environment
 +   using pytest directly::
++
 +     ./setup.py test
++
 +   Specific options can be passed to ``pytest`` using the ``--addopts``
 +   option::
++
 +     ./setup.py test --addopts 'tests/frontend/buildtrack.py::test_build_track'
++
  Adding tests
  ~~~~~~~~~~~~

MANIFEST.in

@@ -24,6 +24,7 @@ recursive-include doc/sessions *.run
  # Tests
  recursive-include tests *
  include conftest.py
 +include tox.ini
  include .coveragerc
  include .pylintrc
@@ -31,7 +32,12 @@ include .pylintrc
  recursive-include buildstream/_protos *.proto
  # Requirements files
 -include dev-requirements.txt
 +include tools/requirements.in
 +include tools/requirements.txt
 +include tools/dev-requirements.in
 +include tools/dev-requirements.txt
 +include tools/plugin-requirements.in
 +include tools/plugin-requirements.txt
  # Versioneer
  include versioneer.py

setup.py

@@ -270,9 +270,12 @@ def get_cmdclass():
  #####################################################
  #               Gather requirements                 #
  #####################################################
 -with open('dev-requirements.txt') as dev_reqs:
 +with open('tools/dev-requirements.in') as dev_reqs:
      dev_requires = dev_reqs.read().splitlines()
 +with open('tools/requirements.in') as install_reqs:
 +    install_requires = install_reqs.read().splitlines()
++
  #####################################################
  #     Prepare package description from README       #
  #####################################################
@@ -334,23 +337,7 @@ setup(name='BuildStream',
                os.path.join('buildstream', 'data', 'bst')
            ])
        ],
 -      install_requires=[
 -          'setuptools',
 -          'psutil',
 -          # According to ruamel.yaml's PyPI page, we are suppose to use
 -          # "<=0.15" in production until 0.15 becomes API stable.
 -          # However we need ruamel.yaml 0.15.41 or greater for Python 3.7.
 -          # We know that ruamel.yaml 0.15.52 breaks API in a way that
 -          # is incompatible with BuildStream.
 -          #
 -          # See issues #571 and #790.
 -          'ruamel.yaml >= 0.15.41, < 0.15.52',
 -          'pluginbase',
 -          'Click >= 7.0',
 -          'jinja2 >= 2.10',
 -          'protobuf >= 3.5',
 -          'grpcio >= 1.10',
 -      ],
 +      install_requires=install_requires,
        entry_points=bst_install_entry_points,
        tests_require=dev_requires,
        zip_safe=False)

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,16 +86,37 @@ 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)
 -    result = cli.run(project=project, args=strict_args(['build', 'invalid-chars|<>-in-name.bst'], 'non-strict'))
 +    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")

tests/frontend/project/elements/invalid-chars|<>-in-name.bst deleted

 -kind: stack
 -description: |
 -  The name of this files contains characters that are not allowed by
 -  BuildStream, using it should raise a warning.

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()

dev-requirements.txt → tools/dev-requirements.in

tools/dev-requirements.txt

 +coverage==4.4
 +pylint==2.2.2
 +pytest==4.0.2
 +pytest-codestyle==1.4.0
 +pytest-cov==2.6.0
 +pytest-datafiles==2.0
 +pytest-env==0.6.2
 +pytest-pylint==0.13.0
 +pytest-xdist==1.25.0
 +pytest-timeout==1.3.3
 +pyftpdlib==1.5.4
 +## The following requirements were added by pip freeze:
 +apipkg==1.5
 +astroid==2.1.0
 +atomicwrites==1.2.1
 +attrs==18.2.0
 +execnet==1.5.0
 +isort==4.3.4
 +lazy-object-proxy==1.3.1
 +mccabe==0.6.1
 +more-itertools==5.0.0
 +pluggy==0.8.0
 +py==1.7.0
 +pycodestyle==2.4.0
 +pytest-forked==0.2
 +six==1.12.0
 +typed-ast==1.1.1
 +wrapt==1.10.11

tools/plugin-requirements.in

	1	+arpy
	2	+PyGObject

tools/plugin-requirements.txt

 +arpy==1.1.1
 +PyGObject==3.30.4
 +## The following requirements were added by pip freeze:
 +pycairo==1.18.0

tools/requirements.in

 +Click
 +grpcio >= 1.10
 +jinja2 >= 2.10
 +pluginbase
 +protobuf >= 3.5
 +psutil
 +# According to ruamel.yaml's PyPI page, we are suppose to use
 +# "<=0.15" in production until 0.15 becomes API stable.
 +# However we need ruamel.yaml 0.15.41 or greater for Python 3.7.
 +# We know that ruamel.yaml 0.15.52 breaks API in a way that
 +# is incompatible with BuildStream.
 +#
 +# See issues #571 and #790.
 +ruamel.yaml >= 0.15.41, < 0.15.52
 +setuptools

tools/requirements.txt

 +Click==7.0
 +grpcio==1.17.1
 +pluginbase==0.7
 +protobuf==3.6.1
 +psutil==5.4.8
 +# According to ruamel.yaml's PyPI page, we are suppose to use
 +# "<=0.15" in production until 0.15 becomes API stable.
 +# However we need ruamel.yaml 0.15.41 or greater for Python 3.7.
 +# We know that ruamel.yaml 0.15.52 breaks API in a way that
 +# is incompatible with BuildStream.
 +#
 +# See issues #571 and #790.
 +ruamel.yaml==0.15.51
 +setuptools==39.0.1
 +## The following requirements were added by pip freeze:
 +Jinja2==2.10
 +MarkupSafe==1.1.0
 +six==1.12.0

tox.ini

 +[tox]
 +envlist = py35,py36,py37
 +skip_missing_interpreters = true
++
 +[testenv]
 +commands = pytest {posargs}
 +deps =
 +    -rtools/requirements.txt
 +    -rtools/dev-requirements.txt
 +    -rtools/plugin-requirements.txt
 +passenv =
 +    BST_FORCE_BACKEND
 +    GI_TYPELIB_PATH
 +    INTEGRATION_CACHE
++
 +[testenv:docs]
 +commands =
 +    make -C doc
 +# Currently sphinx_rtd_theme does not support Sphinx >1.8, this breaks search functionality
 +deps =
 +    sphinx==1.7.9
 +    sphinx-click
 +    sphinx_rtd_theme
 +    -rtools/requirements.txt
 +    -rtools/plugin-requirements.txt
 +passenv =
 +    BST_FORCE_SESSION_REBUILD
 +    BST_SOURCE_CACHE
 +    HOME
 +    LANG
 +    LC_ALL
 +whitelist_externals =
 +    make

[Notes] [Git][BuildStream/buildstream][chandan/fix-bad-filename-mini-disaster] 10 commits: Add requirements files for install, test and plugin dependencies

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

Commits:

15 changed files:

Changes: