Skip to content

Commit

Permalink
Refactor documentation
Browse files Browse the repository at this point in the history
- Introduce and document a new style regarding wrapping:
  - Each sentence starts in a new line
  - No wrapping
  - Consistent style to enforce a newline
- Consistent style in examples using A, B, X and similar as placeholders
  for arbitrary entities
- Remove some outdated information
- Fix typos
- Streamline several sentences
  • Loading branch information
martis42 committed Sep 9, 2023
1 parent 946016a commit b133b3f
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 119 deletions.
20 changes: 15 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,26 @@

All contributions require a review from the [code owners](.github/CODEOWNERS).

## Code Style and Code Quality
## Code Quality

The project uses several formatters and linters. [poetry](https://github.com/python-poetry/poetry) and
[pre-commit-hooks](https://github.com/pre-commit/pre-commit-hooks) are used to manage and execute those.
The project uses several formatters and linters.
[poetry](https://github.com/python-poetry/poetry) and [pre-commit-hooks](https://github.com/pre-commit/pre-commit-hooks) are used to manage and execute those.

When contributing code, please make sure to execute the checks.

After you have installed `poetry` for your platform install the tools required by DWYU: `poetry install`.
Then, you can execute all relevant checks via `poetry run pre-commit run --all-files` or configure `pre-commit-hooks`
to run automatically for each commit.
Then, you can execute all relevant checks via `poetry run pre-commit run --all-files` or configure `pre-commit-hooks` to run automatically for each commit.

## Code Style

### Markdown

Most of the markdown stile is automatically enforced with [mdformat](https://github.com/executablebooks/mdformat).
Some part is however maintained manually:

- Each sentence starts in a new line.
- Sentences are not wrapped, no matter how long they are.
- `<br>` is used to enforce a newline.

# Bug reports

Expand Down
133 changes: 61 additions & 72 deletions README.md

Large diffs are not rendered by default.

33 changes: 16 additions & 17 deletions docs/concept_behind_dwyu.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,31 @@
Depend On What You Use (DWYU) has been inspired by [Include What You Use (IWYU)](https://github.com/include-what-you-use/include-what-you-use).
Essentially DWYU applies the idea behind IWYU to the relationship between C++ include statements and dependencies.

DWYUs design philosophy is<br/>
DWYUs design philosophy is<br>
**A C++ target shall list all dependencies from which it is including header files.
Header files from transitive dependencies shall not be used.**

For example, target A depends on `cc_library` B. B depends on `cc_library` C.
A is allowed to include headers from target B.
A is however not allowed to use headers from C, unless it specifies C as direct dependency.<br/>
For example, target _A_ depends on `cc_library` _B_.
_B_ depends on `cc_library` _C_.
_A_ is allowed to include headers from target _B_.
_A_ is however not allowed to use headers from _C_, unless it specifies _C_ as direct dependency.<br>
This rule is not enforced by the compiler.
A using headers from C would compile, since all public headers from all direct and transitive dependencies are present
in the Bazel sandbox and are part of the include path for compiling A.
_A_ using headers from _C_ would compile, since all public headers from all direct and transitive dependencies are present in the Bazel sandbox and are part of the include path for compiling _A_.

Adhering to this design principle has several advantages:

- One follows the Bazel dependency modeling best practices. Bazel is not yet generally enforcing its depndency rules,
but this can change at any time. For more details see:
- One follows the Bazel dependency modeling best practices.
Bazel is not yet generally enforcing its dependency rules, but this can change at any time.
For more details see:
- `cc_library` design [regarding include paths](https://bazel.build/reference/be/c-cpp#hdrs).
- Dependencies concept [documentation](https://bazel.build/concepts/dependencies#actual-and-declared-dependencies).
- Dependency management [documentation](https://bazel.build/basics/dependencies). Note that Bazel is already enforcing
limited access to transitive dependencies for Java code.
- Dependency management [documentation](https://bazel.build/basics/dependencies).
Note that Bazel is already enforcing limited access to transitive dependencies for Java code.
- Bazel performs efficient incremental builds by analyzing the dependency tree of the targets and doing only what is really required.
A dependency tree modeling the C++ source code as close as possible enables Bazel to work most efficiently.
- Depending on transitive dependencies can cause unexpected build failures.
Assume one of your dependencies X is dropping its transitive dependency Y.
This should not influence your target as long as Xs interface is not changing.
However, if you use headers from Y, your build will fail unexpectedly.
- While reading a BUILD file one sees at a glance all other targets directly influencing the content of the BUILD file
without having to read the source code.
- When analyzing the direct downstream dependencies of `cc_library` X (e.g. with bazel query) one can be sure that all
places where headers of X are used are discoverable through the Bazel dependency tree.
Assume one of your dependencies _X_ is dropping its transitive dependency _Y_.
This should not influence your target as long as the interface of _X_ is not changing.
However, if you use headers from _Y_, your build will fail unexpectedly.
- While reading a BUILD file one sees at a glance all other targets directly influencing the content of the BUILD file without having to read the source code.
- When analyzing the direct downstream dependencies of `cc_library` _X_ one can be sure that all places where headers of _X_ are used are discoverable through the Bazel dependency tree.
41 changes: 21 additions & 20 deletions docs/project_design_rationales.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,34 @@

## Why use Python

There are many programming languages available which could be used to implement DWYU. Why use Python because:
There are many programming languages available which could be used to implement DWYU.
Why use Python because:

- It is well established tool many developers are familiar with.
- Most platforms support Python well and developer setups often have an interpreter pre-installed.
- There is a wide range of well established third-party libraries.
- There is no need to deploy pre compiled binaries for a wide range of platforms.
- It is well suited for scripting tasks and testing.
- The task done by DWYU does not require many resources. Thus, efficiency and performance are secondary.
- The project maintainer is experienced with Python.
- The task done by DWYU does not require many resources.
Thus, efficiency and performance are secondary.

## Why use a multi step automatic fixes workflow

Having to execute a separate tool to apply fixes seems bothersome. Ideally, DWYU would perform fixes
while analyzing the problems.<br/>
Having to execute a separate tool to apply fixes seems bothersome.
Ideally, DWYU would perform fixes while analyzing the problems.<br>
However, given DWYU is implemented as a Bazel aspect, there are limitations to what we can do in a single step:

- The DWYU aspect is analyzing the dependencies of the targets. Changing the dependencies while analyzing them would
invalidate the dependency graph and require rebuilding the graph after each fix before continuing to
analyze more targets. There is no standard feature of Bazel aspects allowing this.
- A Bazel aspect is executed in the sandbox. To be able to modify the BUILD files in the workspace, one would have to
escape the sandbox. This is generally considered a bad practice when working with Bazel.

We circumvent the above problems by using a two step approach. First we discover all problems and store the result in
a machine readable format. Then, we use a separate tool to process the results and apply fixes to the BUILD files in
the workspace. There are no problems regarding the sandboxing, since we utilize `bazel run` to execute the fixing tool.
- The DWYU aspect is analyzing the dependencies of the targets.
Changing the dependencies while analyzing them would invalidate the dependency graph and require rebuilding the graph after each fix before continuing to analyze more targets.
There is no standard feature of Bazel aspects allowing this.
- A Bazel aspect is executed in the sandbox.
To be able to modify the BUILD files in the workspace, one would have to escape the sandbox.
This is generally considered a bad practice when working with Bazel.

We circumvent the above problems by using a two step approach.
First we discover all problems and store the result in a machine readable format.
Then, we use a separate tool to process the results and apply fixes to the BUILD files in the workspace.
There are no problems regarding the sandboxing, since we utilize `bazel run` to execute the fixing tool.
A tool being executed like this can access any part of the host system.

# Platforms
Expand All @@ -48,8 +51,7 @@ The DWYU aspect is incompatible to Bazel \< 4.0.0.
Bazel 4.x is not tested, but should still work.

We officially support and test only Bazel 5.0.0 and later.
One major feature of DWYU is ensuring the proper usage of `implementation_deps`, which is a feature which is only
available starting from Bazel 5.0.0.
One major feature of DWYU is ensuring the proper usage of `implementation_deps`, which is a feature which is only available starting from Bazel 5.0.0.
Furthermore, many Bazel rule sets already rely on Bazel 5.x as well.

## Why is Python \< 3.8 not supported
Expand All @@ -67,10 +69,9 @@ Essentially, this makes parsing of source code and aggregating the include state
One downside is, that this only works for source files, but not for header only code.
This could be mitigated by generating source files for the headers and then running the compiler on them.

A major drawback of this approach is however, that the `.d` files list all transitively included headers which are
required for compiling a source file.
A major drawback of this approach is however, that the `.d` files list all transitively included headers which are required for compiling a source file.

For example, given the 3 target `A`, `B` and `C` with the files:
For example, given the 3 targets _A_, _B_ and _C_ with the files:

`a.h`

Expand All @@ -95,7 +96,7 @@ void doC() { doB(); };
```
The `.d` file for `c.cpp` will list the headers `a.h` and `b.h`.
This makes sense, after all the compiler requires all used headers to compile `c.cpp`.
This makes sense, as the compiler requires all used headers to compile `c.cpp`.
However, it makes the `.d` file impractical for DWYU.
We need to know if header `a.h` was included directly in `c.cpp` or is used transitively by `b.h`.
Without this distinction we cannot compare the include statements to the list of direct dependencies.
Expand Down
2 changes: 1 addition & 1 deletion test/apply_fixes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ Each `test_*.py` file represents a test case. The `execute_test.py` is the entry

For each test case the workspace template is copied into a temporary directory in which then DWYU is executed to detect
a problem. Afterwards, the `//:apply_fixes` tool is executed and the adapted `BUILD` files are analyzed to check if the
expected change happened.<br/>
expected change happened.<br>
This approach based on a temporary workspace is chosen to make sure cleaning up the test side effects is not
messing with the source code.
4 changes: 2 additions & 2 deletions test/aspect/REAME.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ The tests are executed with various supported Bazel versions.

The test cases and their expected behavior are defined in [execute_tests.py](execute_tests.py).

Execute all tests with various Bazel versions: \
Execute all tests with various Bazel versions:<br>
`./execute_tests.py`

You can execute specific test cases or use specific Bazel versions. For details see the help: \
You can execute specific test cases or use specific Bazel versions. For details see the help:<br>
`./execute_tests.py --help`
3 changes: 1 addition & 2 deletions test/aspect/tree_artifact/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@ A `TreeArtifact` is a special output of rules where one does not know beforehand
Code generators are a common use case relying on this construct.
In other words, a `TreeArtifact` is a predeclared directory with unknown content.
The content will not be available until the execution phase.
Essentially, this makes it impossible to analyze a `TreeArtifact` in StarLark and requires doing so inside an action
which is performed in the execution phase.
Essentially, this makes it impossible to analyze a `TreeArtifact` in StarLark and requires doing so inside an action which is performed in the execution phase.

0 comments on commit b133b3f

Please sign in to comment.