diff options
Diffstat (limited to 'doc/dev_guide')
| -rw-r--r-- | doc/dev_guide/building_from_source.rst | 288 | ||||
| -rw-r--r-- | doc/dev_guide/building_with_make.rst | 195 | ||||
| -rw-r--r-- | doc/dev_guide/development_guide.rst | 342 | ||||
| -rw-r--r-- | doc/dev_guide/index.rst | 9 |
4 files changed, 834 insertions, 0 deletions
diff --git a/doc/dev_guide/building_from_source.rst b/doc/dev_guide/building_from_source.rst new file mode 100644 index 000000000..64250dd09 --- /dev/null +++ b/doc/dev_guide/building_from_source.rst @@ -0,0 +1,288 @@ +Building from source +==================== + +You're going to need the following tools to get started: + +* gcc or clang +* meson +* ninja +* pkg-config +* sphinx-build* + +| \* optional, to build man-pages and html documentation + +And the following dependencies: + +* cmocka [#b1]_ +* linux-headers [#b2]_ +* libpci [#b2]_ +* libusb1 [#b2]_ +* libftdi1 [#b2]_ +* libjaylink [#b2]_ +* NI-845x driver & library package [#b3]_ + +.. [#b1] | optional, for building unit testing +.. [#b2] | optional, depending on the selected programmer +.. [#b3] | optional, proprietary and Windows only. (See Windows build instructions) + +If you are cross compiling, install the dependencies for your target. + +TL;DR +----- +:: + + meson setup builddir + meson compile -C builddir + meson install -C builddir + + +.. _installing-dependencies: + +Installing dependencies +----------------------- + +.. todo:: Move the bullet points to `tabs <https://www.w3schools.com/howto/howto_js_tabs.asp>`_ + + * No external dependencies (documentation should be build without fetching all of pypi) + * No Javascript? + +* Linux + * Debian / Ubuntu + :: + + apt-get install -y \ + gcc meson ninja-build pkg-config python3-sphinx \ + libcmocka-dev libpci-dev libusb-1.0-0-dev libftdi1-dev libjaylink-dev + + * ArchLinux / Manjaro + :: + + pacman -S --noconfirm \ + gcc meson ninja pkg-config python-sphinx cmocka \ + pciutils libusb libftdi libjaylink + + * openSUSE / SUSE + :: + + zypper install -y \ + gcc meson ninja pkg-config python3-Sphinx \ + libcmocka-devel pciutils-devel libusb-1_0-devel libftdi1-devel libjaylink-devel + + * NixOS / nixpkgs + * There is a ``shell.nix`` under ``scripts/`` + + :: + + nix-shell -p \ + gcc meson ninja pkg-config sphinx \ + cmocka pciutils libusb1 libftdi1 libjaylink + + * Alpine Linux + :: + + apk add \ + build-base meson ninja pkgconf py3-sphinx \ + cmocka-dev pciutils-dev libusb-dev libjaylink-dev + +* Windows + * MSYS2 + Install `MSYS2 <https://www.msys2.org/>`_ and ensure it is `fully updated <https://www.msys2.org/docs/updating/>`_. + + * ``libpci`` is not available through the package manager and pci based programmer are not supported on Windows. + * ``ni845x_spi`` is only available with the proprietary library from National Instruments. Download and install the driver + from `ni.com <https://www.ni.com/en-us/support/downloads/drivers/download.ni-845x-driver-software.html>`_ and build flashrom + for **32-bit**. Add ``-Dprogrammer=ni845x_spi`` to your meson configuration. + + In the MINGW64 shell run:: + + pacman -Sy \ + mingw-w64-x86_64-gcc mingw-w64-x86_64-meson mingw-w64-x86_64-ninja mingw-w64-x86_64-pkg-config mingw-w64-x86_64-python-sphinx \ + mingw-w64-x86_64-cmocka mingw-w64-x86_64-libusb mingw-w64-x86_64-libftdi mingw-w64-x86_64-libjaylink-git + + For building flashrom as 32-bit application, use the MSYS2 MINGW32 shell and run:: + + pacman -Sy \ + mingw-w64-i686-gcc mingw-w64-i686-meson mingw-w64-i686-ninja mingw-w64-i686-pkg-config mingw-w64-i686-python-sphinx \ + mingw-w64-i686-cmocka mingw-w64-i686-libusb mingw-w64-i686-libftdi mingw-w64-i686-libjaylink-git + +* MacOS + * Homebrew + * ``libpci`` is not available through the package manager + * ``libjaylink`` is not available through the package manager + + :: + + brew install \ + meson ninja pkg-config sphinx-doc \ + libusb libftdi + +* BSD + * FreeBSD / DragonFlyBSD + * ``libusb1`` is part of the system + * ``libjaylink`` is not available through the package manager + + :: + + pkg install \ + meson ninja pkgconf py39-sphinx \ + cmocka libpci libftdi1 + + * OpenBSD + * ``libjaylink`` is not available through the package manager + + :: + + pkg_add \ + meson ninja pkg-config py39-sphinx\ + cmocka pciutils libusb1 libftdi1 + + * NetBSD + * ``libjaylink`` is not available through the package manager + * note: https://www.cambus.net/installing-ca-certificates-on-netbsd/ + + :: + + pkgin install \ + meson ninja pkg-config py39-sphinx \ + cmocka pciutils libusb1 libftdi1 + +* OpenIndiana (Illumos, Solaris, SunOS) + * ``libpci`` missing, pciutils is build without it + * ``libftdi1`` & ``libjaylink`` are not available through the package manager + * TODO: replace ``build-essential`` with the default compiler + + :: + + pkg install build-essential meson ninja cmocka libusb-1 + +* DJGPP-DOS + * Get `DJGPP <https://www.delorie.com/djgpp/>`_ + * A great build script can be found `here <https://github.com/andrewwutw/build-djgpp>`_ + * Download the `pciutils <https://mj.ucw.cz/sw/pciutils/>`_ sources + + | Run the following commands in the the pciutils directory to build libpci for DOS. + | Replace ``<DOS_INSTALL_ROOT>`` with your cross-compile install root. + + :: + + make install-lib \ + ZLIB=no \ + DNS=no \ + HOST=i386-djgpp-djgpp \ + CROSS_COMPILE=i586-pc-msdosdjgpp- \ + STRIP="--strip-program=i586-pc-msdosdjgpp-strip -s" \ + PREFIX=<DOS_INSTALL_ROOT> + + Point pkg-config to the ``<DOS_INSTALL_ROOT>`` :: + + export PKG_CONFIG_SYSROOT=<DOS_INSTALL_ROOT> + + * To compile flashrom use the ``meson_cross/i586_djgpp_dos.txt`` cross-file + * You will need `CWSDPMI.EXE <https://sandmann.dotster.com/cwsdpmi/>`_ to run flashrom + +* libpayload + .. todo:: Add building instructions for libpayload + + +Configuration +------------- +In the flashrom repository run:: + + meson setup [builtin options] [flashrom options] <builddir> + +Mesons ``[builtin options]`` can be displayed with ``meson setup --help``. +The flashrom specific options can be found in ``meson_options.txt`` in the top-level +directory of flashrom and are used like in cmake with ``-Doption=value`` +Run ``meson configure`` to display all configuration options. + +.. todo:: Write a sphinx extension to render ``meson_options.txt`` here + + +Configuration for Crossbuilds +----------------------------- +Flashrom specific cross-files can be found in the ``meson_cross`` folder. +To use them run:: + + meson setup --cross-file <path/to/crossfile> [builtin options] [flashrom options] <builddir> + +The options are the same as the normal configuration options. For more information see +https://mesonbuild.com/Cross-compilation.html + + +Compiling +--------- +Run:: + + meson compile -C <builddir> + + +Update configuration +-------------------- +If you want to change your initial configuration for some reason +(for example you discovered that a programmer is missing), run:: + + meson configure [updated builtin options] [updated flashrom options] <builddir> + + +Unit Tests +---------- +To execute the unit tests run:: + + meson test -C <builddir> + +You will get a summary of the unit test results at the end. + + +Code coverage +""""""""""""" +gcov + Due to a bug in lcov, the html file will only be correct if lcov is not + installed and gcovr is installed. See + https://github.com/linux-test-project/lcov/issues/168 and + https://github.com/mesonbuild/meson/issues/6747 + + To create the coverage target add ``-Db_coverage=true`` to your build configuration. + After executing the tests, you can run :: + + ninja -C <builddir> coverage + + to generate the coverage report. + +lcov / llvm + https://clang.llvm.org/docs/SourceBasedCodeCoverage.html + Make sure that you are using `clang` as compiler, e.g. by setting `CC=clang` during configuration. + Beside that you need to add ``-Dllvm_cov=enabled`` to your build configuration :: + + CC=clang meson setup -Dllvm_cov=enable <builddir> + meson test -C <builddir> + ninja -C <builddir> llvm-cov-tests + +For additional information see `the meson documentation <https://mesonbuild.com/Unit-tests.html#coverage>`_ + + +Installing +---------- +To install flashrom and documentation, run:: + + meson install -C <builddir> + +This will install flashrom under the PREFIX selected in the configuration phase. Default is ``/usr/local``. + +To install into a different directory use DESTDIR, like this:: + + DESTDIR=/your/destination/directory meson install -C <your_build_dir> + +You can also set the prefix during configuration with:: + + meson setup --prefix <DESTDIR> <your_build_dir> + +Create distribution package +--------------------------- +To create a distribution tarball from your ``builddir``, run:: + + meson dist -C <builddir> + +This will collect all git tracked files and pack them into an archive. + +Current flashrom version is in the VERSION file. To release a new flashrom +version you need to change VERSION file and tag the changing commit. diff --git a/doc/dev_guide/building_with_make.rst b/doc/dev_guide/building_with_make.rst new file mode 100644 index 000000000..710aad3d1 --- /dev/null +++ b/doc/dev_guide/building_with_make.rst @@ -0,0 +1,195 @@ +Building with make +================== + +TLDR +---- + +:: + + make + make install + +Build instructions +------------------ + +**To build flashrom you need to install the following software:** + + * C compiler (GCC / clang) + * pkg-config + + * pciutils+libpci (if you want support for mainboard or PCI device flashing) + * libusb (if you want FT2232, Dediprog or USB-Blaster support) + * libftdi (if you want FT2232 or USB-Blaster support) + * libjaylink (if you want support for SEGGER J-Link and compatible devices) + * NI-845x driver & library package (if you want support for NI-845x devices; uses a proprietary driver) + +**Linux et al:** + + * pciutils / libpci + * pciutils-devel / pciutils-dev / libpci-dev + * zlib-devel / zlib1g-dev (needed if libpci was compiled with libz support) + +**On FreeBSD, you need the following ports:** + + * devel/gmake + * devel/libpci + +**On OpenBSD, you need the following ports:** + + * devel/gmake + * sysutils/pciutils + +**To compile on Linux, use**:: + + make + +**To compile on FreeBSD, OpenBSD or DragonFly BSD, use**:: + + gmake + +**To compile on Nexenta, use**:: + + make + +**To compile on Solaris, use**:: + + gmake LDFLAGS="-L$pathtolibpci" CC="gcc -I$pathtopciheaders" CFLAGS=-O2 + +**To compile on NetBSD (with pciutils, libftdi, libusb installed in /usr/pkg/), use**:: + + gmake + +**To compile and run on Darwin/Mac OS X:** + +Install DirectHW from coresystems GmbH. +DirectHW is available at https://www.coreboot.org/DirectHW . + +**To compile on Windows:** + +Install MSYS tools (and the NI-845x drivers if desired) as described in +:ref:`installing-dependencies`. + +To build with support for NI-845x:: + + make HAS_LIB_NI845X=yes CONFIG_NI845X_SPI=yes + +**To cross-compile on Linux for DOS:** + +Get packages of the DJGPP cross compiler and install them: + + * djgpp-filesystem djgpp-gcc djgpp-cpp djgpp-runtime djgpp-binutils + +As an alternative, the DJGPP web site offers packages for download as well: + + * djcross-binutils-2.29.1-1ap.x86_64.rpm + * djcross-gcc-7.2.0-1ap.x86_64.rpm + * djcrx-2.05-5.x86_64.rpm + +The cross toolchain packages for your distribution may have slightly different +names (look for packages named *djgpp*). + +Alternatively, you could use a script to build it from scratch: +https://github.com/andrewwutw/build-djgpp + +You will need the libpci and libgetopt library source trees and +their compiled static libraries and header files installed in some +directory say libpci-libgetopt/, which will be later specified with +LIBS_BASE parameter during flashrom compilation. Easiest way to +handle it is to put pciutils, libgetopt and flashrom directories +in one subdirectory. There will be an extra subdirectory libpci-libgetopt +created, which will contain compiled libpci and libgetopt. + +Download pciutils 3.5.6 and apply https://flashrom.org/File:Pciutils-3.5.6.patch.gz +Compile pciutils, using following command line:: + + make ZLIB=no DNS=no HOST=i386-djgpp-djgpp CROSS_COMPILE=i586-pc-msdosdjgpp- \ + PREFIX=/ DESTDIR=$PWD/../libpci-libgetopt \ + STRIP="--strip-program=i586-pc-msdosdjgpp-strip -s" install install-lib + +Download and compile with 'make' https://flashrom.org/File:Libgetopt.tar.gz + +Copy the libgetopt.a to ../libpci-libgetopt/lib and +getopt.h to ../libpci-libgetopt/include + +Enter the flashrom directory:: + + make CC=i586-pc-msdosdjgpp-gcc STRIP=i586-pc-msdosdjgpp-strip LIBS_BASE=../libpci-libgetopt/ strip + +If you like, you can compress the resulting executable with UPX:: + + upx -9 flashrom.exe + +To run flashrom.exe, download https://flashrom.org/File:Csdpmi7b.zip and +unpack CWSDPMI.EXE into the current directory or one in PATH. + +**To cross-compile on Linux for Windows:** + +Get packages of the MinGW cross compiler and install them:: + + mingw32-filesystem mingw32-cross-cpp mingw32-cross-binutils mingw32-cross-gcc + mingw32-runtime mingw32-headers + +The cross toolchain packages for your distribution may have slightly different +names (look for packages named *mingw*). +PCI-based programmers (internal etc.) are not supported on Windows. +Run (change CC= and STRIP= settings where appropriate):: + + make CC=i686-w64-mingw32-gcc STRIP=i686-w64-mingw32-strip + +**Processor architecture dependent features:** + +On non-x86 architectures a few programmers don't work (yet) because they +use port-based I/O which is not directly available on non-x86. Those +programmers will be disabled automatically if you run "make". + +**Compiler quirks:** + +If you are using clang and if you want to enable only one driver, you may hit an +overzealous compiler warning from clang. Compile with "make WARNERROR=no" to +force it to continue and enjoy. + +**Bindings:** + +Foreign function interface bindings for the rust language are included in the +bindings folder. These are not compiled as part of the normal build process. +See the readme under bindings/rust for more information. + + +Installation +------------ + +In order to install flashrom and the manpage into /usr/local, type:: + + make install + +For installation in a different directory use DESTDIR, e.g. like this:: + + make DESTDIR=/usr install + +If you have insufficient permissions for the destination directory, use sudo +by adding sudo in front of the commands above. + + +Packaging +--------- + +To package flashrom and remove dependencies on Git, either use:: + + make export + +or:: + + make tarball + +``make export`` will export all flashrom files from the Git repository at +revision HEAD into a directory named ``$EXPORTDIR/flashrom-$RELEASENAME`` +and will additionally add a ``versioninfo.inc`` file in that directory to +contain the Git revision of the exported tree and a date for the manual +page. + +``make tarball`` will simply tar up the result of make export and compress +it with bzip2. + +The snapshot tarballs are the result of ``make tarball`` and require no +further processing. Some git files (for example the rust bindings) are omitted +from the tarball, as controlled by the .gitattributes files. diff --git a/doc/dev_guide/development_guide.rst b/doc/dev_guide/development_guide.rst new file mode 100644 index 000000000..40837a943 --- /dev/null +++ b/doc/dev_guide/development_guide.rst @@ -0,0 +1,342 @@ +================= +Development Guide +================= + +We welcome contributions from every human being, corporate entity or club. + +This document describes the rules and recommendations about the development, contribution and review processes. + +If you introduce new features (not flash chips, but stuff like partial +programming, support for new external programmers, voltage handling, etc) +please **discuss your plans** on the :ref:`mailing list` first. That way, we +can avoid duplicated work and know about how flashrom internals need to be +adjusted and you avoid frustration if there is some disagreement about the +design. + +You can `look at the latest flashrom development efforts in Gerrit <https://review.coreboot.org/q/project:flashrom>`_. + +Set up the git repository and dev environment +============================================= + +#. Clone git repository + + * If using https: :code:`git clone "https://review.coreboot.org/flashrom"` + * If using ssh: :code:`git clone "ssh://<gerrit_username>@review.coreboot.org:29418/flashrom"` + +#. Follow the build guidelines to install dependencies :doc:`building_from_source` + +#. Install Git hooks: :code:`./util/git-hooks/install.sh` + +#. Add upstream as a remote: + + * If using https: :code:`git remote add -f upstream https://review.coreboot.org/flashrom` + * If using ssh: :code:`git remote add -f upstream ssh://<gerrit_username>@review.coreboot.org:29418/flashrom` + +#. Check out a new local branch that tracks :code:`upstream/master`: :code:`git checkout -b <branch_name> upstream/master` + +#. Every patch is required to be signed-off (see also :ref:`sign-off`). + Set up your ``user.name`` and ``user.email`` in git config, and don't forget + to ``-s`` when creating a commit. + +#. See also build guidelines :doc:`building_from_source` and `git docs <https://git-scm.com/doc>`_ + +Creating your patch +=================== + +In short, commit your changes with a descriptive message and remember to sign off +on the commit (``git commit -s``). + +.. _commit-message: + +Commit message +-------------- + +Commit messages shall have the following format: + +.. code-block:: + + <component>: Short description (up to 72 characters) + + This is a long description. Max width of each line in the description + is 72 characters. It is separated from the summary by a blank line. You + may skip the long description if the short description is sufficient, + for example "flashchips: Add FOO25Q128" to add FOO25Q128 chip support. + + You may have multiple paragraphs in the long description, but please + do not write a novel here. For non-trivial changes you must explain + what your patch does, why, and how it was tested. + + Finally, follow the sign-off procedure to add your sign-off! + + Signed-off-by: Your Name <your@domain> + +Commit message should include: + +* Commit title +* Commit description: explain what the patch is doing, or what it is fixing. +* Testing information: how did you test the patch. +* Signed-off-by line (see below :ref:`sign-off`) +* If applicable, link to the ticket in the bugtracker `<https://ticket.coreboot.org/projects/flashrom>`_ +* Change-Id for Gerrit. If commit message doesn't have Change-Id, you forgot to install git hooks. + +.. _sign-off: + +Sign-off procedure +^^^^^^^^^^^^^^^^^^ + +We employ a similar sign-off procedure as the `Linux kernel developers +<http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html>`_ +do. Add a note such as + +:code:`Signed-off-by: Random J Developer <random@developer.example.org>` + +to your email/patch if you agree with the Developer's Certificate of Origin 1.1 +printed below. Read `this post on the LKML +<https://lkml.org/lkml/2004/5/23/10>`_ for rationale (spoiler: SCO). + +You must use your known identity in the ``Signed-off-by`` line and in any +copyright notices you add. Anonymous patches lack provenance and cannot be +committed! + +Developer's Certificate of Origin 1.1 +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + By making a contribution to this project, I certify that: + + (a) The contribution was created in whole or in part by me and I have + the right to submit it under the open source license indicated in the file; or + + (b) The contribution is based upon previous work that, to the best of my + knowledge, is covered under an appropriate open source license and I have the + right under that license to submit that work with modifications, whether created + in whole or in part by me, under the same open source license (unless I am + permitted to submit under a different license), as indicated in the file; or + + (c) The contribution was provided directly to me by some other person who + certified (a), (b) or (c) and I have not modified it; and + + (d) In the case of each of (a), (b), or (c), I understand and agree that + this project and the contribution are public and that a record of the contribution + (including all personal information I submit with it, including my sign-off) is + maintained indefinitely and may be redistributed consistent with this project or the + open source license indicated in the file. + +.. note:: + + The `Developer's Certificate of Origin 1.1 + <http://web.archive.org/web/20070306195036/http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html>`_ + is licensed under the terms of the `Creative Commons Attribution-ShareAlike + 2.5 License <http://creativecommons.org/licenses/by-sa/2.5/>`_. + +Coding style +------------ + +Flashrom generally follows Linux kernel style: +https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst + +The notable exception is line length limit. Our guidelines are: + +* 80-columns soft limit for most code and comments. This is to encourage simple design and concise naming. +* 112-columns hard limit. Use this to reduce line breaks in cases where they + harm grep-ability or overall readability, such as print statements and + function signatures. Don't abuse this for long variable/function names or + deep nesting. +* Tables are the only exception to the hard limit and may be as long as needed + for practical purposes. + +Our guidelines borrow heavily from `coreboot coding style +<https://doc.coreboot.org/contributing/coding_style.html>`_ and `coreboot Gerrit +guidelines <https://doc.coreboot.org/contributing/gerrit_guidelines.html>`_, +and most of them apply to flashrom as well. The really important part is about +the :ref:`sign-off procedure <sign-off>`. + +We try to **reuse as much code as possible** and create new files only if +absolutely needed, so if you find a function somewhere in the tree which +already does what you want, please use it. + +Testing a patch +--------------- + +We expect the patch to be appropriately tested by the patch owner. +Please add the testing information in commit message, for example that could be some of these: +programmer you were using, programmer params, chip, OS, operations you were running +(read/write/erase/verify), and anything else that is relevant. + +.. _working-with-gerrit: + +Working with Gerrit +=================== + +All of the patches and code reviews need to go via +`Gerrit on review.coreboot.org <https://review.coreboot.org/#/q/project:flashrom>`_. +While it is technically possible to send a patch to the mailing list, that patch +still needs to be pushed to Gerrit by someone. We treat patches on the mailing list as a very +exceptional situation. Normal process is to push a patch to Gerrit. +Please read below for instructions and check `official Gerrit documentation <https://gerrit-review.googlesource.com/Documentation/>`_. + +Creating an account +--------------------- + +#. Go to https://review.coreboot.org/login and sign in using the credentials of + your choice. +#. Edit your settings by clicking on the gear icon in the upper right corner. +#. Set your Gerrit username (this may be the different from the username of an + external account you log in with). +#. Add an e-mail address so that Gerrit can send notifications to you about + your patch. +#. Upload an SSH public key, or click the button to generate an HTTPS password. + +.. _pushing-a-patch: + +Pushing a patch +--------------- + +To push patch to Gerrit, use the follow command: :code:`git push upstream HEAD:refs/for/master`. + +* If using HTTPS you will be prompted for the username and password you + set in the Gerrit UI. +* If successful, the Gerrit URL for your patch will be shown in the output. + +There is an option to add a topic to the patch. For one-off standalone patches this +is not necessary. However if your patch is a part of a larger effort, especially if the +work involves multiple contributors, it can be useful to mark that the patch belongs +to a certain topic. + +Adding a topic makes it easy to search "all the patches by the topic", even if the patches +have been authored by multiple people. + +To add a topic, push with the command: :code:`git push upstream HEAD:refs/for/master%topic=example_topic`. +Alternatively, you can add a topic from a Gerrit UI after the patch in pushed +(on the top-left section) of patch UI. + +Adding reviewers to the patch +----------------------------- + +After pushing the patch, ideally try to make sure there are some reviewers added to your patch. + +flashrom has MAINTAINERS file with people registered for some areas of the code. People who +are in MAINTAINERS file will be automatically added as reviewers if the patch touches that +area. However, not all areas are covered in the file, and it is possible that for the patch you +sent no one is added automatically. + +If you know someone in the dev community who can help with patch review, add the person(s) you know. + +In general, it's a good idea to add someone who has a knowledge of whatever the patch is doing, +even if the person has not been added automatically. + +If you are new, and don't know anyone, and no one has been added automatically: you can add +Anastasia Klimchuk (aklm) as a reviewer. + +Going through code reviews +-------------------------- + +You will likely get some comments on your patch, and you will need to fix the comments. +After doing the work locally, amend your commit ``git commit --amend -s`` and push to Gerrit again. +Check that Change-Id in commit message stays the same. This way Gerrit knows your change belongs +to the same patch, and will upload new change as new patchset for the same patch. + +After uploading the work, go through comments and respond to them. Mark as Done the ones you done +and mark them as resolved. If there is something that is impossible to do, or maybe you have more questions, +or maybe you are not sure what you are asked about: respond to a comment **without marking it as resolved**. + +It is completely fine to ask a clarifying questions if you don't understand what the comment is asking you to do. +If is also fine to explain why a comment can't be done, if you think it can't be done. + +The patch reviews may take some time, but please don't get discouraged. +We have quite high standards regarding code quality. + +Initial review should include a broad indication of acceptance or rejection of +the idea/rationale/motivation or the implementation + +In general, reviews should focus on the architectural changes and things that +affect flashrom as a whole. This includes (but is by no means limited to) +changes in APIs and types, safety, portability, extensibility, and +maintainability. The purpose of reviews is not to create perfect patches, but +to steer development in the right direction and produce consensus within the +community. The goal of each patch should be to improve the state of the project +- it does not need to fix all problems of the respective field perfectly. + + New contributors may need more detailed advices and should be told about + minor issues like formatting problems more precisely. The result of a review + should either be an accepted patch or a guideline how the existing code + should be changed to be eventually accepted. + +To get an idea whether the patch is ready or not, please check :ref:`merge-checklist`. + +If you sent a patch and later lost interest or no longer have time to follow up on code review, +please add a comment saying so. Then, if any of our maintainers are interested in finishing the work, +they can take over the patch. + +Downloading patch from Gerrit +----------------------------- + +Sometimes you may need to download a patch into your local repository. This can be needed for example: + +* if you want to test someone else's patch, +* if multiple developers are collaborating on a patch, +* if you are continuing someone else's work, when original author left or unable to continue. + +First prepare local repository: sync to head or to desired tag / commit. + +Open patch in Gerrit, open "three dot" menu on top-right, open Download patch. Copy Cherry-pick command (pick +the relevant tab for you: anonymous http / http / ssh) and run the copied command in your local repo. + +Now you have the commit locally and can do the testing or futher developing. To upload your local changes, +push patch to Gerrit again (see :ref:`pushing-a-patch`). + +Make sure people involved in the patch agree that you are pushing new version of someone else's patch, +so this does not come at a surprise for an original author. + +Merging patches +--------------- + +Merging to branches is limited to the "flashrom developers" group on Gerrit (see also :doc:`/about_flashrom/team`). + +The list of requirements for the patch to be ready for merging is below, see :ref:`merge-checklist`. +Some of the requirements are enforced by Gerrit, but not all of them. In general, a person who clicks +Submit button is responsible to go through Merge checklist. Code reviewers should be aware of the checklist +as well. + +Patch owners can use the checklist to detect whether the patch is ready for merging or not. + +.. _merge-checklist: + +Merge checklist +^^^^^^^^^^^^^^^ + +#. Every patch has to be reviewed and needs at least one +2 that was not given by the commit's author. + Ideally, people who were actively reviewing the patch and adding comments, would be the ones approving it. +#. If a patch is authored by more than one person (Co-developed-by), each author may +2 the other author's changes. +#. Patch needs to get Verified +1 vote, typically from Jenkins build bot. This means the patch builds successfully + and all unit tests pass. +#. Commit message should have Signed-off-by line, see :ref:`sign-off` and align with the rest + of the rules for :ref:`commit-message` +#. All the comments need to be addressed, especially if there was a negative vote in the process of review (-1 or -2). +#. flashrom developers are people from literally all around the planet, and various timezones. We usually wait + for 3 days (3 * 24hours) after the patch is fully approved just in case of last minute concerns from all timezones. +#. In the case of emergency, merging should not take place within less than 24 hours after the review + started (i.e. the first message by a reviewer on Gerrit). + +To help search for patches which are potential candidates for merging, you can try using this search in Gerrit:: + + status:open project:flashrom -is:wip -label:Verified-1 label:Verified+1 -label:Code-Review<0 age:3d is:mergeable is:submittable -has:unresolved + +Note the search is not a replacement for Merge checklist, but it can help find candidates for merging. + +Bugtracker +========== + +We have a bugtracker on `<https://ticket.coreboot.org/projects/flashrom>`_. +Anyone can view tickets, but to be able to create/update/assign tickets you need an account. + +Mirrors +======== + +The only official repository is https://review.coreboot.org/flashrom ; GitHub and GitLab are just mirrors. +**Reviewers do not look at pull requests** on mirrors. +Even if pull requests were automatically transferred to Gerrit, +requirements such as :ref:`sign-off` still present a problem. + +The quickest and best way to get your patch reviewed and merged is by sending +it to review.coreboot.org (see :ref:`working-with-Gerrit`). Conveniently, you can use your GitHub, GitLab or +Google account as an OAuth2 `login method <https://review.coreboot.org/login>`_. diff --git a/doc/dev_guide/index.rst b/doc/dev_guide/index.rst new file mode 100644 index 000000000..57cd8468c --- /dev/null +++ b/doc/dev_guide/index.rst @@ -0,0 +1,9 @@ +Developers documentation +======================== + +.. toctree:: + :maxdepth: 1 + + building_from_source + building_with_make + development_guide |