Change-Id: Id28311d2a7de96edbc8bcd7fa6eadb66223e5278
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/558438
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Change-Id: I2fdbff5c4d58d5eace99abd809b2c9861c629ce4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/557356
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
The tools in genrules [1] correspond to the HOST's version, which
is a problem if we are doing cross-platform builds. In this case,
when I was using my M1 Mac to run on a Linux RBE platform, the
Mac's python binaries (e.g. python3_9_aarch64-apple-darwin/bin/python3)
were being uploaded to RBE and attempted to be used, which resulted
in OSError: [Errno 8] Exec format error because the won't run
on the Linux RBE machine.
Thanks to tjgq@, I learned about exec_tools [2], which will use
the version of the tools for the EXECUTION platform, which
is exactly what we want.
While debugging this, I added a minimal reproduction case in
//experimental/bazel_test and updated the py_tools version
in an effort to diagnose the issue further.
We will need to contribute similar fixes to @spirv_tools
[1] https://bazel.build/reference/be/general#genrule.tools
[2] https://bazel.build/reference/be/general#genrule.exec_tools
Bug: skia:12541
Change-Id: Ib14deb4e326d3103fd08c21e93afe342d751c17a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/554518
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Bug: skia:13477
Change-Id: I524e076282e9dde3a8ad5d39c58e6054f684fbf0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/554376
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
MSVC and clang-cl do not recognize -isystem, but do recognize
-I. Thus if those compilers use our rules, we want our copts
to be maximally compatible.
Some of these options didn't seem to do anything, so I removed
them rather than replace them.
Change-Id: I73feb63d5c682a6df646d731b10ca2509e105e6f
Bug: skia:12541, b/237076898
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/553878
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
This can let us create versions of skia_public with certain settings
enabled in an easy way (i.e. not requiring clients to set the compile
flags themselves).
Change-Id: I909449c6186f4912ea80dd9d579fcd7cafb4775c
Bug: skia:12541 b/237076898
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/553377
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Change-Id: Id6efc88e2d048ade8aa2c0e7870c0bc73b1afcf5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/553537
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: James Godfrey-Kittle <jamesgk@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: James Godfrey-Kittle <jamesgk@google.com>
Our SVG code depends on our XML code which depends on expat.
See also cl/457481999
Change-Id: I83b61f5d73570d0aa7e851a01ccd019b4b1019e4
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/553376
Reviewed-by: Florin Malita <fmalita@google.com>
These changes are necessary to use the toolchain on both the
M1 Mac and Intel Macs.
This adds a way to detect the host platform and choose different
compile options in the toolchain.
We cannot statically link in libc++.a from the clang zip because
it appears to be x64 only.
Finally, this fixes copts not being passed to objective c libraries.
Known issue:
- Intel Mac building has an error about the default CC toolchain.
Change-Id: Ie8e5e83dc41513563ac684e70a8a6947b36df445
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/552472
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
The big change here is having the C++ toolchain use
Bazel platforms instead of the C++ specific flags/setup.
In Bazel, platforms are a general purpose way to define
things like os, cpu architecture, etc. We were not using
platforms previously, because the best documentation at
the time focused on the old ways.
However, the old ways were clumsy/difficult when trying
to manage cross-compilation, specifically when trying
to have a Mac host trigger a build on our Linux RBE
system targeting a Linux x64 system. Thus, rather than
keep investing in the legacy system, this CL migrates
us to using platforms where possible.
Suggested background reading to better understand this CL:
- https://bazel.build/concepts/platforms-intro
- https://bazel.build/docs/platforms
- https://bazel.build/docs/toolchains#registering-building-toolchains
The hermetic toolchain itself is not changing in this CL
(and likely does not need to), only how we tell Bazel
about it (i.e. registering it) and how Bazel decides
to use it (i.e. resolving toolchains).
Here is my understanding of how platforms and toolchains
interact (supported by some evidence from [1][2])
- Bazel needs to resolve platforms for the Host, Execution,
and Target.
- If not specified via flags, these are the machine from
which Bazel is invoked, aka "@local_config_platform//:host".
- With this CL, the Host could be a Mac laptop, the Execution
platform is our Linux RBE pool, and the Target is "a Linux
system with a x64 CPU"
- To specify the Host, that is, describe to Bazel the
capabilities of the system it is running on, one can
set --host_platform [3] with a label pointing to a platform()
containing the appropriate settings. Tip: have this
platform inherit from @local_config_platform//:host
so it can add to any of the constraint_settings and
constraint_values that Bazel deduces automatically.
- To specify the Target platform(s), that is, the system
on which a final output resides and can execute, one
can set the --platforms flag with a label referencing
a platform().
- Bazel will then choose an execution platform to fulfill
that request. Bazel will look through a list of available
platforms, which can be augmented* with the
--extra_execution_platforms. Platforms specified by this
flag will be considered higher than the default platforms!
- Having selected the appropriate platforms, Bazel now
needs to select a toolchain to actually run the actions
of the appropriate type.
- Bazel looks through the list of available toolchains
and finds one that "matches" the Execution and the Target
platform. This means, the toolchain's exec_compatible_with
is a strict subset of the Execution platform and
the toolchain's target_compatible_with is a strict subset
of the Target platform. To register toolchains* (i.e. add
them to the resolution list), we use --extra_toolchains.
Once Bazel finds a match, it stops looking.
Using --toolchain_resolution_debug=".*" makes Bazel log
how it is resolving these toolchains and what execution
platform it picked.
* We can also register execution platforms and toolchains in
WORKSPACE.bazel [4], but the flags come with higher priority
and that made resolution a bit tricky. Also, when we want
to conditionally add them (e.g. --config=linux_rbe), we
cannot remove them conditionally in the WORKSPACE.bazel file.
The above resolution flow directly necessitated the changes
in this CL.
Example usage of the new configs and platforms:
# Can be run on a x64 Linux host and uses the hermetic toolchain.
bazel build //:skia_public
# Can be run on Mac or Linux and uses the Linux RBE system along
# with the hermetic toolchain to compile a binary for Linux x64.
bazel build //:skia_public --config=linux_rbe --config=for_linux_x64
# Shorthand for above
bazel build //:skia_public --config=for_linux_x64_with_rbe
Notice we don't have to type out --config=clang_linux anymore!
That was due to me reading the Bazel docs more carefully and
realizing we can set options for *all* Bazel build commands.
Current Limitations:
- Targets which require a py_binary (e.g. Dawn's genrules)
will not work on RBE when cross compiling because the
python runtime we download is for the host machine, not
the executor. This means //example:hello_world_dawn does
not work on Mac when cross-compiling via linux_rbe.
- Mac M1 linking not quite working with SkOpts settings.
Probably need to set -target [5]
Suggested Review order:
- toolchain/BUILD.bazel Notice how we do away with
cc_toolchain_suite for toolchain. These have the same
role: giving Bazel the information about where a toolchain
can run. The platforms one is more expressive (IMO), allowing
us to say both where to run the toolchain and what it can
make. In order to more easily force the use of our hermetic
toolchain, but also allow the hermetic toolchain to be used
on RBE, we specify "use_hermetic_toolchain" only on the target,
because the RBE image does not have the hermetic toolchain
on it by default (but can certainly run it).
- bazel/platform/BUILD.bazel to see the custom constraint_setting
and corresponding constraint_value. The names for both of these
are completely arbitrary - they do not need to have any deeper
meaning or relation to any file or Docker image or system or
any other constraints. Think of the constraint_setting as
an Enum and the constraint_value being the one and only member.
We need to pass around a constant value, not a type, so we
need to provide the constraint_value (e.g. in toolchain/BUILD.bazel)
but not a constraint_setting. However we need a
constraint_setting declared so we can make a constraint_value
of that "type".
Notice the platform declared here - it allows us to force
Bazel to use the hermetic toolchain because of the extra
constraint_value.
- .bazelrc I set a few flags that will be on for all
bazel build commands. Importantly, this causes the C++
build logic to use platforms and not the old, bespoke way.
I also found a way to avoid using the local toolchain on
the host, which will hopefully lead to clearer errors
if platforms are mis-specified instead of odd compile
errors because the host toolchain is too old or something.
There are also a few RBE settings tweaked to be a bit
more modern, as well the new shorthands for specifying
target platforms (e.g. for_linux_x64).
- bazel/buildrc where we have to turn off the platforms
logic for emscripten https://github.com/emscripten-core/emsdk/issues/984
- bazel/rbe/BUILD.bazel for a fix in the platform description
that makes it work on Mac.
- Notice that _m1 has been removed from the mac-related toolchain
files because the same toolchain should work on both
architectures.
- All other changes in any order.
[1] https://bazel.build/docs/toolchains#debugging-toolchains
[2] https://bazel.build/docs/toolchains#toolchain-resolution
[3] https://bazel.build/reference/command-line-reference
[4] https://bazel.build/docs/toolchains#registering-building-toolchains
[5] 17dc3f16fc/gn/skia/BUILD.gn (L258-L271)
Change-Id: I515c114099d659639a808f74e47d489a68b7af62
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549737
Reviewed-by: Erik Rose <erikrose@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Without this define, I was seeing a compile error in SkKeyHelpers.cpp
Change-Id: I4d4a804f11fdd67e69bac4c7a6d0d686bebd8922
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/552076
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This reverts commit d78d52a86f.
That reason this CL was reverted in the first place is that it broke the Android roller - perfetto.h could not be located.
We now disable the use of perfetto for Android in the gn_to_bp.py script, so the roller should be good to go.
Reason for revert: Relanding
Original change's description:
> Revert "Add Perfetto library (gn & bazel) and bare-bones SkPerfTrace class"
>
> This reverts commit bfc9b94840.
>
> Reason for revert: new dependency causing Android build error
>
> Original change's description:
> > Add Perfetto library (gn & bazel) and bare-bones SkPerfTrace class
> >
> > First incremental step to incorporating Perfetto tracing into Skia, more CLs to follow
> >
> > NOTE: The presubmit check is failing. This appears to be due to the known issue where the presubmit check bugs out if the DEPS file is edited, which it was on this CL (modified to include Perfetto).
> >
> > Bug: skia:13303
> > Change-Id: I908be0392b520e8da14b34588b842bf6d955bd93
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543081
> > Reviewed-by: Greg Daniel <egdaniel@google.com>
> > Commit-Queue: Nicolette Prevost <nicolettep@google.com>
>
> Bug: skia:13303
> Change-Id: Ia2b883bbab1f8fb4f3914b63104a39240cc60e86
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544239
> Reviewed-by: Tyler Denniston <tdenniston@google.com>
> Auto-Submit: Tyler Denniston <tdenniston@google.com>
> Reviewed-by: Nicolette Prevost <nicolettep@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Tyler Denniston <tdenniston@google.com>
Bug: skia:13303
Change-Id: Ibd369b9c8c0e69fc9615fc05cf588ee4440c8ed5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544244
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Nicolette Prevost <nicolettep@google.com>
We decided to be ok with an empty version because caching
shader compilations is not that much overhead compared to
the rest of the backends that just have to redo them.
Change-Id: I995b3c75a7e723a1188184f8753450634fa0b629
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548356
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
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>
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>
Needed to fix the Dawn and Harfbuzz Bazel rules.
Change-Id: I21f63c970bdc972b97e42ef85d82d7f478bd45e2
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545721
Reviewed-by: Eric Boren <borenet@google.com>
Organization v3.5, if we are keeping track :)
This splits the "srcs" filegroup into "srcs" and "private_hdrs",
and renames "hdrs" to "public_hdrs".
To assist with the split, I created the macro split_srcs_and_hdrs.
Rather than keep two separate lists of header and source files,
I figured it would be easiest, at least for the common case,
to keep one list of files and then have a for loop split them
apart. I've tried to be consistent with having the list
of files be named with a _FILES suffix - maybe we can use this
as a marker to generate .gni files in the future?
Suggested review order:
- //bazel/macros.bzl. Note this needs a corresponding
G3 change (http://cl/452279799) as well. The exports_files_legacy
change is the better approach to something I manually
handled yesterday when fixing the G3 roll.
- //BUILD.bazel to see the new target skia_internal and
the previous skia_core renamed to skia_public.
- //src/core/BUILD.bazel to see a typical usage of
split_srcs_and_hdrs.
- //include/... to see the change to public_hdrs and
private_hdrs
- //src/... to see many more usages of split_srcs_and_hdrs
- //tools/... to see changes to skia_internal where
appropriate.
- Everything else. Note that //modules/... might also need
to be built with skia_internal instead of skia_public,
but we can fix that up later, if necessary.
Change-Id: Ie1cc969455d97b029b2d77faa222c4a9bad70671
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545716
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
There are no functional changes to the rules, just a relocation.
Change-Id: I90af5ec792fc54ce2978b0bbb1afd2c932e183b4
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545719
Reviewed-by: Ben Wagner <bungeman@google.com>
I had to copy some config_settings out of //bazel/common_config_settings
because these are now treated as separate entities and cannot
see that file.
For libpng, note that we use a genrule to create the
pnglibconf.h instead of pointing to one somewhere else.
This ended up being easier than other things I tried.
Another approach would be to not depend on the version
in third_party/externals, but to clone it via
new_git_repository [1] and apply a patch that creates
the configuration file.
[1] https://bazel.build/rules/lib/repo/git#new_git_repository
Bug: skia:12541
Change-Id: I9a284775dc0f2bdabb145518d5f0803c74fb99fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545368
Reviewed-by: Ben Wagner <bungeman@google.com>
This may look like a lot, but //modules/canvaskit/BUILD.bazel
is nearly identical to how it was with gazelle:
162dfca340/modules/canvaskit/BUILD.bazel
I removed the "wasm_gm_tests" targets from it, because they
had bitrotted slightly and fixing them is its own task.
CanvasKit depends on Skottie and Particles, which depend on
the SkParagraph, SkShaper, SkUnicode, and SkResources modules.
I've structured the BUILD.bazel files in the //modules directory
in a similar fashion as the "hierarchical filegroup"
introduced in https://skia-review.googlesource.com/c/skia/+/543977
Suggested Review Order
- //modules/skottie/...
- //modules/skparagraph/...
- all other modules.
- Note that modules/canvaskit/go/gold_test_env/BUILD.bazel is
generated from gazelle, because we like how gazelle handles
golang files and deps.
- All other files in any order.
Change-Id: I0aa9e6f81dba2c00f15cae7b19fe49a2027dcf1d
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544676
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
This adds or fixes rules to build three binaries that previously
had Bazel support.
How we build skslc with Bazel differs from GN in a large way:
GN has a small set of C++ files [1] it compiles in, but with
Bazel that was too hard/clumsy to pipe through (and get the
headers to work well). So, I just had skslc depend on
//:skia_core for simplicity. skslc did need to include something
in //src, so I made a special filegroup for it. For more
complex executables that need more headers from //src, we
should probably make a "src_hdrs" filegroup or something
to expose those. That or a skia_for_tests cc_library that
has all headers as "public".
[1] https://github.com/google/skia/blob/main/gn/sksl.gni#L235
Change-Id: Ie1382e982228059369886f4bfef4947f686b11b5
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544637
Reviewed-by: Ben Wagner <bungeman@google.com>
This adds targets which test our Dawn, GL, and Vulkan backends.
It follows the hierarchical filegroup pattern, as
outlined in https://skia-review.googlesource.com/c/skia/+/543977
Suggested Review order:
- tools/sk_app/BUILD.bazel. For many things in tools, I anticipate
they will depend on //:skia_core and other //tools targets.
sk_app shows this off, as well how to make the target
specific to a given platform and pull in the proper native code.
I'm trying out setting test_only = True, to see if we can
partition Skia's tests and helpers from the actual Skia library.
- other changes to //tools/, especially looking at sk_app's
dependencies.
- //example/BUILD.bazel. This uses the cc_binary_with_flags which
existed previously [1] to make it so people don't have to
specify all the flags for a given binary and can build it as is.
These targets nows how up in //bazel/Makefile
- //include/... and //src/..., where some typos from previous
CLs were fixed and rules expanded.
- Misc changes to .cpp files to remove unnecessary includes
that were assuming the GL backend was being compiled in.
- All other changes
[1] 162dfca340/bazel/cc_binary_with_flags.bzl
Change-Id: Ieacec464d44368cad0da0890c7dc85a6c0b900c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544317
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
The primary goal of this organization structure is to keep
our top level BUILD.bazel file short, with as little logic
as feasible. The logic required to control which files to
include, which third_party deps are needed, what system libraries
should be linked again, etc, should be in the BUILD.bazel
file best should be as close to the affected files as feasible.
In essence, we use filegroup() rules to bubble up the files
needed to build Skia (all as one big cc_library call) and
cc_library rules to bubble up the other components needed to build.
For example, //src/ports/SkFontHost_FreeType.cpp needs FreeType,
but only if we are compiling Skia with that type of font
support. With the new organization structure in this CL,
//src/ports/BUILD.bazel should have the logic that determines
if the cpp file should be included in the build of Skia and
if it is, that the Skia build should depend on //third_party:freetype2
Another example is //src/gpu/ganesh/BUILD.bazel, which
chooses which of the dawn, gl, vulkan, etc backend sources,
and the associated dependencies to include in the build.
It does not specify what those are, but delegates to the
BUILD.bazel files in the subdirectories housing the
backend-specific code.
The structure guidelines for BUILD.bazel files are as follows:
- Have a filegroup() called "hdrs" (for public headers) or
"srcs" (for private headers and all .cpp files) that is
visible to the parent directory. This should list the
files from the containing directory to include in the
build.
See //include/core/BUILD.bazel and //src/effects/BUILD.bazel
as examples.
- filegroup() rules can list a child directory's "hdrs"
or "srcs" in their "srcs" attributes, but should not contain
select statements pertaining to child directory files.
See //include/gpu/BUILD.bazel and //src/gpu/ganesh/BUILD.bazel
as examples.
- May have a cc_library() called "deps". This can specify
dependencies, cc_opts, and linkopts, but not srcs or hdrs. [1]
See //src/codec/BUILD.bazel as an example. These should
be visible to the parent directory.
- "hdrs", "srcs", and "deps" for the primary Skia build
(currently called "skia_core") should bubble up through
//include/BUILD.bazel and //src/BUILD.bazel, one directory
at a time.
This CL demonstrates a very basic build of Skia with many features
turned off (CPU only, no fonts, no codecs). Follow-on CLs will
add to these rules as more targets are supported. See bazel/Makefile
for the builds that work with just this CL.
Suggested Review Order:
- //BUILD.bazel to see the very small skia_core rule which
delegates all the logic down stack. Note that it has a
dependency on //bazel:defines_from_flags which will set
all the defines listed there when compiling all the
.cpp and .h files in skia_core *and* anything that depends
on skia_core, but *not* //src:deps.
- //include/BUILD.bazel and other BUILD.bazel files in the
subdirectories of that folder. Note that the filegroups in
//include/private/... are called "srcs" to be similar to
how Bazel wants "private headers" to be in the "srcs" of
cc_library, cc_binary, etc. and only public headers are
to be in "hdrs" [2].
- //src/BUILD.bazel and other BUILD.bazel files in the
subdirectories of that folder. //src/gpu/ganesh/...
will be filled in for dawn, vulkan, and GL in the next CL.
- //PRESUBMIT.py, which adds a check that runs buildifier [3]
on modified BUILD.bazel files to make sure they stay
consistently formatted.
- //bazel/... to see the new option I added to make sksl
opt-in or opt-out, so one could build Skia with sksl,
but not with a gpu backend.
- Misc .h and .cpp files, whose includes were removed if
unnecessary or #ifdef'd out to make the minimal build
work without GPU or SkSL includes.
- //bazel/Makefile to see the builds that work with this CL.
[1] Setting srcs or hdrs is error-prone at best, because those
files will be compiled with a different set of defines than
the rest of skia_core, because they wouldn't depend on
//bazel:defines_from_flags.
[2] https://bazel.build/reference/be/c-cpp#cc_library.hdrs
[3] https://github.com/bazelbuild/buildtools/releases
Change-Id: I5e0e3ae01ad42d672506d5aad1239f2512188191
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543977
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
gazelle ended up being more liability than asset for our C++ rules.
It required devs to manually run the command frequently (and was
easy to forget until the CQ failed). The fact that we still had to
edit the source files (e.g. the "srcs" cc_libraries) meant that
the mixture between generated and hand-written caused some
tension (see include/third_party/vulkan for a good example).
The combination of gazelle and our IWYU enforcement added several
bits of churn without any real benefit. The generated rules
also didn't help identify cases where we were not keeping tight
boundaries (e.g. non-gpu code and gpu code).
Identifying third_party deps automatically ended up being trickier
than anticipated (see the deleted //third_party/file_map_for_bazel.json)
Using the "maximum set of dependencies" worked ok, but ended up
increasing build time unnecessarily. For example, compiling
CanvasKit for WebGL always needed to compile Dawn because
SkSLCompiler.cpp sometimes needs to include tint/tint.h.
Follow-up CLs will rebuild the BUILD.bazel rules without gazelle.
Note to Reviewers:
- The only file worth manually reviewing here is bazel/Makefile.
Change-Id: I36d6fc3747487fabaf699690780c95f1f6765770
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543976
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
The `make -C bazel generate` command doesn't function properly on
Windows builders because Windows uses backslashes as the path separator.
Fortunately, adding quotes around the path is enough to concince
Windows to run the executable, allowing the `generate` target to
complete successfully, and doing so doesn't impact Linux builders.
Bug: skia:13366
Change-Id: I50c461635b70cc59cd6e79bf17a6945c80df1986
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544756
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This reverts commit bfc9b94840.
Reason for revert: new dependency causing Android build error
Original change's description:
> Add Perfetto library (gn & bazel) and bare-bones SkPerfTrace class
>
> First incremental step to incorporating Perfetto tracing into Skia, more CLs to follow
>
> NOTE: The presubmit check is failing. This appears to be due to the known issue where the presubmit check bugs out if the DEPS file is edited, which it was on this CL (modified to include Perfetto).
>
> Bug: skia:13303
> Change-Id: I908be0392b520e8da14b34588b842bf6d955bd93
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543081
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Nicolette Prevost <nicolettep@google.com>
Bug: skia:13303
Change-Id: Ia2b883bbab1f8fb4f3914b63104a39240cc60e86
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544239
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Auto-Submit: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Nicolette Prevost <nicolettep@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
First incremental step to incorporating Perfetto tracing into Skia, more CLs to follow
NOTE: The presubmit check is failing. This appears to be due to the known issue where the presubmit check bugs out if the DEPS file is edited, which it was on this CL (modified to include Perfetto).
Bug: skia:13303
Change-Id: I908be0392b520e8da14b34588b842bf6d955bd93
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543081
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Nicolette Prevost <nicolettep@google.com>
Change-Id: Ib4f2ae63ceee424eb93d098c7cd65e6ea8ac92f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543056
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
- Introduced the SK_ENABLE_WGSL_VALIDATION macro which is currently only
enabled when skslc gets compiled when using the `skia_compile_sksl_tests`
setting.
- SkSLCompiler::toWGSL now validates its output using Tint's WGSL reader
structures based on conditionally compiled code depending on the
SK_ENABLE_WGSL_VALIDATION flag.
- Fixed `warning: use of deprecated language feature: struct members should be separated with commas"
warnings that were generated for HelloWorld.wgsl.
Bug: skia:13092
Change-Id: Ib894457030004966221faf82f61360e390b95e22
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537802
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 5c6bf4f692.
Reason for revert: git diff does not work well with deleted files
Original change's description:
> [bazel] Run gazelle only on the files that changed
>
> With this change, make generate takes 1.8 seconds instead of
> 7.9 seconds.
>
> We still have generate_force to run on everything.
>
> Change-Id: I6d57031adbe38a7f25a59570baea89970eea024f
> Bug: skia:12541
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540740
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Bug: skia:12541
Change-Id: I47c23adf09bbc6324817e166f7ab33eb16f4bf61
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540743
Reviewed-by: John Stiles <johnstiles@google.com>
With this change, make generate takes 1.8 seconds instead of
7.9 seconds.
We still have generate_force to run on everything.
Change-Id: I6d57031adbe38a7f25a59570baea89970eea024f
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540740
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Rather than having a monolithic third_party/BUILD.bazel, this moves
our Dawn rules to their own subdirectory and makes it callable via
@dawn instead of //third_party/dawn.
This will help with the G3 roll and make our rules more organized in
general.
This also rolls Dawn
Roll Dawn from ab9757036bd6 to e831fb61046b (22 revisions)
https://dawn.googlesource.com/dawn.git/+log/ab9757036bd6..e831fb61046b
Suggested Review Order:
- WORKSPACE.bazel, where we define @dawn and its deps
(@vulkan_headers and @vulkan_tools). I initially thought
I needed to define all of Dawn's deps in the workspace_file_content
for new_local_repository, but that WORKSPACE file is
ignored when building Skia rules.
- third_party/dawn/BUILD.bazel, the contents of which were copied
from //third_party/BUILD.bazel and modified largely via
find-and-replace to point to files relative to
//third_party/externals/dawn. One exception is the cpu_wasm
config_setting because @dawn isn't able to see Skia's
//bazel/macros.bzl.
- All other files
Change-Id: Ib2d7bc972ef00b6b68370ce5c2839ffb70ed9a2f
Bug: skia:12541, skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538638
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
For additional context, see "Codifying Certain Build Options"
and "Building on the CI" in the design doc go/skia-bazel
Suggested review order:
- builder_name_schema.json to see the three required and
one optional part of BazelBuild jobs.
- jobs.json to see one new BazelBuild job added. In an
ideal world, this job would have been named
BazelBuild-//modules/canvaskit:canvaskit_wasm-debug-linux_x64
but Buildbucket (?) requires jobs match the regex
^[a-zA-Z0-9\\-_.\\(\\) ]{1,128}$
so we use spaces instead of slashes or colons.
- gen_tasks_logic.go; noting the makeBazelLabel function
expands most of the spaces to / and the last one to a
colon to make a single-target label. If there are three
dots, then it is a multi-target label, and we do not
need to add a colon.
- bazel_build.go; This is a very simple task driver, and
I do not anticipate getting too much more complex.
The place where we decide which args to augment
a build with depend on the host platform and thus
should be set in gen_tasks_logic.go.
- bazel/buildrc to see some initial configurations set,
one of which, "debug", is used by the new job.
The "release" version of CanvasKit probably works on
3.1.10 which had a bugfix, but we are still on
3.1.9
- .bazelrc to see a rename of the linux-rbe config to
linux_rbe (our configs should have no dashes if
we want to specify them verbatim in our Job names).
It also imports the Skia-specified build configs
from //bazel/buildrc and supports the user-specified
//bazel/user/buildrc file if it exists.
- All other files in any order.
Change-Id: Ib954dd6045100eadcbbf4ffee0888f6fbce65fa7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537797
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Bug: skia:13040
Change-Id: I1749f21162ea400a8b8fb00ed52e6024eb658d52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537082
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
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>
To make it easier to use the G3 version of this dep,
we make a reference to it in the WORKSPACE.bazel, so
we can refer to this external dep with the @ notation.
I would like to do this for all of our third party
deps, but one at a time.
Change-Id: I03e0beca124225e0faf1232278dae641da8a3e4d
Bug: skia:13240, skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535358
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
