AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity
59,943 Commits 1 Branch 0 Tags 544 MiB
f0fdce0b42
Commit Graph

5 Commits

Author SHA1 Message Date
Kevin Lubick
fed97e8f40 [bazel] Add RBE support using hermetic Linux Clang toolchain 
A new RBE worker-pool called gce_linux was created in
conjunction with this CL. See
https://docs.google.com/document/d/14xMZCKews69SSTfULhE8HDUzT5XvPwZ4CvRufEvcZ74/edit#
for some details on that.

Note: everything under bazel/rbe/gce_linux was autogenerated
and can be ignored from manual review. It basically specifies
what files are on the RBE image that are necessary for running
Bazel.

Testing it out can be done by authenticating for RBE
gcloud auth application-default login --no-browser

Then, run make -C bazel rbe_known_good_builds
to test it out.

On my 4 core laptop with an empty local cache, but a
warm remote cache, the build took <2 min instead of the
10+ minutes it would have [1].

The folder structure in //bazel/rbe is meant to let us
have multiple remote configurations there, e.g.
//bazel/rbe/gce_windows.

Suggested Review Order:
 - bazel/rbe/README.md
 - bazel/rbe/gce_linux_container/Dockerfile to see the
   bare-bones RBE image.
 - bazel/rbe/BUILD.bazel to see a custom platform defined.
   It is nearly identical to the autogenerated one
   in bazel/rbe/gce_linux/config/BUILD, with one extra
   field to force the gce_linux pool to be used.
 - .bazelrc to see the settings needed to make
   --config=linux-rbe work. The naming convention was
   inspired by SkCMS's setup [2], and allows us to have
   some common RBE settings (i.e. config:remote) and
   some specialized ones for the given host machine
   (e.g. config:linux-rbe) A very important, but subtle
   configuration, is on line 86 of .bazelrc where we say
   to use our hermetic toolchain and not whatever C++
   compiler and headers are on the host machine (aka
   the RBE container).
 - toolchain/build_toolchain.bzl to see some additional
   dependencies needed in the toolchain (to run IWYU) which
   I had installed locally but didn't realize were important.
 - third_party/BUILD.bazel to see an example of how failing
   to specify all files can result in something that works
   locally, but fails remotely.
   --execution_log_json_file=/tmp/execlog.json helped debug
   these issues.
 - All other files.

[1] http://go/scrcast/NjM1ODE4MDI0NzM3MTc3Nnw3ODViZmFkMi1iOA
[2] https://skia.googlesource.com/skcms/+/30c8e303800c256febb03a09fdcda7f75d119b1b/.bazelrc#20


Change-Id: Ia0a9e6a06c1a13071949ab402dc5d897df6b12e1
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524359
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
 2022-03-28 13:56:16 +00:00
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>
 2022-03-16 13:09:46 +00:00
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>
 2022-01-11 13:06:19 +00:00
Kevin Lubick
1f8c31b101 [infra] Add initial Bazel rules and files 
These rules can be used to build our GMs on WASM+WebGL and
libskia.a with just the CPU backend (and most other features
turned off).

This can be done with the following commands:
  - bazel build //modules/canvaskit:gm-bindings-wasm --gpu_backend=gl_backend --with_gl_standard=webgl_standard
  - bazel build :skia-core --config clang

This pivots slightly from http://review.skia.org/463517
by using config_settings [1] instead of platforms for
the optional features that we control. This pivot was
suggested in [2]

We have BUILD.bazel files in many of the subdirectories
that specify filegroups for the appropriate files. In
an effort to make //BUILD.bazel more readable, it is
the responsibility of these subfolders to deal with
conditionally including certain .h or .cpp files.
This is done using select statements and config_settings
or platform constraints as necessary.

For example, src/gpu/BUILD.bazel will different private
filegroups for each of the supported gpu backends [3]
and a more-visible filegroup called "srcs" that has
the right selection of the private files to be used
for compilation.

An effort has been made to avoid using glob() in our
BUILD.bazel files. These file lists were made by using
`ls -1` and some regex to add in quotes. We might want
to make a helper script to assist with that, if necessary.

To specify which options we have, the settings in
//bazel/common_config_settings/BUILD.bazel have been
redesigned. They make use of a macro `string_flag_with_values`
that removes the boilerplate. Patchset 36 shows what the
file looks like w/o the macro.

The top level BUILD.bazel file will still need to use
some logic to handle defines, because local_defines is
a list of strings, not a list of labels [4].

Suggested Review Order:
  - WORKSPACE.bazel to see the new dependencies on the
    emsdk toolchain and bazel_skylib
  - bazel/common_config_settings/* to see the few settings
    defined (we have more to define, see BUILD.gn and
    //gn/skia.gni for ideas)
  - BUILD.bazel to see the "skia-core" cc_library rule.
    See also "gms" and "tests"
  - modules/canvaskit/BUILD.bazel to see the use of
    the emscripten "wasm_cc_binary" rule, which depends
    on the "skia-core", "gms", and "tests" rule. Note that
    it only builds some of the gms as a proof of concept.
  - The other BUILD.bazel files. Some of these are not
    platform or feature dependent (e.g. pathops). Others
    are (e.g. gpu).
  - All other files.

[1] https://docs.bazel.build/versions/4.2.1/skylark/config.html#user-defined-build-settings
[2] https://github.com/emscripten-core/emsdk/pull/920
[3] In this CL, that's just the webgl one.
[4] https://docs.bazel.build/versions/main/be/c-cpp.html#cc_library.local_defines

Change-Id: Ieecf9c106d5e3a6ae97d13d66be06b4b3c207089
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458637
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Owners-Override: Kevin Lubick <kjlubick@google.com>
 2021-11-09 12:32:25 +00:00
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>
 2021-10-21 12:43:49 +00:00