Skip to content

Commit

Permalink
Revert "[bazel] Housekeeper-PerCommit-CheckGeneratedFiles: Add "go ge…
Browse files Browse the repository at this point in the history
…nerate", Gazelle and Buildifier steps."

This reverts commit 0f6a4e6.

Reason for revert: Breaking external-client tests

Original change's description:
> [bazel] Housekeeper-PerCommit-CheckGeneratedFiles: Add "go generate", Gazelle and Buildifier steps.
>
> Rationale: In a follow-up CL I'll land some code that requires generating Bazel configs from a Go struct via a //go:generate comment.
>
> Regarding the changes in //bazel/exporter/interfaces/mocks/generate.go: For some reason, mockery runs from its Bazel runfiles directory by default. This does not happen in the Skia Infra repository, and I couldn't figure out why. In any case, I fixed it by forcing it to run from the current directory via a --run_under flag.
>
> Most of the changes in BUILD.bazel files are made by Buildifier, or hand-made to appease Buildifier.
>
> Bug: b/40045301
> Change-Id: I01e9a6f5ea0a915be6554e2f5e299725ab535e97
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/778517
> Reviewed-by: Kevin Lubick <[email protected]>
> Auto-Submit: Leandro Lovisolo <[email protected]>
> Commit-Queue: Leandro Lovisolo <[email protected]>

Bug: b/40045301
Change-Id: I95715d646c38809e3662552232219aebb9196cd1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/778718
Commit-Queue: Rubber Stamper <[email protected]>
Auto-Submit: Kevin Lubick <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
  • Loading branch information
kjlubick authored and SkCQ committed Nov 14, 2023
1 parent 0d9175b commit 5778a54
Show file tree
Hide file tree
Showing 13 changed files with 1,444 additions and 173 deletions.
22 changes: 0 additions & 22 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
load("@com_github_bazelbuild_buildtools//buildifier:def.bzl", "buildifier")
load("@skia_user_config//:copts.bzl", "DEFAULT_OBJC_COPTS")
load("//:defines.bzl", "DEFAULT_DEFINES", "DEFAULT_LOCAL_DEFINES")
load("//bazel:gen_compile_flags_txt_linux_amd64.bzl", "gen_compile_flags_txt_linux_amd64")
Expand Down Expand Up @@ -124,27 +123,6 @@ alias(
visibility = ["//visibility:public"],
)

# Sample usage: "bazel run //:buildifier".
buildifier(
name = "buildifier",
exclude_patterns = [
"./bazel/rbe/gce_linux/*",
"./modules/*",
"./node_modules/*",
"./**/node_modules/*",
"./third_party/externals/*",
],
lint_mode = "fix",
lint_warnings = [
# Prevents https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#native-android.
"-native-android",
# Prevents https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#native-cc.
"-native-cc",
# Prevents https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#native-py.
"-native-py",
],
)

test_suite(
name = "all_go_tests",
tests = [
Expand Down
19 changes: 2 additions & 17 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,15 +414,12 @@ def _CheckBuildifier(input_api, output_api):
BUILD.bazel or *.bzl files.
"""
files = []
# Please keep the below exclude patterns in sync with those in the //:buildifier rule definition.
for affected_file in input_api.AffectedFiles(include_deletes=False):
affected_file_path = affected_file.LocalPath()
if affected_file_path.endswith('BUILD.bazel') or affected_file_path.endswith('.bzl'):
if not affected_file_path.endswith('public.bzl') and \
not affected_file_path.endswith('go_repositories.bzl') and \
not "bazel/rbe/gce_linux/" in affected_file_path and \
not affected_file_path.startswith("third_party/externals/") and \
not "node_modules/" in affected_file_path: # Skip generated files.
not "bazel/rbe/gce_linux/" in affected_file_path: # Skip generated files.
files.append(affected_file_path)
if not files:
return []
Expand All @@ -439,19 +436,7 @@ def _CheckBuildifier(input_api, output_api):
# One can change --lint=warn to --lint=fix to have things automatically fixed where possible.
# However, --lint=fix will not cause a presubmit error if there are things that require
# manual intervention, so we leave --lint=warn on by default.
#
# Please keep the below arguments in sync with those in the //:buildifier rule definition.
output_api, [
'buildifier',
'--mode=fix',
'--lint=warn',
'--warnings',
','.join([
'-native-android',
'-native-cc',
'-native-py',
])
] + files)
output_api, ['buildifier', '--mode=fix', '--lint=warn'] + files)


def _CheckBannedAPIs(input_api, output_api):
Expand Down
4 changes: 2 additions & 2 deletions bazel/exporter/interfaces/mocks/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@

package mocks

//go:generate bazelisk run //infra:mockery "--run_under=cd $PWD && " -- --name QueryCommand --srcpkg=go.skia.org/skia/bazel/exporter/interfaces --output ${PWD}
//go:generate bazelisk run //infra:mockery "--run_under=cd $PWD && " -- --name FileSystem --srcpkg=go.skia.org/skia/bazel/exporter/interfaces --output ${PWD}
//go:generate bazelisk run //infra:mockery -- --name QueryCommand --srcpkg=go.skia.org/skia/bazel/exporter/interfaces --output ${PWD}
//go:generate bazelisk run //infra:mockery -- --name FileSystem --srcpkg=go.skia.org/skia/bazel/exporter/interfaces --output ${PWD}
2 changes: 2 additions & 0 deletions bazel/external/icu4x/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ load("@rules_rust//rust:defs.bzl", "rust_static_library")
load(
"//bazel:skia_rules.bzl",
"exports_files_legacy",
"select_multi",
"skia_cc_library",
)

exports_files_legacy()
Expand Down
3 changes: 0 additions & 3 deletions bazel/remove_indentation.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ def remove_indentation(string):
indentation += char
else:
break

# For some reason Buildifier thinks the below variable is uninitialized.
# buildifier: disable=uninitialized
return indentation

lines = string.split("\n")
Expand Down
23 changes: 9 additions & 14 deletions example/external_client/WORKSPACE.bazel
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

# A real client should download a pinned version of Skia such as:
#
# load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
#
# git_repository(
# name = "skia",
# commit = "8b051126be8ae6c3e718bd3817eebb867b2fd612",
# remote = "https://skia.googlesource.com/skia",
# )
#
# git_repository(
# name = "skia",
# commit = "8b051126be8ae6c3e718bd3817eebb867b2fd612",
# remote = "https://skia.googlesource.com/skia",
# )
# We use local_repository to allow us to test Skia at head as if it were checked
# out via git_repository.
local_repository(
Expand All @@ -31,12 +29,9 @@ local_repository(
# These two workspace functions will add dependencies for Skia's Bazel rules
# (e.g. @bazel_skylib) and the C++ dependencies (e.g. @libpng)
load("@skia//bazel:deps.bzl", "bazel_deps", "c_plus_plus_deps", "header_based_configs")

# Be sure to call the functions.
bazel_deps()

c_plus_plus_deps()

header_based_configs()

##############################################################################
Expand All @@ -47,7 +42,7 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
# https://github.com/bazelbuild/rules_cc
http_archive(
name = "rules_cc",
urls = ["https://github.com/bazelbuild/rules_cc/releases/download/0.0.8/rules_cc-0.0.8.tar.gz"],
sha256 = "ae46b722a8b8e9b62170f83bfb040cbf12adb732144e689985a66b26410a7d6f",
strip_prefix = "rules_cc-0.0.8",
urls = ["https://github.com/bazelbuild/rules_cc/releases/download/0.0.8/rules_cc-0.0.8.tar.gz"],
)
)
Loading

0 comments on commit 5778a54

Please sign in to comment.