Reland: [bazel] Housekeeper-PerCommit-CheckGeneratedFiles: Add "go ge…

…nerate", Gazelle and Buildifier steps. This CL relands https://skia-review.googlesource.com/c/skia/+/778517. It turns out that defining "buildifier" in //BUILD.bazel breaks some tasks, such as BazelTest-external_client-png_decoder-default-linux_x64. This CL fixes the breakage by moving its definition to //bazel/BUILD.bazel. Bug: b/40045301 Change-Id: Ib22071cb897de62c834962f68ca2f9646fbf4c75 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/778609 Auto-Submit: Leandro Lovisolo <[email protected]> Commit-Queue: Kevin Lubick <[email protected]> Reviewed-by: Kevin Lubick <[email protected]>
HistoryFork · Nov 15, 2023 · 42444c0 · 42444c0
1 parent d0e4a53
commit 42444c0
Show file tree

Hide file tree

Showing 14 changed files with 185 additions and 1,444 deletions.
diff --git a/BUILD.bazel b/BUILD.bazel
@@ -179,3 +179,14 @@ alias(
     actual = "@go_sdk//:bin/go",
     visibility = ["//visibility:public"],
 )
+
+##############
+# buildifier #
+##############
+
+# Sample usage: "bazel run //:buildifier".
+alias(
+    name = "buildifier",
+    actual = "//bazel:buildifier",
+    visibility = ["//visibility:public"],
+)
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
@@ -414,12 +414,15 @@ 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:  # Skip generated files.
+        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.
         files.append(affected_file_path)
   if not files:
     return []
@@ -436,7 +439,19 @@ 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.
-    output_api, ['buildifier', '--mode=fix', '--lint=warn'] + files)
+    #
+    # 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)
 
 
 def _CheckBannedAPIs(input_api, output_api):

diff --git a/bazel/BUILD.bazel b/bazel/BUILD.bazel
@@ -1,5 +1,28 @@
+load("@com_github_bazelbuild_buildtools//buildifier:def.bzl", "buildifier")
 load("//bazel:skia_rules.bzl", "exports_files_legacy")
 
 licenses(["notice"])
 
 exports_files_legacy()
+
+# Sample usage: "bazel run //bazel: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",
+    ],
+    visibility = ["//visibility:public"],
+)
diff --git a/bazel/exporter/interfaces/mocks/generate.go b/bazel/exporter/interfaces/mocks/generate.go
@@ -5,5 +5,5 @@
 
 package mocks
 
-//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}
+//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}
diff --git a/bazel/external/icu4x/BUILD.bazel b/bazel/external/icu4x/BUILD.bazel
@@ -2,8 +2,6 @@ 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()

diff --git a/bazel/remove_indentation.bzl b/bazel/remove_indentation.bzl
@@ -45,6 +45,9 @@ 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")

diff --git a/example/external_client/WORKSPACE.bazel b/example/external_client/WORKSPACE.bazel
@@ -1,11 +1,13 @@
-load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
-
 # A real client should download a pinned version of Skia such as:
-# git_repository(
-#     name = "skia",
-#     commit = "8b051126be8ae6c3e718bd3817eebb867b2fd612",
-#     remote = "https://skia.googlesource.com/skia",
-# )
+#
+#     load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
+#
+#     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(
@@ -29,9 +31,12 @@ 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()
 
 ##############################################################################
@@ -42,7 +47,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"],
+)