--- /dev/null
+Source tree checker scripts {#page_dev_gmxtree}
+===========================
+
+There is a set of Python scripts, currently under `docs/doxygen/`, that check
+various aspects of the source tree for consistency. The script is based on
+producing an abstract representation of the source tree from various sources:
+ * List of files in the source tree (for overall layout of the source tree)
+ * List of installed headers (extracted from the generated build system)
+ * git attributes (to limit the scope of some checks)
+ * Doxygen XML documentation:
+ * For tags about public/private nature of documented headers and other
+ constructs
+ * For actual documented constructs, to check them for consistency
+ * Hard-coded knowledge about the \Gromacs source tree layout
+
+This representation is then used for various purposes:
+ * Checking Doxygen documentation elements for common mistakes: missing brief
+ descriptions, mismatches in file and class visibility, etc.
+ * Checking for consistent usage and documentation of headers: e.g., a header
+ that is documented as internal to a module should not be used outside that
+ module.
+ * Checking for module-level cyclic dependencies
+ * Checking for consistent style and order of \#include directives
+ (see \ref page_devstyle_includes)
+ * Actually sorting and reformatting \#include directives to adhere to the
+ checked style
+ * Generating dependency graphs between modules and for files within modules
+
+The checks are run as part of a single `doc-check` target, but are described
+in separate sections below. In addition to printing the issues to `stderr`,
+the script also writes them into `docs/doxygen/doxygen-check.log` for later
+inspection. Jenkins runs the checks as part of the Documentation job, and the
+build is marked unstable if any issues are found.
+
+For correct functionality, the scripts depend on correct usage of Doxygen
+annotations described in \ref page_doxygen, in particular the visibility and
+API definitions in file-level comments.
+
+For some false positives from the script, the suppression mechanism described
+below is the easiest way to silence the script, but otherwise the goal would be
+to minimize the number of suppressions.
+
+The scripts require Python 2.7 (other versions may work, but have not been
+tested).
+
+To understand how the scripts work internally, see comments in the Python
+source files under `docs/doxygen/`.
+
+Checker details
+===============
+
+The `doc-check` target currently checks for a few different types of issues.
+These are listed in detail below, mainly related to documentation and include
+dependencies. Note in particular that the include dependency checks are much
+stricter for code in modules/directories that are documented with a
+\c \\defgroup: all undocumented code is assumed to be internal to such modules.
+The rationale is that such code has gotten some more attention, and some effort
+should also have been put into defining what is the external interface of the
+module and documenting it.
+
+* For all Doxygen documentation (currently does not apply for members that do
+ not appear in the documentation):
+ * If a member has documentation, it should have a brief description.
+ * A note is issued for in-body documentation for functions, since this is
+ ignored by our current settings.
+ * If a class has documentation, it should have public documentation only if
+ it appears in an installed header.
+ * If a class and its containing file has documentation, the class
+ documentation should not be visible if the file documentation is not.
+* For all files:
+ * Consistent usage of
+
+ #include "..." // This should be used for Gromacs headers
+
+ and
+
+ #include <...> // This should be used for system and external headers
+
+ * Installed headers must not include non-installed headers.
+ * All source files must include "gmxpre.h" as the first header.
+ * A source/header file should include "config.h" if and only if it uses a
+ macro declared there.
+ * If the file has a git attribute to identify it as a candidate for include
+ sorting, the include sorter described below should not produce any
+ changes (i.e., the file should follow \ref page_devstyle_includes).
+* For documented files:
+ * Installed headers should have public documentation, and other files should
+ not.
+ * The API level specified for a file should not be higher than where its
+ documentation is visible. For example, only publicly documented headers
+ should be specified as part of the public API.
+ * If an \c \\ingroup `module_foo` exists, it should match the subdirectory
+ that the file is actually part of in the file system.
+ * If a \c \\defgroup `module_foo` exists for the subdirectory where the file
+ is, the file should contain \c \\ingroup `module_foo`.
+ * Files should not include other files whose documentation visibility is
+ lower (if the included file is not documented, the check is skipped).
+* For files that are part of documented modules
+ (\c \\defgroup `module_foo` exists for the subdirectory), or are explicitly
+ documented to be internal or in the library API:
+ * Such files should not be included from outside their module if they are
+ undocumented (for documented modules) or are not specified as part of
+ library or public API.
+* For all modules:
+ * There should not be cyclic include dependencies between modules.
+
+As a side effect, the XML extraction makes Doxygen parse all comments in the
+code, even if they do not appear in the documentation. This can reveal latent
+issues in the comments, like invalid Doxygen syntax. The messages from the XML
+parsing are stored in `docs/doxygen/doxygen-xml.log` in the build tree, similar to
+other Doxygen runs.
+
+Suppressing issues
+------------------
+
+The script is not currently perfect (either because of unfinished
+implementation, or because Doxygen bugs or incompleteness of the Doxygen XML
+output), and the current code also contains issues that the script detects, but
+the authors have not fixed. To allow the script to still be used,
+`doxygen/suppressions.txt` contains a list of issues that are filtered out from
+the report. The syntax is simple:
+
+ <file>: <text>
+
+where `<file>` is a path to the file that reports the message, and `<text>` is
+the text reported. Both support `*` as a wildcard. If `<file>` is empty, the
+suppression matches only messages that do not have an associated file.
+`<file>` is matched against the trailing portion of the file name to make it
+work even though the script reports absolute paths.
+Empty lines and lines starting with `#` are ignored.
+
+To add a suppression for an issue, the line that reports the issue can be
+copied into `suppressions.txt`, and the line number (if any) removed. If the
+issue does not have a file name (or a pseudo-file) associated, a leading `:`
+must be added. To cover many similar issues, parts of the line can then be
+replaced with wildcards.
+
+A separate suppression mechanism is in place for cyclic dependencies: to
+suppress a cycle between moduleA and moduleB, add a line with format
+
+ moduleA -> moduleB
+
+into `doxygen/cycle-suppressions.txt`. This suppresses all cycles that contain
+the mentioned edge. Since a cycle contains multiple edges, the suppression
+should be made for the edge that is determined to be an incorrect dependency.
+This also affects the layout of the include dependency graphs (see below): the
+suppressed edge is not considered when determining the dependency order, and is
+shown as invalid in the graph.
+
+Include order sorting {#section_dev_includesorter}
+=====================
+
+The script checks include ordering according to \ref page_devstyle_includes.
+If it is not obvious how the includes should be changed to make the script
+happy, or bulk changes are needed in multiple files, e.g., because of a header
+rename or making a previously public header private, it is possible to run a
+Python script that does the sorting:
+
+ docs/doxygen/includesorter.py -S . -B ../build <files>
+
+The script needs to know the location of the source tree (given with `-S`) and
+the build tree (given with `-B`), and sorts the given files. To sort the whole
+source tree, one can also use:
+
+ admin/reformat_all.sh includesort -B=../build
+
+For the sorter to work correctly, the build tree should contain up-to-date list
+of installed files and Doxygen XML documentation. The former is created
+automatically when `cmake` is run, and the latter can be built using the
+`doc-xml` target.
+
+Note that currently, the sorter script does not change between angle brackets
+and quotes in include statements.
+
+Include dependency graphs
+=========================
+
+The same set of Python scripts can also produce include dependency graphs with
+some additional annotations compared to what, e.g., Doxygen produces for a
+directory dependency graph. Currently, a module-level graph is automatically
+built when the Doxygen documentation is built and embedded in the documentation
+(not in the public API documentation). The graph, together with a legend, is
+on a separate page: \ref page_modulegraph.
+
+The Python script produces the graphs in a format suitable for `dot` (from the
+`graphviz` package) to lay them out. The build system also provides a
+`dep-graphs` target that generates PNG files from the intermediate `dot` files.
+In addition to the module-level graph, a file-level graph is produced for
+each module, showing the include dependencies within that module.
+The file-level graphs can only be viewed as the PNG files, with some
+explanation of the notation below. Currently, these are mostly for eye candy,
+but they can also be used for analyzing problematic dependencies to clean up
+the architecture.
+
+Both the intermediate `.dot` files and the final PNG files are put under
+`docs/doxygen/depgraphs/` in the build tree.
+
+File graphs
+-----------
+
+The graphs are written to <em>module_name</em>`-deps.dot.png`.
+
+Node colors:
+<dl>
+<dt>light blue</dt>
+<dd>public API (installed) headers</dd>
+<dt>dark blue</dt>
+<dd>library API headers</dd>
+<dt>gray</dt>
+<dd>source files</dd>
+<dt>light green</dt>
+<dd>test files</dd>
+<dt>white</dt>
+<dd>other files</dd>
+</dl>
+
+Each edge signifies an include dependency; there is no additional information
+currently included.
--- /dev/null
+Guidelines for \#include directives {#page_devstyle_includes}
+===================================
+
+The following include order is used in \Gromacs. An empty line should appear
+between each group, and headers within each group sorted alphabetically.
+
+ 1. Each _source_ file should include `gmxpre.h` first.
+ 2. If a _source_ file has a corresponding header, it should be included next.
+ If the header is in the same directory as the source, then it is included
+ without any path (i.e., relative to the source), otherwise relative to
+ `src/`. The latter case is for headers in `legacyheaders/` and for tests.
+ 3. If the file depends on defines from `config.h`, that comes next.
+ 4. This is followed by standard C/C++ headers, grouped as follows:
+
+ 1. Standard C headers (e.g., `<stdio.h>`)
+ 2. C++ versions of the above (e.g., `<cstdio>`)
+ 3. Standard C++ headers (e.g., `<vector>`)
+
+ Preferably, only one of the first two groups is present, but this is not
+ enforced.
+ 5. This is followed by other system headers: platform-specific headers such as
+ `<unistd.h>`, as well as external libraries such as
+ `<boost/scoped_ptr.hpp>`.
+ 6. \Gromacs-specific libraries from `src/external/`, such as
+ `"thread_mpi/threads.h"`.
+ 7. \Gromacs-specific headers that are not internal to the including module,
+ included with a path relative to `src/`.
+ 8. In _test_ files, headers not internal to the module, but specific to
+ testing code, are in a separate block at this point, paths relative to
+ `src/`.
+ 9. Finally, \Gromacs headers that are internal to the including module are
+ included using a relative path (but never with a path starting with `../`;
+ such headers go into group 7 instead). For test files, this group contains
+ headers that are internal to tests for that module.
+
+All \Gromacs headers are included with quotes (`"gromacs/utility/path.h"`),
+other headers with angle brackets (`<stdio.h>`). Headers under `src/external/`
+are generally included with quotes (whenever the include path is relative to
+`src/`, as well as for thread-MPI and TNG), but larger third-party entities are
+included as if they were provided by the system. The latter group currently
+includes boost and gtest/gmock.
+
+If there are any conditionally included headers (typically, only when some
+\#defines from `config.h` are set), these should be included at the end of
+their respective group. Note that the automatic checker/sorter script does not
+act on such headers, nor on comments that are between \#include statements; it
+is up to the author of the code to put the headers in proper order in such
+cases. Trailing comments on the same line as \#include statements are
+preserved and do not affect the checker/sorter.
+
+Enforcing a consistent order and style has a few advantages:
+ * It makes it easy at a quick glance to find the dependencies of a file,
+ without scanning through a long list of unorganized \#includes.
+ * Including the header corresponding to the source file first makes most
+ headers included first in some source file, revealing potential problems
+ where headers would not compile unless some other header would be included
+ first. With this order, the person working on the header is most likely to
+ see these problems instead of someone else seeing them later when
+ refactoring unrelated code.
+ * Consistent usage of paths in \#include directives makes it easy to use
+ `grep` to find all uses of a header, as well as all include dependencies
+ between two modules.
+ * An automatic script can be used to re-establish clean code after
+ semi-automatic refactoring like renaming an include file with `sed`, without
+ causing other unnecessary changes.
+
+The guidelines are enforced by an automatic checker script that can also
+sort/reformat include statements to follow the guidelines.
+See \ref page_dev_gmxtree for details.
Style guidelines {#page_devstyle}
================
-TODO: Add guidelines
+ - \subpage page_devstyle_includes <br/>
+ Guidelines for \#include style (ordering, paths to use, etc.).
+
+ - [Doxygen guidelines](\ref section_doxygen_guidelines) <br/>
+ Guidelines for using Doxygen to document the source code are currently in a
+ section on the page on general Doxygen usage.
+
+TODO: Add more guidelines
`uncrustify.sh` automatically before every commit to check for formatting
issues.</dd>
-<dt>include directive checker</dt>
-<dd></dd>
+<dt>`docs/doxygen/includesorter.py`</dt>
+<dd>This Python script sorts and reformats \#include directives according to
+the guidelines at \ref page_devstyle_includes. Details are documented on a
+separate page (with the whole suite of Python scripts used for source code
+checks): \ref section_dev_includesorter.</dd>
-<dt>include sorting</dt>
-<dd></dd>
+<dt>include directive checker</dt>
+<dd>In its present form, the above include sorter script cannot be conveniently
+applied in `uncrustify.sh`. To check for issues, it is instead integrated into
+a `doc-check` build target. When this target is built, it also checks for
+include formatting issues. Internally, it uses the sorter script. This check
+is run in Jenkins as part of the Documentation job.
+Details for the checking mechanism are on a separate page (common for several
+checkers): \subpage page_dev_gmxtree.</dd>
<dt>`admin/reformat_all.sh`</dt>
<dd>This `bash` script runs uncrustify/`copyright.py`/include sorter
<dd>Doxygen produces warnings about some incorrect uses and wrong
documentation, but there are many common mistakes that it does not detect.
\Gromacs uses an additional, custom Python script to check for such issues.
+This is most easily invoked through a `doc-check` target in the build system.
The script also checks that documentation for a header matches its use in the
source code (e.g., that a header documented as internal to a module is not
-actually used from outside the module).</dd>
+actually used from outside the module). These checks are run in Jenkins as
+part of the Documentation job. Details for the custom checker are on a
+separate page (common for several checkers): \subpage page_dev_gmxtree.</dd>
+
+<dt>module dependency graphs</dt>
+<dd>\Gromacs uses a custom Python script to generate an annotated dependency
+graph for the code, showing \#include dependencies between modules.
+The generated graph is embedded into the Doxygen documentation:
+\ref page_modulegraph.
+This script shares most of its implementation with the custom checkers, and is
+documented on the same page: \subpage page_dev_gmxtree.</dd>
<dt>Pandoc</dt>
<dd>Pandoc (<http://johnmacfarlane.net/pandoc/>) is used for building some
Documentation flavors
=====================
+
The \Gromacs source tree is set up to produce three different levels of Doxygen
documentation:
figures will be missing from the documentation.
-General guidelines for Doxygen markup
+General guidelines for Doxygen markup {#section_doxygen_guidelines}
=====================================
Doxygen provides quite a few different alternative styles for documenting the
documentation, as well as how \Gromacs uses Doxygen groups to organize the
documentation.
+Some consistency checks are done automatically using custom scripts.
+See \ref page_dev_gmxtree for details.
+
Controlling documentation visibility
------------------------------------
separate page in the generated documentation can only belong to one group;
in such a case, the module group is preferred over the API group.
-Automatic dependency checking
------------------------------
-
-The build system provides a `doc-check` target that automatically checks some
-aspects of the documentation, as well as checking that header files are
-actually used according to their API specifications (i.e., an internal headers
-are not included from other modules). The checks depend on correct usage of
-the commands listed above, in particular the visibility and API definitions in
-file-level comments. These checks also provide some level of enforcement for
-rules about dependencies between the modules, but currently the checks are not
-run automatically.
-
-The checker currently checks for a few different types of issues:
-* For all Doxygen documentation (currently does not apply for members that do
- not appear in the documentation):
- * If a member has documentation, it should have a brief description.
- * A note is issued for in-body documentation for functions, since this is
- ignored by our current settings.
- * If a class has documentation, it should have public documentation only if
- it appears in an installed header.
- * If a class and its containing file has documentation, the class
- documentation should not be visible if the file documentation is not.
-* For all files:
- * Consistent usage of
-
- #include "..." // This should be used for Gromacs headers
-
- and
-
- #include <...> // This should be used for system and external headers
-
- * Installed headers must not include non-installed headers, and must include
- all other \Gromacs headers using relative paths so that they resolve
- correctly also when installed.
-* For documented files:
- * Installed headers should have public documentation, and other files should
- not.
- * The API level specified for a file should not be higher than where its
- documentation is visible. For example, only publicly documented headers
- should be specified as part of the public API.
- * If an \c \\ingroup module_foo exists, it should match the subdirectory
- that the file is actually part of in the file system.
- * If a \c \\defgroup module_foo exists for the subdirectory where the file is,
- the file should contain \c \\ingroup module_foo.
- * Files should not include other files whose documentation visibility is
- lower (if the included file is not documented, the check is skipped).
-* For files that are part of documented modules
- (\c \\defgroup module_foo exists for the subdirectory):
- * Such files should not be included from outside their module if they are
- undocumented or are not specified as part of library or public API.
-* For all modules:
- * There should not be cyclic include dependencies between modules.
-
-The checker is based on extracting the Doxygen documentation in XML format.
-This information is then read using a Python script, and combined with
-information extracted from the file system and knowledge about the \Gromacs
-source tree layout. The Python scripts are in the `doxygen/` folder.
-In addition to printing the issues, they are also written into
-`doxygen/doxygen-check.log` for later inspection.
-
-The script is not currently perfect (either because of unfinished
-implementation, or because Doxygen bugs or incompleteness of the Doxygen XML
-output), and the current code also contains issues that the script detects, but
-the authors have not fixed. To allow the script to still be used,
-`doxygen/suppressions.txt` contains a list of issues that are filtered out from
-the report. The syntax is simple:
-
- <file>: <text>
-
-where `<file>` is a path to the file that reports the message, and `<text>` is
-the text reported. Both support `*` as a wildcard. If `<file>` is empty, the
-suppression matches only messages that do not have an associated file.
-`<file>` is matched against the trailing portion of the file name to make it
-work even though the script reports absolute paths.
-Empty lines and lines starting with `#` are ignored.
-
-To add suppression for an issue, the line that reports the issue can be copied
-into `suppressions.txt`, and the line number (if any) removed. If the
-issue does not have a file name (or a pseudo-file) associated, a leading `:`
-must be added. To cover many similar issues, parts of the line can then be
-replaced with wildcards.
-
-A separate suppression mechanism is in place for cyclic dependencies: to
-suppress a cycle between moduleA and moduleB, add a line with format
-
- moduleA -> moduleB
-
-into `doxygen/cycle-suppressions.txt`. This suppresses all cycles that contain
-the mentioned edge. Since a cycle contains multiple edges, the suppression
-should be made for the edge that is determined to be an incorrect dependency.
-This also affects the layout of the include dependency graphs (see below): the
-suppressed edge is not considered when determining the dependency order, and is
-shown as invalid in the graph.
-
-For some false positives from the script, the suppression mechanism is the
-easiest way to silence the script, but otherwise the goal would be to minimize
-the number of suppressions.
-
-As a side effect, the XML extraction makes Doxygen parse all comments in the
-code, even if they do not appear in the documentation. This can reveal latent
-issues in the comments, like invalid Doxygen syntax. The messages from the XML
-parsing are stored in `docs/doxygen/doxygen-xml.log` in the build tree, similar to
-other Doxygen runs.
-
-The `doc-check` target requires Python 2.7 (other versions may work, but have
-not been tested).
-
-Include dependency graphs
--------------------------
-
-The build system also provides an `dep-graphs` target that generates include
-dependency graphs with some additional annotations.
-One graph is produced that shows all the modules under `src/gromacs/`, and
-their include dependencies. Additionally, a file-level graph is produced for
-each module, showing the include dependencies within that module. Currently,
-these are mostly for eye candy, but they can also be used for analyzing
-problematic dependencies to clean up the architecture.
-The output is put in `docs/doxygen/depgraphs/` in the build tree.
-
-As with `doc-check`, Python 2.7 is required (other versions may work, but have
-not been tested). To get `.png` versions of the graphs, `graphviz` is
-additionally required.
-
-### Module graph ###
-
-The graph is written into `module-deps.dot.png`, and embedded into the Doxygen
-documentation: \ref page_modulegraph. The embedded version contains a legend
-explaining the graph.
-
-### File graph ###
-
-The graphs are written to <em>module_name</em>`-deps.dot.png`.
-
-Node colors:
-<dl>
-<dt>light blue</dt>
-<dd>public API (installed) headers</dd>
-<dt>dark blue</dt>
-<dd>library API headers</dd>
-<dt>gray</dt>
-<dd>source files</dd>
-<dt>light green</dt>
-<dd>test files</dd>
-<dt>white</dt>
-<dd>other files</dd>
-</dl>
-
-Each edge signifies an include dependency; there is no additional information
-currently included.
Documenting specific code constructs
====================================
biod.pnpi.spb.ru / alexxy / gromacs.git / commitdiff