46eaab3959
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Kevin Lubick
|
e253cc3e55 |
[bazel] Use toolchain features to opt files into being IWYU compliant.
PS1 regenerates the Bazel files.
It is recommended to review this CL with a diff from PS1.
Example output when a file does not pass the test:
tools/sk_app/CommandSet.h should add these lines:
#include "include/core/SkTypes.h"
#include "include/private/SkTArray.h"
#include "tools/skui/InputState.h"
#include "tools/skui/Key.h"
#include "tools/skui/ModifierKey.h"
namespace sk_app { class Window; }
tools/sk_app/CommandSet.h should remove these lines:
- #include "tools/sk_app/Window.h"
The full include-list for tools/sk_app/CommandSet.h:
#include "include/core/SkString.h"
#include "include/core/SkTypes.h"
#include "include/private/SkTArray.h"
#include "tools/skui/InputState.h"
#include "tools/skui/Key.h"
#include "tools/skui/ModifierKey.h"
#include <functional>
#include <vector>
class SkCanvas;
namespace sk_app { class Window; }
---
This makes use of Bazel's toolchain features
https://bazel.build/docs/cc-toolchain-config-reference#features
to allow us to configure compiler flags when compiling
individual files. This analysis is off by default, and can
be turned on with --features skia_enforce_iwyu. When enabled,
it will only be run for files that have opted in.
Example:
bazelisk build //example:hello_world_gl --config=clang \
--sandbox_base=/dev/shm --features skia_enforce_iwyu
There are two ways to opt files in:
- Add enforce_iwyu = True to a generated_cc_atom rule
- Add enforce_iwyu_on_package() to a BUILD.bazel file
(which enforces IWYU for all rules in that file)
Note that Bazel does not propagate features to dependencies
or dependents, so trying to enable the feature on cc_library
or cc_executable targets will only impact any files listed in
srcs or hdrs, not deps. This may be counter-intuitive when
compared to things like defines.
IWYU supports a mapping file, which we supply to help properly
handle things system headers (//toolchain/IWYU_mapping.imp)
Suggested Review Order:
- toolchain/build_toolchain.bzl to see how we get the IWYU
binaries into the toolchain
- toolchain/BUILD.bazel and toolchain/IWYU_mapping.imp
to see how the mapping file is made available for
all compile steps
- toolchain/clang_toolchain_config.bzl, where we define the
skia_enforce_iwyu feature to turn on any verification at
all and skia_opt_file_into_iwyu to enable the check for
specific files using a define.
- toolchain/clang_trampoline.sh, which is the toolchain is
configured to call instead of clang directly (see line 83
of clang_toolchain_config.bzl). This bash script used to
just forward all arguments directly onto clang. Now it
inspects them and either calls clang directly (if
it does not find the define in the arguments or we are
linking [bazel sometimes links with clang instead of ld])
or calls clang and then include-what-you-use. In all cases,
the trampoline sends the arguments to clang and IWYU
unchanged).
- //tools/sk_app/... to see enforcement enabled (and fixed)
for select files, as an example of that method.
- //experimental/bazel_test/... to see enforcement enabled
for all rules in a BUILD.bazel file.
- all other files.
Change-Id: I60a2ea9d5dc9955b6a8f166bd449de9e2b81a233
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519776
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
|
||
|
Kevin Lubick
|
9488e0237d |
[infra] Experiment generating BUILD.bazel files
This uses the gazelle extension from https://skia-review.googlesource.com/c/buildbot/+/473357 Review Tips: - Ignore any changes to .h or .cpp files. Those have been pulled out into their own CLs. - Start with bazel/macros.bzl. - Read the CL with the generation code, if you haven't already. - Look at third_party/file_map_for_bazel.json. - See experimental/bazel_test for an idea of how a cc_binary would be made. - Spot check one or two of the BUILD.bazel files. This CL generates the "atomic" rules for src/, include/ and modules/skshaper, as a starting point. `bazel build --config clang //include/...` works `bazel build --config clang //src/...` starts compiling, (which verifies that the BUILD.bazel files are all valid), but runs into errors because not all third_party deps have been resolved, and there are some files missing from the toolchain still (e.g. EGL headers). Change-Id: Ib7e0fb0efdb9f08655f06cbc56e9bb4cf416294b Bug: skia:12541 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474240 Reviewed-by: Ben Wagner <bungeman@google.com> Reviewed-by: Leandro Lovisolo <lovisolo@google.com> |
||
|
Kevin Lubick
|
97284f255b |
[infra] Add initial platforms and constraint values
This makes use of Bazel's pre-defined platforms https://github.com/bazelbuild/platforms and some of our own defined values (see //bazel/common_config_settings/BUILD.bazel) to customize the build rules. I verified this by building bazel_test locally for linux x64 as well as using the third_party deps for a WASM build (using build files not seen in this CL). Suggested Review Order: - https://docs.bazel.build/versions/main/platforms.html if not already familiar with Bazel Platforms - third_party/BUILD.bazel to see that 1) all globs have been removed and 2) select() targets various platform constants or groups of constants to control sources, headers, and local_defines. - common_config_settings/ to see the groups of constraints created, as well as new constraint_settings defined (skdebug_impl) - supported_combinations/ to see how we can define supported sets of the constraint values (aka Bazel platforms). I imagine expanding this more, so we might have platforms named "linux_x64_emptyfontmgr_vulkan" or such. - //BUILD.bazel and bazel_test.cpp to see use of SkDebugf. - Everything else. Change-Id: I49e4abdbcf7b76f0674efdbb1f53dc8823d110ee Bug: skia:12541 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463517 Reviewed-by: Ben Wagner <bungeman@google.com> Reviewed-by: Leandro Lovisolo <lovisolo@google.com> Owners-Override: Kevin Lubick <kjlubick@google.com> |
||
|
Kevin Lubick
|
4d41304def |
[infra] Add hermetic toolchain for C/C++ using Clang+Musl
This can successfully build a C library: bazel build --config=clang //third_party:libpng This can build and run a statically-linked executable: bazel test --config=clang //:bazel_test For more verbose compile and linking output, add the `--features diagnostic` flag to a Bazel command (see _make_diagnostic_flags() in toolchain/clang_toolchain_config.bzl. Similarly, a `--features print_search_dirs` can be used to show where clang is looking for libraries etc to link against. These features are made available for easier debugging. Suggested review order: - Read https://docs.bazel.build/versions/4.2.1/tutorial/cc-toolchain-config.html if unfamiliar with setting up C++ toolchains in Bazel - .bazelrc and WORKSPACE.bazel that configure use and download of the toolchain (Clang 13, musl 1.2.2) - toolchain/build_toolchain.bzl which downloads and assembles the toolchain (w/o installing anything on the host machine) - toolchain/BUILD.bazel and toolchain/*trampoline.sh to see the setup of the toolchain rules. - toolchain/clang_toolchain_config.bzl to see the configuration of the toolchain. Pay special attention to the various command line flags that are set. - See that tools/bazel_test.cc has made a new home in experimental/bazel_test/bazel_test.cpp, with a companion BUILD.bazel. Note the addition of some function calls that test use of the C++ standard library. The number being used to test the PNG library is the latest and greatest that verifies we are compiling the one brought in via DEPS (and not a local one). - third_party/* to see how png (and its dependent zlib) have been built. Pay special attention to the musl_compat hack to fix static linking (any idea what the real cause is?) - //BUILD.bazel to see definition of the bazel_test executable. Change-Id: I7b0922d0d45cb9be8df2fd5fa5a1f48492654d5f Bug: skia:12541 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461178 Reviewed-by: Ben Wagner <bungeman@google.com> Reviewed-by: Leandro Lovisolo <lovisolo@google.com> |