bazelbuild / bazel-skylib

Common useful functions and rules for Bazel
https://bazel.build/
Apache License 2.0
387 stars 180 forks source link

Gazelle plugin just deleting `srcs` from some libraries #454

Open shahms opened 1 year ago

shahms commented 1 year ago

When running the bzl_library Gazelle plugin over https://github.com/kythe/kythe/blob/master/tools/build_rules/verifier_test/BUILD it ends up modifying that file to just delete the srcs attribute from the respective bzl_library targets and it's not obvious to me why.

$ bazel run //:gazelle -- -mode diff  tools/build_rules/verifier_test/
--- tools/build_rules/verifier_test/BUILD       1970-01-01 00:00:00.000000001 +0000
+++ tools/build_rules/verifier_test/BUILD       1970-01-01 00:00:00.000000001 +0000
@@ -17,7 +17,6 @@

 bzl_library(
     name = "cc_indexer_test_bzl",
-    srcs = ["cc_indexer_test.bzl"],
     deps = [
         ":verifier_test_bzl",
         "@bazel_skylib//lib:paths",
@@ -27,13 +26,11 @@

 bzl_library(
     name = "java_verifier_test_bzl",
-    srcs = ["java_verifier_test.bzl"],
     deps = [":verifier_test_bzl"],
 )

 bzl_library(
     name = "verifier_test_bzl",
-    srcs = ["verifier_test.bzl"],
     deps = [
         "@bazel_skylib//lib:shell",
     ],
@@ -41,21 +38,19 @@

 bzl_library(
     name = "jvm_verifier_test_bzl",
-    srcs = ["jvm_verifier_test.bzl"],
     deps = [":verifier_test_bzl"],
 )

 bzl_library(
-    name = "kzip_archive_bzl",
-    srcs = ["kzip_archive.bzl"],
-)
-
-bzl_library(
     name = "rust_indexer_test_bzl",
-    srcs = ["rust_indexer_test.bzl"],
     deps = [
         ":verifier_test_bzl",
         "@bazel_skylib//lib:paths",
     ],
 )

+bzl_library(
+    name = "kzip_archive",
+    srcs = ["kzip_archive.bzl"],
+)
+
achew22 commented 1 year ago

Interesting! Were the old values generated with the gazelle plugin?

shahms commented 1 year ago

No, these were hand-written rules originally. I'm trying to enable gazelle support for them so we can stop hand-writing so much :-)

shahms commented 1 year ago

It appears to be because these are files which define a rule which is itself a test rule, thus the files end in "_test.bzl" and are (erroneously) ignored due to: https://github.com/bazelbuild/bazel-skylib/blob/288731ef9f7f688932bd50e704a91a45ec185f9b/gazelle/bzl/gazelle.go#L51

achew22 commented 1 year ago

Very interesting find! I believe the logic behind that is that it is possible to make bzl_library tests a la this.

I'm curious what your thoughts on this are? Would you be amenable to naming the file java_verifier.bzl or cc_indexer.bzl?

shahms commented 1 year ago

yeah, it's just a lot of work to update all of the references

achew22 commented 1 year ago

You don't want an excuse to fire up rosie 😉 ?

keith commented 10 months ago

I hit this case too, we have a file called swift_binary_test.bzl which contains a swift_binary and swift_test rule