go_repositories.bzl is generated by Gazelle, which does not add
module/function docstrings. Skipping the buildifier check on this
file to prevent presubmit failure.
Bug: none
Change-Id: I242319f25f2ba686efc2b0ade42852bbe28aabe3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/558925
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Chris Mumford <cmumford@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>
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>
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>
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>
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>
Calls to "python" won't work on vanilla MacOS installs (nor many
distributions of Linux), and AFAICT we're assuming it refers to python3
anyways.
xreadlines was removed in Python 3, and the suggested equivalent is to
use the file directly as an iterator:
https://docs.python.org/2.7/library/stdtypes.html#file.xreadlines
Change-Id: Iae51e3923e38ea63f857a17b9ff80e79871efee5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537982
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: James Godfrey-Kittle <jamesgk@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>
This is a reland of commit 094bcdb9e5
Original change's description:
> [infra] Use Python3 for our Presubmits
>
> https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_support.py;l=319;drc=443d9135cc33f3156d5fe25ebec33f9adffbab65
>
> This also makes any errors from `make train -C infra/bots`
> look well formatted because check_output returns a bytestring
> in Python3 and when that is printed, the newlines et al are not
> rendered correctly. Thus we want the output of check_output
> to be encoded to UTF-8.
>
> Without setting the USE_PYTHON3 = True in PRESUBMIT.py,
> it appears that `git cl upload` would try to run our
> infra_tests.py in Python2 mode, which does not have
> the encoding argument for check_output.
>
> Apparently cipd_bin_packages/vpython3 does not have the
> "six" package installed (but cipd_bin_packages/vpython does)
> so I replaced the six.StringIO with io.StringIO, which
> is where that is located in Python3.
>
> Change-Id: Ic8f61bf943531583ba3d110a85260d69bdbf5eb2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537677
> Reviewed-by: Ravi Mistry <rmistry@google.com>
> Reviewed-by: Eric Boren <borenet@google.com>
Change-Id: Ia7fb2f3b6d8b70ca95cf10763782d4a0122053e0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537978
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This reverts commit 094bcdb9e5.
Reason for revert: Breaking Housekeeper-PerCommit-InfraTests_Linux. The bot will have to be updated as part of this change.
Original change's description:
> [infra] Use Python3 for our Presubmits
>
> https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_support.py;l=319;drc=443d9135cc33f3156d5fe25ebec33f9adffbab65
>
> This also makes any errors from `make train -C infra/bots`
> look well formatted because check_output returns a bytestring
> in Python3 and when that is printed, the newlines et al are not
> rendered correctly. Thus we want the output of check_output
> to be encoded to UTF-8.
>
> Without setting the USE_PYTHON3 = True in PRESUBMIT.py,
> it appears that `git cl upload` would try to run our
> infra_tests.py in Python2 mode, which does not have
> the encoding argument for check_output.
>
> Apparently cipd_bin_packages/vpython3 does not have the
> "six" package installed (but cipd_bin_packages/vpython does)
> so I replaced the six.StringIO with io.StringIO, which
> is where that is located in Python3.
>
> Change-Id: Ic8f61bf943531583ba3d110a85260d69bdbf5eb2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537677
> Reviewed-by: Ravi Mistry <rmistry@google.com>
> Reviewed-by: Eric Boren <borenet@google.com>
Change-Id: I0becb25d155ce0b0281c25d83662f1b01aee8033
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537777
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Ravi Mistry <rmistry@google.com>
https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_support.py;l=319;drc=443d9135cc33f3156d5fe25ebec33f9adffbab65
This also makes any errors from `make train -C infra/bots`
look well formatted because check_output returns a bytestring
in Python3 and when that is printed, the newlines et al are not
rendered correctly. Thus we want the output of check_output
to be encoded to UTF-8.
Without setting the USE_PYTHON3 = True in PRESUBMIT.py,
it appears that `git cl upload` would try to run our
infra_tests.py in Python2 mode, which does not have
the encoding argument for check_output.
Apparently cipd_bin_packages/vpython3 does not have the
"six" package installed (but cipd_bin_packages/vpython does)
so I replaced the six.StringIO with io.StringIO, which
is where that is located in Python3.
Change-Id: Ic8f61bf943531583ba3d110a85260d69bdbf5eb2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537677
Reviewed-by: Ravi Mistry <rmistry@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Change-Id: Ia1d43e4814eb23fc111df0c1800425272337936c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/536639
Reviewed-by: Ravi Mistry <rmistry@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
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>
This is now done in SkCQ
Bug: skia:12487
Change-Id: I5a1414b7594ef371727c3391b18dbe6948b6c25a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454457
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
This is now enforced using OWNERS
Change-Id: Iee0c639731376d7bc4c3deaa65ccadef3fae53f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445642
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This work is being done by the skiastatus Gerrit plugin.
Bug: skia:12403
Change-Id: I42b81dcc9e2b4d5c99f9756eabed186f489a9adb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445105
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
This reverts commit 42d753031d.
Reason for revert: Replaced by the work in skbug.com/11824
Original change's description:
> Display a Docs-Preview link for each modified MD file
>
> Bug: skia:11824
> Change-Id: I6bd557affca5ccf9f2936d86e2b0da168ceb2670
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/390577
> Commit-Queue: Ravi Mistry <rmistry@google.com>
> Reviewed-by: Joe Gregorio <jcgregorio@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:11824
Change-Id: Ia874ed6474fcae4698cd676dbdf208dbdefdedc4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/399716
Reviewed-by: Ravi Mistry <rmistry@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
This reverts commit 4ac25118b5.
Reason for revert: missing comma :|
Original change's description:
> make brianosman a public API owner
>
> ... and sort, and remove dead emails
>
> Change-Id: I0ef35918a5b4539559319c32b95109ca7c66f87c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365519
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=djsollen@google.com,mtklein@google.com,bsalomon@google.com,hcm@google.com,brianosman@google.com,reed@google.com
Change-Id: Ie3ac51c82c83e8b76fc3933f4f68938f608e32d5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365507
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
... and sort, and remove dead emails
Change-Id: I0ef35918a5b4539559319c32b95109ca7c66f87c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365519
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Errors will be reported like this:
"""""""""
Running presubmit upload checks ...
** Presubmit ERRORS **
Git conflict markers found in whitespace.txt:14 <<<<<<< HEAD
Git conflict markers found in whitespace.txt:16 =======
Git conflict markers found in whitespace.txt:18 >>>>>>> 75a85179504e59df7b3abbc09113f24f2233001b
"""""""""
Bug: skia:11098
Change-Id: I24f90ec545b797f1d782a1733b8273ce176b1012
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345037
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
This reverts commit 540b51ce8f.
Reason for revert: RPI disk space
Original change's description:
> Reland "[infra] Switch from isolate to RBE-CAS"
>
> Bug: skia:10883
> Change-Id: Iec82a07fdf3c0807a9bb1870309eded85d4f0b1e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341679
> Commit-Queue: Eric Boren <borenet@google.com>
> Reviewed-by: Weston Tracey <westont@google.com>
TBR=borenet@google.com,westont@google.com
Change-Id: I0156842b995d275d8deb5bd73c6877de7f8af72e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10883
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342596
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
Golden SkSL outputs are intended to eventually replace the majority of
our unit tests, since they can automatically update themselves when we
change implementation details of the compiler.
If you change the compiler output without updating the Golden files, the
CheckGeneratedFiles housekeeper will be triggered. Set
`skia_compile_processors` or `skia_compile_sksl_tests` to true in your
GN args to regenerate them.
Almost all of the tests from SkSLFPTests.cpp and SkSLGLSLTests.cpp can
be migrated into separate unit-test .fp/.sksl files in a followup CL.
hcm@ has signed off on removing the copyright boilerplate preamble from
our unit test files.
Change-Id: I9e24a944bbac8f8efd62c92481b022a0b1ecdd0b
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316336
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I78eff440486eb2e76acdbd8a43c6c99cf6ed9bbb
Bug: chromium:1064305
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/282276
Commit-Queue: Eric Boren <borenet@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
The CheckGeneratedFiles bot only required rewriting
.gn files, while the presubmit wants both .gn and .gni files.
It also appears that the #includes rewrite script runs on
both the presubmit and CheckGeneratedFiles bots.
These presubmits run on the CQ before landing right?
If so, no need for them in the CheckGeneratedFIles bot at all.
And of course, format .gni files.
Change-Id: Icd4526d62f85088862ad93566cc9ace11dc3e33f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281505
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
* Mainly updates to documentation.
* Fix some comments in PRESUBMIT.py.
* Delete tools/lua/trigger_ct_lua. It has not worked in many years.
Bug: skia:9962
Change-Id: If6f58f173f2c8bd3fc9bdfc4db440f42489fee08
Docs-Preview: https://skia.org/?cl=274597
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/274597
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
I think this fixes issues with `git cl upload` on Windows machines.
Note that fetch-gn grabs gn.exe (not gn.bat).
Change-Id: Iba64ec62cce00e8ba0076ce83d47e4e6e6ef6197
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/271604
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I3f2f3713fc27207c26d02c29358cd582440e8e65
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/271037
Commit-Queue: Eric Boren <borenet@google.com>
Auto-Submit: Ben Wagner aka dogben <benjaminwagner@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Change-Id: I649f07e036f184e6dbb3a335bdd979dedea27ec8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/270876
Commit-Queue: Eric Boren <borenet@google.com>
Auto-Submit: Ben Wagner aka dogben <benjaminwagner@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
change.AddDescriptionFooters() doesn't exist, the right function is
change.AddDescriptionFooter()
Bug: 1042324
Change-Id: Icb417eab494dc217bc8bb9a51810f9cf65cce828
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268667
Auto-Submit: Edward Lemur <ehmaldonado@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Change-Id: I10f9083ebda6038026cbc322b07528a4da820634
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/266858
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The CQ handles tree closure,
and these {dm,nanobench}_flags.py files don't exist.
Change-Id: I19b5842763efd121a2bf7c3ff9075bf46089d74a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/264342
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This might be nice as an early reminder that you'll need special review,
and ought to let us not have to re-run this (~1m) PRESUBMIT bot on CQ+2.
Change-Id: I94c370d3cfcf7b593961fcaf3907e0a045ff1c89
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/264336
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
urllib_request was introduced in crrev.com/c/1866170 to add
python 3 compatibility.
Bug: 1009814
Change-Id: Iecc246d4f251983051bd0cf812903831ea7e0c2d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/249588
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
Also:
Revert "don't try to format includes on windows"
This reverts commit e68b5bd4e6.
Change-Id: I93dc4dba4808c07882e977ac0693c93e282bbad5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/230878
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
It's not going to work:
- windows\style\paths don't work well in Python
- something seems to be slipping in extra \r, probably print?
Change-Id: I6d9a0cc0bff403b9b45412b0e92290a9346bcf14
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/230641
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
