6b6015ff79
6 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
|
7a14f783bd |
[bazel] Sketching out HelloWorld sk_app using GL
bazel run //example:hello_world --config=clang causes a window to open and draws a circle and a square. Text to follow in a future CL. To make this work, I had to get rid of musl and use glibc. All the shared libraries (.so files) that were pre-built and available for download (e.g. from https://packages.debian.org/bullseye/amd64/libgl1/download) were compiled against glibc. When I tried to run a program statically linked with musl and dynamically linked against things using glibc, I got a segmentation fault on things like calloc(). Initial attempts to use glibc had failed because it was thought that the libc.so.6 file could only be referred to by absolute path (and thus Bazel would not be happy about it). As it turns out, that was simply a misconfiguration of the builtin_sysroot parameter to cc_common.create_cc_toolchain_config_info (see //toolchain/clang_toolchain_config.bzl). By setting that to `external/clang_linux_amd64` and not `external/clang_linux_amd64/usr`, the libc binary which had been extracted to `external/clang_linux_amd64/lib/x86_64-linux-gnu` was perfectly reachable from `external/clang_linux_amd64/usr/usr/lib/x86_64-linux-gnu/libc.so` To bring in the shared libraries to link against (e.g. X11, GL) I made build_toolchain.bzl easier to modify in that we simply need to add a debian download url and sha256 hash to a list (rather than having to plumb this through via arguments). Recommended Review Order: - example/BUILD.bazel (not sure if we always want to set bare link arguments like that or if we want to use "features" to pass those along to the toolchain). - tools/sk_app/BUILD.bazel to see initial cc_library for wrapping sk_app code. - toolchain/build_toolchain.bzl to see removal of musl and new list of debs. - toolchain/clang_toolchain_config.bzl (where use of the no-canonical-prefixes was key to compilation success). Notice also that we statically linked libc++ (I did not have any shared libraries for it locally, so I guessed a typical developer might not either). - Rest of toolchain/ for trivial renames. - bazel/Makefile to see extra docs on those targets and a new target that compiles all the exes so far for a quick way to test the build. - third_party/BUILD.bazel and src/gpu/BUILD.bazel which have non-generated changes. (all other BUILD.bazel files do). - go.mod, which needed to update the infra repo version in order to pick up http://review.skia.org/491736). Change-Id: I8687bd227353040eca2dffa9465798d8bd395027 Bug: skia:12541 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/492117 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
|
cc9d0cd5a9 |
[bazel] Add go/gazelle to WORKSPACE and use c++ extension.
This adds a simple go program to test the installed go toolchain, and a Make rule to codify the arguments to our gazelle binary, built with extensions. I could not figure out how to get the .json file to work with the gazelle() Bazel rule, but this works ok for now. Bug: skia:12541 Change-Id: I5067b15c7518951aeb69559d3871799d3b5745f4 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475716 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
|
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> |