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>
It's been broken for quite some time.
Change-Id: Ic75f75e2062e9c1f624aa59c2d8de8909a208ea2
Bug: skia:12830
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550256
Reviewed-by: Eric Boren <borenet@google.com>
I saw some strange errors in the logs and locally after
updating to Bazel 5.2.0:
Compiling third_party/externals/freetype/src/base/ftwinfnt.c failed: missing input file 'external/emscripten_npm_linux/node_modules/@caporal/core/CHANGELOG.md', owner: '@emscripten_npm_linux//:node_modules/@caporal/core/CHANGELOG.md'
When I did a bazel clean --expunge locally, it went away.
Looking closely at the Bazel cache folder, some npm dependencies
starting with an @ sign were missing from caches for the 5.0.0
but there in 5.2.0. I could not find an entry in the changelog,
but it's simple enough to clear our caches for now.
Once we update emsdk to a new version, we shouldn't need this
as the dependencies should be downloaded fresh.
Change-Id: Id21d4e4806792d2eee48e1c5e1d6a106230be17a
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549856
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
codesize.skia.org was missing data and a misunderstanding of
file names was one cause.
To be consistent with Gold (and Perf?) we should use UTC as the
time zone for the folders and add hours. The server has been
switched to UTC in https://skia-review.googlesource.com/c/buildbot/+/548262/
This also fixes a race condition caused by upload the .tsv files
before the .json files. When the server sees a Pub/Sub event
for a .tsv file, it indexs the corresponding .json file.
Frequently, the .json file wouldn't have been uploaded yet,
so the indexing would fail.
Change-Id: Iabe64786db6e5c6020a3fc5dda244ccbe478c401
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548357
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
This doesn't work on Nvidia's desktop Linux drivers, probably due to the
same root cause as skia:13035. (Reported the bug to Khronos at
https://gitlab.khronos.org/Tracker/vk-gl-cts/-/issues/3748)
Change-Id: Iafe73df72e175f5ccb6f4663e5b102fe7efaf57c
Bug: skia:13395
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547443
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 3ba37e1d38.
Reason for revert: disabled test on Win+Intel/Android+Adreno6xx GPUs
(skia:13393)
Broke out non-folding tests to a separate test due to failures on
Quadro/RTX3060. (skia:13395)
Original change's description:
> Revert "Add unit test covering struct field folding."
>
> This reverts commit 0cbba91940.
>
> Reason for revert: returns red on Adreno and Intel
>
> Original change's description:
> > Add unit test covering struct field folding.
> >
> > At present, we don't try to optimize away `myStruct.myField` accesses
> > even when `myStruct` is known or constant, so the output from the test
> > is not too impressive.
> >
> > Change-Id: I563559e5cdc6c2669d69ec78ad8ca09d3be02a68
> > Bug: skia:13387
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547276
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> Bug: skia:13387
> Change-Id: I2e651ddb82fac08cdc16fa8b77696cdd314e805f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547438
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Change-Id: Ic86ff6b1034363d1343793f94e3ba707adb2fcc3
Bug: skia:13387, skia:13393, skia:13395
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547439
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This removes the previous task driver dedicated to enforcing
IWYU and adds individual jobs for the binaries we want to
test. This aligns with the current plan of organizing our
Bazel CI jobs - going wide where possible.
Change-Id: I8b163957e34e594c1a80d7b9e6b63c64a277bef1
Bug: skia:12541 skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547019
Reviewed-by: Eric Boren <borenet@google.com>
We need to make sure we have the required chroma sampling options for the
format.
Bug: skia:12820, skia:13265
Change-Id: I6e0d0f9b50a5730ecbe5de2626bf1d1ee1051d7a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546861
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This version dodges skia:13380 by reordering the list of uniforms.
Disabled on Win10 with older Intel GPUs due to crashes when performing
vector/matrix casts in GLSL.
Change-Id: If053908fd1a6921257e5af0abdce1ff69a03297f
Bug: skia:13378, skia:12179
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546551
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The job had been removed from jobs.json previously. We no
longer intend to generate Bazel files using gazelle, which
is what this checked.
Change-Id: I1bcf98deae3831a18b9a70e4f0a295eb3f2785b8
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546137
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
With a hot Bazel cache, https://task-scheduler.skia.org/job/4XhwJfG1wR38Hp3dF1pI
took 59 seconds and did not have to wait 1-2 minutes
for TaskScheduler to schedule a prerequisite task
and Swarming to deduplicate said task, which is
the best case scenario for "all hot caches".
Change-Id: Ic86599e0b886ecded836641cceb19b623065a91e
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546136
Reviewed-by: Eric Boren <borenet@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>
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>
This change introduces the use of "Android12" as an OS, so
many checks had to be changed from b.os("Android") to
b.matchOs("Android") which treats "Android" and "Android12"
the same.
Bug: skia:12912
Change-Id: Ib0a5f9d8f1a06b62e529a1567efcde52c7bb44d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527496
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Appeasing IWYU and the Gazelle-generated BUILD.bazel files is currently
annoying for devs working on non-linux environments.
I have been experimenting with removing the gazelle rules (but still
keeping our BUILD.bazel rules organized), which would alleviate
some of the burden here. Until that is finished, we can stop one
of the two jobs.
Change-Id: Id6c43bd98c6ce41e1ae9a6bfc1102ff47a6e5b1f
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543496
Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I6db702c2d231bfe527b3fd4bf4f99beb6e58bf8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542641
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Fix for this landed in ANGLE.
Bug: skia:13290
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac10.15.1-Clang-MacBookAir7.2-GPU-IntelHD6000-x86_64-Debug-All-ANGLE
Change-Id: Id14366c5f9fe33bf1347a9bdaad12ec38b93fa6b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542640
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
subprocess.check_output needs encoding or it defaults
to bytes.
Python 3 has / be float division and // be integer
division.
The skpbench fix is speculative.
Change-Id: I13d0e976c03bba30761c8ab533e6fdd041b795c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541696
Reviewed-by: Ravi Mistry <rmistry@google.com>
This is similar to https://skia-review.googlesource.com/c/buildbot/+/538218
BIG CHANGE: If we fail to download uninteresting hashes
(because Gold is down or the script otherwise fails), we
now crash/fail our Test-* tasks. In an early version of
this removal, that lack of failure masked an issue with
the script (urllib2 is not in Python3) and nearly would
have landed. Failing loudly is better, IMO.
This removes the symbolize_stack_trace script, which
stopped working with Python3 for reasons unknown.
If we need the behavior, we can rewrite it.
Relatedly, we removed 4 jobs, the Docker ones, because
we will not get much value out of them as we migrate
towards Bazel and removing the symbolization script
was tricky to get right.
There are a few cleanups around copypasta that I noticed
when combing through the recipes.
Change-Id: I8dfab416e964fd494267800b4ebe216061895f19
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538636
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
The version in CIPD is 4.46, which has the following
issue when run in Python 3:
https://github.com/GoogleCloudPlatform/gsutil/issues/961
This asset uses the latest version, 5.10, which *only*
supports Python 3.
Our gsutil recipe module had some "Windows-specific"
behavior that was actually running on Linux and needed
to be changed because it was using Python2 to run
gsutil.
Change-Id: I6884b9b6fcc600351a8e608dc9e3c0114907a586
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541220
Reviewed-by: Ravi Mistry <rmistry@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Kevin Lubick <kjlubick@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>
Looks like really old drivers struggled with weird no-op blocks.
Change-Id: Ie32754a9c221eb7c20924ee27e5facca7e9701f0
Bug: skia:13309
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539561
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This simplifies the bot script slightly. More importantly, it means that
local developers don't need to remember to do this when building ANGLE
on Mac.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac12-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-ANGLE
Change-Id: Ia60cd07f15e3b447b58cfa1198ea26f68f72384b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539036
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: 1320964
Change-Id: I5f05ea469b824b1382f64c1eb3fb9be0498cf329
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537517
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Robert Phillips <robertphillips@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>
This CL could, arguably, confuse the issue wrt why these images are
being generated but making the sizes match will make the images easier
to triage.
Bug: skia:13278
Change-Id: Ifb0b82eaaa4351fb589118d3b28673da9dd32e20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537576
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: I1ca07f97757f70debc0589648a57ba10b1729636
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537081
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Bug: skia:13040
Bug: skia:9235
Bug: skia:10412
Bug: skia:12437
Change-Id: Ia96f8332b5372ecf65cb20ffb9549bc0dc8f931b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537080
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
There are two SKP versions we deal with here:
* The min and current supported versions in src/core/SkPicturePriv.h.
* The SKP asset version that is incremented weekly in infra/bots/assets/skp/VERSION.
We will need to update the asset version used by the bot when the min version in src/core/SkPicturePriv.h is updated.
Instructions on how to do that have been documented in src/core/SkPicturePriv.h.
As noted in https://bugs.chromium.org/p/skia/issues/detail?id=13278#c2 DM currently does not fail when it fails to parse an SKP. Till DM is updated to fail, developers will have to look for blank images in Gold via this new bot to determine when we are failing to support the oldest SKP version.
Bug: skia:13278
Change-Id: I8fff62cc289c3bd6abf5179bcee349baf0a8188a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/536106
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Added Test-Ubuntu18-Clang-Golo-GPU-QuadroP400-x86_64-Debug-All-Dawn
to exercise the Dawn Vulkan backend.
Change-Id: I3679822a1460eda36654f99c70ba9510e394b40c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/536736
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
