7183f29125
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
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> |
||
|
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> |
||
|
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
|
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> |