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

9 Commits

Author SHA1 Message Date
Jorge Betancourt
c88d96f890 fix typo in toolchain comments 
Change-Id: I3fccd780fc1ca84ec7968c5f22d40d2d5c55b2a0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522982
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
 2022-03-21 20:20:04 +00:00
Kevin Lubick
08ece0c9a0 [includes] Enforce IWYU on sksl code 
PS1 regenerates the Bazel files. Use it as the base change when
    comparing patchsets.

IWYU seems to do a good job of working with MyFile.cpp and
MyFile.h, but if there is just a MyHeader.h, it doesn't always
seem to throw errors if the includes aren't correct. This was
observed with include/sksl/DSL.h This might be due to the fact
that headers are not compiled on their own, so they are never
sent directly to the IWYU binary.

This change sets enforce_iwyu_on_package() on the all sksl
packages and then fixes the includes until all those checks
are happy. There were a few files that needed fixes outside
of the sksl folder. Examples include:
 - src/gpu/effects/GrConvexPolyEffect.cpp
 - tests/SkSLDSLTest.cpp

To really enforce this, we need to add a CI/CQ job that runs
bazel build //example:hello_world_gl --config=clang \
  --sandbox_base=/dev/shm --features skia_enforce_iwyu

If that failed, a dev could make the changes described in
the logs and/or run the command locally to see those
prescribed fixes.

I had to add several entries to toolchain/IWYU_mapping.imp
in order to fix some private includes and other atypical
choices. I tried adding a rule there to allow inclusion of
SkTypes.h to make sure defines like SK_SUPPORT_GPU, but
could not get it to work for all cases, so I deferred to
using the IWYU pragma: keep (e.g. SkSLPipelineStageCodeGenerator.h)

Change-Id: I4c3e536d8e69ff7ff2d26fe61a525a6c2e80db06
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522256
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
 2022-03-21 12:43:02 +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
7ac7413f08 [bazel] Support Vulkan 
PS1 regenerates BUILD.bazel files

I suggest reviewing the deltas between PS1 and the latest
PS to focus on the interesting bits.

The changes here allow for a Vulkan-only build of HelloWorld
based on sk_app. The toughest change was properly fetching
the VisualID after removing the gl calls that used to
fill that in.

There are a few changes that fix resolution of Dawn
header files, but those won't actually be built until
a follow-on CL.

Change-Id: I54fb58b5dd7ecd4313562aed401759b3eaed53c0
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516999
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
 2022-03-08 21:21:17 +00:00
Kevin Lubick
98d664096d [bazel] Regenerate files and build with c++17 
See toolchain/clang_toolchain_config.bzl for the c++17 switch.
Most of the other changes were automatically generated
(with the exception of //third_party).

Change-Id: I8c0f4b29b5967da3f48b17eb298a7e92156277ac
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502407
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
 2022-02-01 13:15:22 +00:00
Kevin Lubick
25a7797820 [bazel] Add mirrors to every external dependency. 
- We always download from https
- All external assets have a primary and a mirror URL.
- We prioritize the sources as follows:
    bazel mirror, github/original source, our mirror
- There is a way (see build_toolchain) to test the sources
  from the mirrors (done before CL submission).

This adds a utility to upload files to the mirror in a
consistent, scripted way. It includes a way to copy in
parts of our bazel files (e.g. debs_to_install from
toolchain/build_toolchain.bzl) to update many things
at once.

Our Bazel mirror (gs://skia-world-readable/bazel)
is a Content Addressable Storage system, where the
file name is based on the sha256sum of the contents
(the same hash that Bazel uses). All files in it should
be publicly accessible.

Change-Id: Ida8b8e07d27a0a557bc49467ebbc86c806cabbd3
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494478
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
 2022-01-13 15:53:27 +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