a43a68a015
5 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
|
6ba9f702ba |
[bazel] Try adding cc_binary rules that use generated rules
To make the atomic rules a bit easier to work with, in many of the folders, this adds in cc_library rules to group together the sources from that folder (and subfolders where prudent). We only needs sources because those atoms should have their headers as deps. One issue that was pointed out is that there is currently no way to restrict the inclusion of certain packages, a la, `gn check`. For example, there is no mechanism from stopping a dev from adding #include "modules/canvaskit/WasmCommon.h" to something in //src/core (except circular dependencies). We can probably address that using Bazel's visibility rules as needed: https://docs.bazel.build/versions/main/visibility.html https://docs.bazel.build/versions/main/be/functions.html#package_group It is recommended to look at this CL patchset by patchset. PS1: Update gazelle command to generate rules in more folders. PS2: A few changes to make generation work better. PS3: The result of running make generate in //bazel PS4: Adding the rules to build sksllex, the simplest binary I could find in the Skia repo. PS5: Adding the rules to build skdiff, a more complex binary. I tried a few approaches, but ended up gravitating back towards the layout where we have each folder/package group up the sources. I imagine at some point, we'll have skdiff depend on skia_core or something, which will have things like //src/core, //src/codecs, //src/pathops all bundled together. PS7: Added in the groupings of sources, similar to what we had earlier. I liked these for readability. These helped fix up the //:skia_core build, and by extension, the CanvasKit build. Change-Id: I3faa7c4e821c876b243617aacf0246efa524cbde Bug: skia:12541 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476219 Reviewed-by: Ben Wagner <bungeman@google.com> Reviewed-by: Leandro Lovisolo <lovisolo@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
|
888d4efa77 |
[canvaskit] Add Freetype/Fonts to Bazel Build
This re-works src/ports/BUILD.bazel to work like our other BUILD files, i.e. one rule "srcs" that brings in the necessary private filegroups. To work around an abort with LLVM [1], we have to go back to an earlier version of emscripten (temporarily?). Future work should look at using transitions [2] to allow various executables (e.g. CanvasKit, DM) to set their own set of Bazel flags, w/o the build invokers having to specify them. These transitions might be able to handle more complex cases that we currently use if statements in GN to deal with. The Freetype build rule was created by taking the BUILD.gn rule, adding in all the sources listed there and then playing compile-whack-a-mole to add in all the headers and included .c files. Suggested Review Order: - third_party/BUILD.bazel to see freetype build rules - bazel/common_config_settings/ to see treatment of fontmgr like codecs (many possible) and fontmgr_factory (only one). - src/ports/BUILD.bazel - BUILD.bazel - modules/canvaskit/BUILD.bazel. Take note of the gen_rule that calls tools/embed_resources.py to produce the .cpp file containing the embedded font data. - Everything else. [1] https://github.com/emscripten-core/emscripten/issues/15528 [2] https://github.com/bazelbuild/examples/tree/main/rules/starlark_configurations/cc_binary_selectable_copts Bug: skia:12541 Change-Id: I08dab82a901d80507007b354ca20cbfad2c2388f Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471636 Reviewed-by: Ben Wagner <bungeman@google.com> Reviewed-by: Leandro Lovisolo <lovisolo@google.com> |
||
|
Kevin Lubick
|
8ed49ea6e3 |
[infra] Add Bazel rules for codecs.
This ports the third_party BUILD.gn files related to codecs (with a best-effort on arm/SIMD stuff). This includes: - libpng - libjpeg-turbo - libwebp - wuffs (gif) - libgifcodec - dng_sdk and piex (raw codec) This expands the string_flag_with_values macro to allow multiple values to be set at once. This was added in Bazel 5.0.0, however the latest pre-release version of that has a bug [1] which slows down compilation dramatically. This was fixed at ToT, but not released. As a result, I started using the Bazel 6 pre-release (via bazelisk). The macro select_multi makes writing select() where multiple elements could be on possible/easier. One can try compiling certain codecs by running: bazel build :skia-core --config clang --include_codec=raw_codec --include_codec=png_codec Suggested Review Order: - bazel/macros.bzl - bazel/common_config_settings/defs.bzl and its BUILD.bazel to see how the codec options are defined. - BUILD.bazel to see how the codec settings are used. - src/codec/BUILD.bazel to see the inclusion of Skia files to deal with specific codecs. - third_party/BUILD.bazel (while referencing the corresponding BUILD.gn files, such as third_party/libwebp/BUILD.gn) - Everything else. Change-Id: I797375a35fa345d9835e7b2a2ab23371c45953c3 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469456 Reviewed-by: Leandro Lovisolo <lovisolo@google.com> Reviewed-by: Ben Wagner <bungeman@google.com> Reviewed-by: Leon Scroggins <scroggo@google.com> |