Commit Graph

11 Commits

Author SHA1 Message Date
Kevin Lubick
c3a448ec61 [bazel] Put licenses() after legacy_exports
G3 prefers license() first.

This was done mechanically with a big find/replace

Change-Id: I8c33c7bc10a6bec42e966cad81c259954e841811
Bug: skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535898
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-05-02 15:04:33 +00:00
Kevin Lubick
46eaab3959 [bazel] Add shims to help translation into G3
Ran the following commands:
find -name "BUILD.bazel" -exec sed -i -e '1iload("//bazel:macros.bzl", "cc_library", "exports_files_legacy")\nexports_files_legacy()' {} +
buildifier --lint=fix --mode=fix -r .

This had the effect of making sure we can export all of our
files in G3 (until we no longer have legacy targets) and
making all of our cc_libraries shim-able.

bazel/macros.bzl has the human-contributed changes, the rest
were mechanical.

Change-Id: I8e24e30e74b038cfd072cdbe4078bfd1d213dd46
Bug: skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535359
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-04-29 19:27:54 +00:00
Kevin Lubick
83cee23c98 [bazel] Run buildifier on BUILD.bazel files
buildifier --lint=fix -r .

Change-Id: I6a41858270d20137978f8271c8f6160b51120777
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529751
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-04-14 18:13:43 +00:00
Kevin Lubick
b98328a27b [bazel] Add license to all our BUILD.bazel files
find -name "BUILD.bazel" -exec sed -i -e '1i licenses(["notice"])\n' {} +

Change-Id: Ie48f163b7d8d6ede9ba5f952e87232dd5c9fa8e6
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529808
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-04-13 19:50:29 +00:00
Kevin Lubick
5e8f45faf1 [includes] Prepare for moving SkColorSpace to forward declare
This updates all our callsites in preparation for removing
the #include "include/core/SkColorSpace.h" from SkImageInfo.h

According to go/chrome-includes [1], this will save ~150MB
(0.07%) from the compilation size. I think SkColorSpace is
a big include because it loads the skcms header, which is
big.

The follow-on CL will remove that link, once clients have
been updated as well.

[1] https://commondatastorage.googleapis.com/chromium-browser-clang/chrome_includes_2022-03-31_124042.html#view=edges&filter=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImageInfo%5C.h%24&sort=asize&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImageInfo%5C.h%24&included=&limit=1000

Change-Id: I1b5ff491ac495317b0e5af3a2082b080d43697ae
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525639
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-31 19:50:10 +00:00
Kevin Lubick
acab911351 [bazel] Make use of test_on_env to spin up server for gms
In order to extract the PNG files produced by our CanvasKit gms,
we need our JS tests to POST them to a server which can write to
disk. The easiest way to do this is to use the test_on_env
rule defined in the Skia Infra repo for exactly this purpose.

This required https://skia-review.googlesource.com/c/buildbot/+/510717
to be able to configure the binary correctly and
https://skia-review.googlesource.com/c/buildbot/+/511862, for nicer
debugging so the skia-infra dep was updated via the following commands:
$ go get go.skia.org/infra@d8a552a29e
$ go mod download
$ make -C infra/bots train
$ make -C bazel gazelle_update_repo
This caused many automated changes to infra/bots/tasks.json

The flow is:
1. User types bazelisk test :hello_world_test_with_env
2. The test_on_env rule starts gold_test_env and waits
   for the file defined in $ENV_READY_FILE to be created.
3. gold_test_env starts a web server on a random port. It
   writes this port number to $ENV_DIR/port. Then, it
   creates $ENV_READY_FILE to signal ready.
4. test_on_env sees the ready file and then starts the
   karma_test rule. (Reminder: this is a bash script
   which starts karma using the Bazel-bundled chromium).
5. The karma_test rule runs the karma.bazel.js file (which
   has been injected with some JS code to fill in Bazel
   paths and settings) using Bazel-bundled node. This reads
   in the port file and sets up a Karma proxy to redirect
   /gold_rpc/report to http://localhost:PORT/report
6. The JS tests run via Karma (and do assertions via Jasmine).
   Some tests, the gms, make POST requests to the proxy.
7. gold_test_env gets these POST requests writes the images
   to a special Bazel folder on disk as defined by
   $TEST_UNDECLARED_OUTPUTS_DIR.
8. test_on_env identifies that the tests finish (because the
   karma_test script returns 0). It sends SIGINT to gold_test_env.
9. gold_test_env stops the webserver. The special Bazel folder
   will zip up anything inside it and make it available for
   future rules (e.g. a rule that will upload to Gold via goldctl).

Suggested Review Order:
 - bazel/karma_test.bzl to see the test_on_env rule bundled into
   the karma_test macro. I chose to put it there because it might
   be confusing to have to define both a karma_test and test_on_env
   rule in the same package but not be able to call one because it
   will fail to talk to the server.
 - gold_test_env.go to see how the appropriate files are written
   to signal the environment is ready and the handlers are set up.
 - karma.bazel.js to see how we make our own proxy given the
   port from the env binary. The fact that we could not create
   our own proxy with the existing karma_test rule was why the
   chain ending in https://skia-review.googlesource.com/c/skia/+/508797
   had to be abandoned.
 - tests/*.js to see how the environment is probed via /healthz
   and then used to make POST requests with data.
 - Everything else.

Change-Id: I32a90def41796ca94cf187d640cfff8e262f85f6
BUG: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510737
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
2022-02-28 14:05:54 +00:00
Kevin Lubick
4bd08c52c0 [infra] Add SkParagraph (harfbuzz, ICU) to Canvaskit Bazel build.
As a follow-up to https://skia-review.googlesource.com/c/skia/+/476219,
this sketches out how we can maybe use cc_library for the things
in //modules to make sure something in //src doesn't depend on
anything in //modules, for example.

The following succeeds:
bazel build //modules/skparagraph:skparagraph --config=clang \
  --shaper_backend=harfbuzz_shaper --with_icu

As does `make bazel_canvaskit_debug` in //modules/canvaskit

Suggested Review Order:
 - third_party/BUILD.bazel for ICU and harfbuzz rules. Pay
   special attention to the genrules used to call the python
   script for turning the icu .dat file into .S or .cpp.
 - bazelrc and bazel/ for new flags and defines that control
   use of ICU and harfbuzz. Unlike GN, with the public_defines
   that get added in automatically if icu or harfbuzz is
   depended upon, we need to set the defines at the top level.
   This necessity might go away if we change the atoms to
   depend on //modules/skshaper, which could define that flag.
 - Top level BUILD.bazel files in //modules/skparagraph,
   //modules/skshaper, //modules/skunicode, //modules/canvaskit
 - All other .bazel file changes are automatic.

Bug: skia:12541
Change-Id: I38a9e0a9261d7e142eeb271c2ddb23f362f91473
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478116
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
2021-11-30 21:01:06 +00:00
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>
2021-11-30 18:18:53 +00:00
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>
2021-11-23 20:00:15 +00:00
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>
2021-11-23 18:53:16 +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