You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by no...@apache.org on 2020/12/29 12:51:25 UTC
[buildstream] branch adamjones/contributing created (now 9beefb8)
This is an automated email from the ASF dual-hosted git repository.
not-in-ldap pushed a change to branch adamjones/contributing
in repository https://gitbox.apache.org/repos/asf/buildstream.git.
at 9beefb8 Using CONTRIBUTING instead of HACKING means Gitlab will automatically track the file via the UI.
This branch includes the following new commits:
new ea79932 Add a section explaining how to open an issue
new 9beefb8 Using CONTRIBUTING instead of HACKING means Gitlab will automatically track the file via the UI.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[buildstream] 02/02: Using CONTRIBUTING instead of HACKING means
Gitlab will automatically track the file via the UI.
Posted by no...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
not-in-ldap pushed a commit to branch adamjones/contributing
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 9beefb832a8d5a8a0b062810f0edff4b63e11fd3
Author: Adam Jones <ad...@codethink.co.uk>
AuthorDate: Fri Sep 14 12:18:01 2018 +0100
Using CONTRIBUTING instead of HACKING means Gitlab
will automatically track the file via the UI.
---
CONTRIBUTING.rst | 614 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 614 insertions(+)
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
new file mode 100644
index 0000000..83abf70
--- /dev/null
+++ b/CONTRIBUTING.rst
@@ -0,0 +1,614 @@
+Contributing
+============
+Some tips and guidelines for developers hacking on BuildStream
+
+
+Feature additions
+-----------------
+Major feature additions should be proposed on the
+`mailing list <https://mail.gnome.org/mailman/listinfo/buildstream-list>`_
+before being considered for inclusion, we strongly recommend proposing
+in advance of commencing work.
+
+If you are experiencing an issue with BuildStream or would like to submit a small patch/feature, then
+you can open issue [here](https://gitlab.com/BuildStream/buildstream/issues/new?issue%5Bassignee_id%5D=&issue%5Bmilestone_id%5D=)
+
+For policies on how to submit and issue and how to use our project labels, we recommend that you read the policies guide
+[here](https://gitlab.com/BuildStream/nosoftware/alignment/blob/master/BuildStream_policies.md)
+
+New features must be well documented and tested either in our main
+test suite if possible, or otherwise in the integration tests.
+
+It is expected that the individual submitting the work take ownership
+of their feature within BuildStream for a reasonable timeframe of at least
+one release cycle after their work has landed on the master branch. This is
+to say that the submitter is expected to address and fix any side effects and
+bugs which may have fell through the cracks in the review process, giving us
+a reasonable timeframe for identifying these.
+
+
+Patch submissions
+-----------------
+Branches must be submitted as merge requests in gitlab and should usually
+be associated to an issue report on gitlab.
+
+Commits in the branch which address specific issues must specify the
+issue number in the commit message.
+
+Merge requests that are not yet ready for review must be prefixed with the
+``WIP:`` identifier. A merge request is not ready for review until the
+submitter expects that the patch is ready to actually land.
+
+Submitted branches must not contain a history of the work done in the
+feature branch. Please use git's interactive rebase feature in order to
+compose a clean patch series suitable for submission.
+
+We prefer that test case and documentation changes be submitted
+in separate commits from the code changes which they test.
+
+Ideally every commit in the history of master passes its test cases. This
+makes bisections more easy to perform, but is not always practical with
+more complex branches.
+
+
+Commit messages
+~~~~~~~~~~~~~~~
+Commit messages must be formatted with a brief summary line, optionally
+followed by an empty line and then a free form detailed description of
+the change.
+
+The summary line must start with what changed, followed by a colon and
+a very brief description of the change.
+
+If there is an associated issue, it **must** be mentioned somewhere
+in the commit message.
+
+**Example**::
+
+ element.py: Added the frobnicator so that foos are properly frobbed.
+
+ The new frobnicator frobnicates foos all the way throughout
+ the element. Elements that are not properly frobnicated raise
+ an error to inform the user of invalid frobnication rules.
+
+ This fixes issue #123
+
+
+Coding style
+------------
+Coding style details for BuildStream
+
+
+Style guide
+~~~~~~~~~~~
+Python coding style for BuildStream is pep8, which is documented here: https://www.python.org/dev/peps/pep-0008/
+
+We have a couple of minor exceptions to this standard, we dont want to compromise
+code readability by being overly restrictive on line length for instance.
+
+The pep8 linter will run automatically when running the test suite.
+
+
+Imports
+~~~~~~~
+Module imports inside BuildStream are done with relative ``.`` notation
+
+Good::
+
+ from .context import Context
+
+Bad::
+
+ from buildstream.context import Context
+
+The exception to the above rule is when authoring plugins,
+plugins do not reside in the same namespace so they must
+address buildstream in the imports.
+
+An element plugin will derive from Element by importing::
+
+ from buildstream import Element
+
+When importing utilities specifically, dont import function names
+from there, instead import the module itself::
+
+ from . import utils
+
+This makes things clear when reading code that said functions
+are not defined in the same file but come from utils.py for example.
+
+
+Policy for private symbols
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+Private symbols are expressed via a leading ``_`` single underscore, or
+in some special circumstances with a leading ``__`` double underscore.
+
+Before understanding the naming policy, it is first important to understand
+that in BuildStream, there are two levels of privateness which need to be
+considered.
+
+These are treated subtly differently and thus need to be understood:
+
+* API Private
+
+ A symbol is considered to be *API private* if it is not exposed in the *public API*.
+
+ Even if a symbol does not have any leading underscore, it may still be *API private*
+ if the containing *class* or *module* is named with a leading underscore.
+
+* Local private
+
+ A symbol is considered to be *local private* if it is not intended for access
+ outside of the defining *scope*.
+
+ If a symbol has a leading underscore, it might not be *local private* if it is
+ declared on a publicly visible class, but needs to be accessed internally by
+ other modules in the BuildStream core.
+
+
+Ordering
+''''''''
+For better readability and consistency, we try to keep private symbols below
+public symbols. In the case of public modules where we may have a mix of
+*API private* and *local private* symbols, *API private* symbols should come
+before *local private* symbols.
+
+
+Symbol naming
+'''''''''''''
+Any private symbol must start with a single leading underscore for two reasons:
+
+* So that it does not bleed into documentation and *public API*.
+
+* So that it is clear to developers which symbols are not used outside of the declaring *scope*
+
+Remember that with python, the modules (python files) are also symbols
+within their containing *package*, as such; modules which are entirely
+private to BuildStream are named as such, e.g. ``_thismodule.py``.
+
+
+Cases for double underscores
+''''''''''''''''''''''''''''
+The double underscore in python has a special function. When declaring
+a symbol in class scope which has a leading underscore, it can only be
+accessed within the class scope using the same name. Outside of class
+scope, it can only be accessed with a *cheat*.
+
+We use the double underscore in cases where the type of privateness can be
+ambiguous.
+
+* For private modules and classes
+
+ We never need to disambiguate with a double underscore
+
+* For private symbols declared in a public *scope*
+
+ In the case that we declare a private method on a public object, it
+ becomes ambiguous whether:
+
+ * The symbol is *local private*, and only used within the given scope
+
+ * The symbol is *API private*, and will be used internally by BuildStream
+ from other parts of the codebase.
+
+ In this case, we use a single underscore for *API private* methods which
+ are not *local private*, and we use a double underscore for *local private*
+ methods declared in public scope.
+
+
+Documenting private symbols
+'''''''''''''''''''''''''''
+Any symbol which is *API Private* (regardless of whether it is also
+*local private*), should have some documentation for developers to
+better understand the codebase.
+
+Contrary to many other python projects, we do not use docstrings to
+document private symbols, but prefer to keep *API Private* symbols
+documented in code comments placed *above* the symbol (or *beside* the
+symbol in some cases, such as variable declarations in a class where
+a shorter comment is more desirable), rather than docstrings placed *below*
+the symbols being documented.
+
+Other than this detail, follow the same guidelines for documenting
+symbols as described below.
+
+
+Documenting BuildStream
+-----------------------
+BuildStream starts out as a documented project from day one and uses
+sphinx to document itself.
+
+
+Documentation formatting policy
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The BuildStream documentation style is as follows:
+
+* Titles and headings require two leading empty lines above them. Only the first word should be capitalized.
+
+ * If there is an ``.. _internal_link`` anchor, there should be two empty lines above the anchor, followed by one leading empty line.
+
+* Within a section, paragraphs should be separated by one empty line.
+
+* Notes are defined using: ``.. note::`` blocks, followed by an empty line and then indented (3 spaces) text.
+
+* Code blocks are defined using: ``.. code:: LANGUAGE`` blocks, followed by an empty line and then indented (3 spaces) text. Note that the default language is `python`.
+
+* Cross references should be of the form ``:role:`target```.
+
+ * To cross reference arbitrary locations with, for example, the anchor ``_anchor_name``, you must give the link an explicit title: ``:ref:`Link text <anchor_name>```. Note that the "_" prefix is not required.
+
+Useful links:
+
+For further information, please see the `Sphinx Documentation <http://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html>`_.
+
+
+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
+
+To build the documentation, just run the following::
+
+ make -C doc
+
+This will give you a ``doc/build/html`` directory with the html docs which
+you can view in your browser locally to test.
+
+
+Regenerating session html
+'''''''''''''''''''''''''
+The documentation build will build the session files if they are missing,
+or if explicitly asked to rebuild. We revision the generated session html files
+in order to reduce the burden on documentation contributors.
+
+To explicitly rebuild the session snapshot html files, it is recommended that you
+first set the ``BST_SOURCE_CACHE`` environment variable to your source cache, this
+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::
+
+ make BST_FORCE_SESSION_REBUILD=1 -C doc
+
+
+Man pages
+~~~~~~~~~
+Unfortunately it is quite difficult to integrate the man pages build
+into the ``setup.py``, as such, whenever the frontend command line
+interface changes, the static man pages should be regenerated and
+committed with that.
+
+To do this, first ensure you have ``click_man`` installed, possibly
+with::
+
+ pip install --user click_man
+
+Then, in the toplevel directory of buildstream, run the following::
+
+ python3 setup.py --command-packages=click_man.commands man_pages
+
+And commit the result, ensuring that you have added anything in
+the ``man/`` subdirectory, which will be automatically included
+in the buildstream distribution.
+
+
+Documenting conventions
+~~~~~~~~~~~~~~~~~~~~~~~
+We use the sphinx.ext.napoleon extension for the purpose of having
+a bit nicer docstrings than the default sphinx docstrings.
+
+A docstring for a method, class or function should have the following
+format::
+
+ """Brief description of entity
+
+ Args:
+ argument1 (type): Description of arg
+ argument2 (type): Description of arg
+
+ Returns:
+ (type): Description of returned thing of the specified type
+
+ Raises:
+ (SomeError): When some error occurs
+ (SomeOtherError): When some other error occurs
+
+ A detailed description can go here if one is needed, only
+ after the above part documents the calling conventions.
+ """
+
+
+Documentation Examples
+~~~~~~~~~~~~~~~~~~~~~~
+The examples section of the documentation contains a series of standalone
+examples, here are the criteria for an example addition.
+
+* The example has a ``${name}``
+
+* The example has a project users can copy and use
+
+ * This project is added in the directory ``doc/examples/${name}``
+
+* The example has a documentation component
+
+ * This is added at ``doc/source/examples/${name}.rst``
+ * A reference to ``examples/${name}`` is added to the toctree in ``doc/source/examples.rst``
+ * This documentation discusses the project elements declared in the project and may
+ provide some BuildStream command examples
+ * This documentation links out to the reference manual at every opportunity
+
+* The example has a CI test component
+
+ * This is an integration test added at ``tests/examples/${name}``
+ * This test runs BuildStream in the ways described in the example
+ and assert that we get the results which we advertize to users in
+ the said examples.
+
+
+Adding BuildStream command output
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+As a part of building the docs, BuildStream will run itself and extract
+some html for the colorized output which is produced.
+
+If you want to run BuildStream to produce some nice html for your
+documentation, then you can do so by adding new ``.run`` files to the
+``doc/sessions/`` directory.
+
+Any files added as ``doc/sessions/${example}.run`` will result in generated
+file at ``doc/source/sessions/${example}.html``, and these files can be
+included in the reStructuredText documentation at any time with::
+
+ .. raw:: html
+ :file: sessions/${example}.html
+
+The ``.run`` file format is just another YAML dictionary which consists of a
+``commands`` list, instructing the program what to do command by command.
+
+Each *command* is a dictionary, the members of which are listed here:
+
+* ``directory``: The input file relative project directory
+
+* ``output``: The input file relative output html file to generate (optional)
+
+* ``fake-output``: Don't really run the command, just pretend to and pretend
+ this was the output, an empty string will enable this too.
+
+* ``command``: The command to run, without the leading ``bst``
+
+When adding a new ``.run`` file, one should normally also commit the new
+resulting generated ``.html`` file(s) into the ``doc/source/sessions-stored/``
+directory at the same time, this ensures that other developers do not need to
+regenerate them locally in order to build the docs.
+
+**Example**:
+
+.. code:: yaml
+
+ commands:
+
+ # Make it fetch first
+ - directory: ../examples/foo
+ command: fetch hello.bst
+
+ # Capture a build output
+ - directory: ../examples/foo
+ output: ../source/sessions/foo-build.html
+ command: build hello.bst
+
+
+Protocol Buffers
+----------------
+BuildStream uses protobuf and gRPC for serialization and communication with
+artifact cache servers. This requires ``.proto`` files and Python code
+generated from the ``.proto`` files using protoc. All these files live in the
+``buildstream/_protos`` directory. The generated files are included in the
+git repository to avoid depending on grpcio-tools for user installations.
+
+
+Regenerating code
+~~~~~~~~~~~~~~~~~
+When ``.proto`` files are modified, the corresponding Python code needs to
+be regenerated. As a prerequisite for code generation you need to install
+``grpcio-tools`` using pip or some other mechanism::
+
+ pip3 install --user grpcio-tools
+
+To actually regenerate the code::
+
+ ./setup.py build_grpc
+
+
+Testing BuildStream
+-------------------
+BuildStream uses 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.
+
+
+Running tests
+~~~~~~~~~~~~~
+To run the tests, just type::
+
+ ./setup.py test
+
+At the toplevel.
+
+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::
+
+ ./setup.py test --addopts -s
+
+You can always abort on the first failure by running::
+
+ ./setup.py test --addopts -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 '-k tests/frontend/'
+
+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'
+
+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'
+
+Alternatively, any IDE plugin that uses pytest should automatically
+detect the ``.pylintrc`` in the project's root directory.
+
+Adding tests
+~~~~~~~~~~~~
+Tests are found in the tests subdirectory, inside of which
+there is a separarate directory for each *domain* of tests.
+All tests are collected as::
+
+ tests/*/*.py
+
+If the new test is not appropriate for the existing test domains,
+then simply create a new directory for it under the tests subdirectory.
+
+Various tests may include data files to test on, there are examples
+of this in the existing tests. When adding data for a test, create
+a subdirectory beside your test in which to store data.
+
+When creating a test that needs data, use the datafiles extension
+to decorate your test case (again, examples exist in the existing
+tests for this), documentation on the datafiles extension can
+be found here: https://pypi.python.org/pypi/pytest-datafiles
+
+Tests that run a sandbox should be decorated with::
+
+ @pytest.mark.integration
+
+and use the integration cli helper.
+
+Measuring BuildStream performance
+---------------------------------
+
+
+Benchmarking framework
+~~~~~~~~~~~~~~~~~~~~~~~
+BuildStream has a utility to measure performance which is available from a
+separate repository at https://gitlab.com/BuildStream/benchmarks. This tool
+allows you to run a fixed set of workloads with multiple versions of
+BuildStream. From this you can see whether one version performs better or
+worse than another which is useful when looking for regressions and when
+testing potential optimizations.
+
+For full documentation on how to use the benchmarking tool see the README in
+the 'benchmarks' repository.
+
+
+Profiling tools
+~~~~~~~~~~~~~~~
+When looking for ways to speed up the code you should make use of a profiling
+tool.
+
+Python provides `cProfile <https://docs.python.org/3/library/profile.html>`_
+which gives you a list of all functions called during execution and how much
+time was spent in each function. Here is an example of running `bst --help`
+under cProfile:
+
+ python3 -m cProfile -o bst.cprofile -- $(which bst) --help
+
+You can then analyze the results interactively using the 'pstats' module:
+
+ python3 -m pstats ./bst.cprofile
+
+For more detailed documentation of cProfile and 'pstats', see:
+https://docs.python.org/3/library/profile.html.
+
+For a richer visualisation of the callstack you can try `Pyflame
+<https://github.com/uber/pyflame>`_. Once you have followed the instructions in
+Pyflame's README to install the tool, you can profile `bst` commands as in the
+following example:
+
+ pyflame --output bst.flame --trace bst --help
+
+You may see an `Unexpected ptrace(2) exception:` error. Note that the `bst`
+operation will continue running in the background in this case, you will need
+to wait for it to complete or kill it. Once this is done, rerun the above
+command which appears to fix the issue.
+
+Once you have output from pyflame, you can use the ``flamegraph.pl`` script
+from the `Flamegraph project <https://github.com/brendangregg/FlameGraph>`_
+to generate an .svg image:
+
+ ./flamegraph.pl bst.flame > bst-flamegraph.svg
+
+The generated SVG file can then be viewed in your preferred web browser.
+
+
+Profiling specific parts of BuildStream with BST_PROFILE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+BuildStream can also turn on cProfile for specific parts of execution
+using BST_PROFILE.
+
+BST_PROFILE can be set to a section name, or 'all' for all
+sections. There is a list of topics in `buildstream/_profile.py`. For
+example, running::
+
+ BST_PROFILE=load-pipeline bst build bootstrap-system-x86.bst
+
+will produce a profile in the current directory for the time take to
+call most of `initialized`, for each element. These profile files
+are in the same cProfile format as those mentioned in the previous
+section, and can be analysed with `pstats` or `pyflame`.
+
+
+Profiling the artifact cache receiver
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Since the artifact cache receiver is not normally run directly, it's
+necessary to alter the ForceCommand part of sshd_config to enable
+profiling. See the main documentation in `doc/source/artifacts.rst`
+for general information on setting up the artifact cache. It's also
+useful to change directory to a logging directory before starting
+`bst-artifact-receive` with profiling on.
+
+This is an example of a ForceCommand section of sshd_config used to
+obtain profiles::
+
+ Match user artifacts
+ ForceCommand BST_PROFILE=artifact-receive cd /tmp && bst-artifact-receive --pull-url https://example.com/ /home/artifacts/artifacts
+
+
+The MANIFEST.in and setup.py
+----------------------------
+When adding a dependency to BuildStream, it's important to update the setup.py accordingly.
+
+When adding data files which need to be discovered at runtime by BuildStream, update setup.py accordingly.
+
+When adding data files for the purpose of docs or tests, or anything that is not covered by
+setup.py, update the MANIFEST.in accordingly.
+
+At any time, running the following command to create a source distribution should result in
+creating a tarball which contains everything we want it to include::
+
+ ./setup.py sdist
[buildstream] 01/02: Add a section explaining how to open an issue
Posted by no...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
not-in-ldap pushed a commit to branch adamjones/contributing
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit ea799329f9d11ed0978b032ee1c4b72ba20a6332
Author: Adam Jones <ad...@codethink.co.uk>
AuthorDate: Fri Sep 14 12:17:09 2018 +0100
Add a section explaining how to open an issue
---
HACKING.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/HACKING.rst b/HACKING.rst
index e658934..83abf70 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -10,6 +10,12 @@ Major feature additions should be proposed on the
before being considered for inclusion, we strongly recommend proposing
in advance of commencing work.
+If you are experiencing an issue with BuildStream or would like to submit a small patch/feature, then
+you can open issue [here](https://gitlab.com/BuildStream/buildstream/issues/new?issue%5Bassignee_id%5D=&issue%5Bmilestone_id%5D=)
+
+For policies on how to submit and issue and how to use our project labels, we recommend that you read the policies guide
+[here](https://gitlab.com/BuildStream/nosoftware/alignment/blob/master/BuildStream_policies.md)
+
New features must be well documented and tested either in our main
test suite if possible, or otherwise in the integration tests.