Commit Graph

26 Commits

Author SHA1 Message Date
John Stiles
03b2e3aeee Revert "set up GL sample app to build through Bazel Mac toolchain"
This reverts commit 9be648ad64.

Reason for revert: breaking G3 roll

Original change's description:
> set up GL sample app to build through Bazel Mac toolchain
>
> Suggested review order.
> 1) tools/sk_app/* and src/gpu/ganesh/*
> sets up the actual target to be built by the toolchain
> 2) toolchain/* and .bazelrc
> changes to the mac hermetic toolchain, including support for framework dependencies, objc compilation, and dynamic lib dependency resolution
>
> Change-Id: Ic8209b97a0d8448f984d43a579e600ba4e9118e1
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549897
> Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
> Reviewed-by: Ben Wagner <bungeman@google.com>

Change-Id: I5414b94f2c6175ea8c7dbc6cd72c7dd8d1b47892
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/551236
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2022-06-17 20:07:06 +00:00
Jorge Betancourt
9be648ad64 set up GL sample app to build through Bazel Mac toolchain
Suggested review order.
1) tools/sk_app/* and src/gpu/ganesh/*
sets up the actual target to be built by the toolchain
2) toolchain/* and .bazelrc
changes to the mac hermetic toolchain, including support for framework dependencies, objc compilation, and dynamic lib dependency resolution

Change-Id: Ic8209b97a0d8448f984d43a579e600ba4e9118e1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549897
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-06-17 13:54:14 +00:00
Kevin Lubick
fb13c18ddf [bazel] Update to v5.2.0
The primary change we are looking for is
"Add support for .ar archives (and .deb files)
https://github.com/bazelbuild/bazel/pull/15218"

https://blog.bazel.build/2022/06/08/bazel-5.2.html

In theory, we should be able to trigger Linux RBE
compilations (e.g. IWYU) from Mac with this change.

Change-Id: I217406d21fd55507e090c4bb5f79c796230717e6
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549659
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
2022-06-13 20:35:18 +00:00
Kevin Lubick
e25e4dc7e7 [bazel] Specify files required for the toolchain more precisely.
This improves performance with sandboxing.

One can have Bazel output performance data [1][2], which can be
viewed via chrome://tracing or https://ui.perfetto.dev/.

With Perfetto, the following SQL queries [3][4] can summarize
the sandboxing metrics, as well as the actual compilation
time. http://screen/5TxbeZTso4gNDfD
Note that the dur column is in nanoseconds, so we convert
to seconds. These numbers could further be divided by
the number of processes (in my case 48) to get a scaled output.

SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "sandbox.createFileSystem";
SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "sandbox.delete";
SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "subprocess.run";

I benchmarked the local compilation of //:skia_public using
--config=clang_linux (our custom Linux toolchain). I was
sure to clear the Bazel cache before each run and not count
the time to download/update a toolchain for the first time.

The control measurements (without this CL) are:
  Wall Time = 272.2s
  sandbox.createFileSystem = 5466.9s
  subprocess.run = 2961.0s
  sandbox.delete = 4112.3s

With this CL:
  Wall Time = 53.9s (5.05x faster)
  sandbox.createFileSystem = 403.4s
  subprocess.run = 1610.3s
  sandbox.delete = 348.8s

The control measurement is a touch misleading. Due to it being
so slow, I had recommended developers use a ramdisk when building
on machines with sufficient RAM via the Bazel flag
--sandbox_base=/dev/shm

Here is the control measurement when using a RAM disk:
  Wall Time = 21.2s
  sandbox.createFileSystem = 58.9s
  subprocess.run = 705.1s
  sandbox.delete = 46.6s

With this CL and a RAM disk:
  Wall Time = 19.2s (10% faster)
  sandbox.createFileSystem = 21.8s
  subprocess.run = 722.9s
  sandbox.delete = 16.2s

For devs who cannot or are not using a RAM disk, this is
a huge win. With a RAM disk, it's a modest improvement.

On an RBE build, this had minimal impact. I did my best
to bust the action cache with a fake define and the before
and after times were about the same.

This was inspired by [5] and [6].

[1] Add --profile=/tmp/profile.gz to any command
[2] https://bazel.build/rules/performance#performance-profiling
[3] https://perfetto.dev/docs/quickstart/trace-analysis#sample-queries
[4] https://perfetto.dev/docs/analysis/sql-tables#slice
[5] 93f21c9ef3
[6] 311acff345
Change-Id: Iceb2606e86111159141a286d01fc002d09fe3fe4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547822
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
2022-06-09 11:58:45 +00:00
Kevin Lubick
69a60e6dc0 [includes] Enforce IWYU on several directories
These four directories had IWYU enforced previously and
that enforcement was turned back on:
  - src/sksl/
  - src/utils/
  - src/svg/
  - tools/debugger/

It was discovered that src/sksl/ir had been missed with the
previous IWYU enforcement, so many files needed updating
(see https://skia-review.googlesource.com/c/skia/+/547256).

We do not currently include src/svg/ or tools/debugger/ in
any Bazel builds, so that enforcement has not been tested
with the new system. When we add in builds that use those
packages, we may need to update includes.

Suggested Review order:
 - clang_trampoline_linux.sh to see list expanded
 - bazel/Makefile to see convenient target for testing this
   locally (follow-up CL will have a CI job for this).

Change-Id: Ifef1659ccd1a0e6c862b82102576a06296a6b42e
Bug: skia:12541 skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546608
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-06-06 14:29:47 +00:00
Kevin Lubick
590d3cc8f7 [bazel] Re-implement IWYU enforcement
Instead of having a feature that a subpackage cc_library
would set (because the subpackage cc_libraries went away in
review.skia.org/543977), we now have a list of directories
for which all descendent files will have IWYU run on them.

As before, we have an overarching feature "skia_enforce_iwyu"
that enables this check because of the extra compilation
overhead it incurs.

We have decided to enforce IWYU on debug builds instead of
release builds because we have some code (e.g. that in
SkDEBUGCODE or SkASSERT) which is only compiled and executed
in a debug build, but we don't want to have
#if defined(SK_DEBUG)
all over the place. We make the assumption that the includes
needed to compile in debug mode are the superset of the
includes necessary for release and other modes.

Change-Id: I10254fcc162627c20eb89959e06417effa3cc396
Bug: skia:12541 skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546607
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-06-06 14:29:47 +00:00
Jorge Betancourt
b894c69abb set up tools for building Skia on Mac semi hermetically
Reviewer notes:
PS1 and PS2 handle everything up to the linking stage of the build
PS6 and PS7 are trivial renamings and rebases, diff between Base->PS5 for a cleaner review

No-Try: true
Change-Id: Ib21ce2e8839ecd4b4dd57280e82f56a98194e476
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532765
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2022-05-04 16:56:46 +00:00
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
62f57a02b3 [includes] Enforce IWYU on src/utils
Client changes:
 - cl/443664347
 - cl/444232853
 - http://ag/17915718
 - http://ag/17913178
 - https://crrev.com/c/3602239

Change-Id: I16bcdbbe2b13bbf52f007b0f131e9ee0c14ed237
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532257
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-04-28 17:32:20 +00:00
Ethan Nicholas
b642184a32 Improved performance of determining line numbers during VM code generation
Change-Id: Ic1723ce7eb760d6b64ca5f694d94283334918899
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/533805
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-04-26 22:32:28 +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
9b7db63e6f [bazel] Sketch out changes for Mac toolchain
Change-Id: Idae84d8d9538012e5cfb75a1a477dbc72a4da5bc
Bug: skia:13125
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526264
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
2022-04-08 13:35:44 +00:00
Kevin Lubick
556ca8f7ee [includes] Enforce IWYU for //tools/debugger/...
This moves the Build-Debian10-BazelClang-x86_64-Release-IWYU
job from experimental to on when a file in one of the
folders that we enforce IWYU is modified (currently
for svg, sksl, and now, debugger).

Change-Id: Ia6fe1e7b30fc486db3eb081b6a64bc4c250cbf0b
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525796
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-30 13:53:13 +00:00
Kevin Lubick
c40158ab98 [bazel] Add CI job that uses RBE to enforce includes
The toolchain now uses extract_ar from
https://skia-review.googlesource.com/c/buildbot/+/524764
which is a static executable to extract the .deb files.
This was necessary because the llvm-ar that had previously
been used requires glibc 2.31+ to run, but our Debian10
machines on Swarming have an older version (2.28).

A longer-term fix is to have Bazel support .ar files,
which I plan to attempt to contribute this week.

The RBE task will be added as an experimental CQ job, to
see how it handles the load of running often. With the
remote execution cache, I hope it performs well, once
the toolchains are cached on both the Swarming
machines and in the RBE workers.

Note: We had to add several files to the CAS spec
(see compile_cas.go) which are required for Bazel to work.

Change-Id: Ie70c70d5f33768c957760f9eeb7835025109b487
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524759
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
2022-03-28 13:56:16 +00:00
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
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