Skip to content

Commit

Permalink
chore: re-work build infra, primarily around code format machinery (#915
Browse files Browse the repository at this point in the history
)

- Removes `gapic_generator_java.bzl`.

- `:google_java_format_verification` is now working. (Fixes #914.)

- `:google_java_format_verification` now runs with `bazel run` instead of `bazel build`, which is consistent with how `:google_java_format` runs.

- Removes manual listing of files to format, which is error-prone. This actually revealed that we were missing out some files for formatting. (For example, `AbstractTransportServiceStubClassComposer.java` had an unused import.) 

- Removes unnecessary `filegroup` definitions triggered by removing manual file listing.

- Reduces visibility of many packages.
  • Loading branch information
chanseokoh authored Jan 28, 2022
1 parent 9e7689a commit 99b3da7
Show file tree
Hide file tree
Showing 54 changed files with 102 additions and 487 deletions.
2 changes: 1 addition & 1 deletion .githooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ fi
if [ $NUM_JAVA_FILES_CHANGED -gt 0 ]
then
echo_status "Running Java linter..."
bazel --batch build --disk_cache="$BAZEL_CACHE_DIR" //:google_java_format_verification
bazel --batch run --disk_cache="$BAZEL_CACHE_DIR" //:google_java_format_verification
FORMAT_STATUS=$?
if [ $FORMAT_STATUS != 0 ]
then
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ jobs:
retention-days: 5

- name: Java Linter
run: bazel --batch build //:google_java_format_verification
run: bazel --batch run //:google_java_format_verification

coverage:
runs-on: ubuntu-latest
Expand All @@ -86,7 +86,7 @@ jobs:

- name: Generate Code Coverage Report
# Run only test targets, and not golden_update targets.
run: bazel coverage $(bazel query "src/test/..." | grep "Test$") --combined_report=lcov
run: bazel coverage $(bazel query "src/test/..." | grep "Test$") --combined_report=lcov

- name: Upload Code Coverage Report
uses: codecov/codecov-action@v1
Expand Down
47 changes: 15 additions & 32 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,28 +1,7 @@
load("@rules_java//java:defs.bzl", "java_binary", "java_proto_library")
load(
"//:gapic_generator_java.bzl",
"google_java_format",
"google_java_format_verification",
)

package(default_visibility = ["//visibility:public"])

JAVA_SRCS = [
"//src/main/java/com/google/api/generator/debug:debug_files",
"//src/main/java/com/google/api/generator:generator_files",
"//src/main/java/com/google/api/generator/engine:engine_files",
"//src/main/java/com/google/api/generator/gapic:gapic_files",
"//src/main/java/com/google/api/generator/util:util_files",
]

TEST_SRCS = [
"//src/test/java/com/google/api/generator/engine:engine_files",
"//src/test/java/com/google/api/generator/gapic:gapic_files",
"//src/test/java/com/google/api/generator/testutils:testutils_files",
"//src/test/java/com/google/api/generator/util:util_files",
"//src/test/java/com/google/api/generator/test/framework:framework_files",
]

# ============= Proto wrappers =================

java_proto_library(
Expand Down Expand Up @@ -104,21 +83,25 @@ java_binary(
name = "google_java_format_binary",
jvm_flags = ["-Xmx512m"],
main_class = "com.google.googlejavaformat.java.Main",
visibility = ["//visibility:public"],
runtime_deps = ["@google_java_format_all_deps//jar"],
)

genrule(
name = "google_java_format",
outs = ["google_java_format.sh"],
cmd = "echo 'find src test -name \'*.java\' | grep -v /goldens/ | xargs $(execpath :google_java_format_binary) --replace' > $(OUTS)",
executable = 1,
tools = [":google_java_format_binary"],
local = 1,
)

# Run `bazel build //:google_java_format_verification` to verify that gapic-generator-java sources
# are formatted correctly.
google_java_format_verification(
genrule(
name = "google_java_format_verification",
srcs = JAVA_SRCS + TEST_SRCS,
formatter = "//:google_java_format_binary",
)

# Run `bazel run //:google_java_format` to format gapic-generator-java sources.
google_java_format(
name = "google_java_format",
srcs = JAVA_SRCS + TEST_SRCS,
formatter = "//:google_java_format_binary",
outs = ["google_java_format_verification.sh"],
cmd = "echo 'find src test -name \'*.java\' | grep -v /goldens/ | xargs $(execpath :google_java_format_binary) --dry-run --set-exit-if-changed' > $(OUTS)",
executable = 1,
tools = [":google_java_format_binary"],
local = 1,
)
2 changes: 1 addition & 1 deletion DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
- Run linter checks without actually doing the formatting.

```sh
bazel build :google_java_format_verification
bazel run :google_java_format_verification
```

- Format files.
Expand Down
53 changes: 0 additions & 53 deletions gapic_generator_java.bzl

This file was deleted.

12 changes: 2 additions & 10 deletions src/main/java/com/google/api/generator/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
load("@rules_java//java:defs.bzl", "java_library", "java_plugin")

package(default_visibility = ["//visibility:public"])
#package(default_visibility = ["//:__pkg__"])

filegroup(
name = "generator_files",
srcs = glob(["*.java"]),
)
package(default_visibility = ["//:__subpackages__"])

java_plugin(
name = "autovalue_plugin",
Expand All @@ -29,9 +23,7 @@ java_library(

java_library(
name = "generator",
srcs = [
":generator_files",
],
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/api/generator/engine",
"//src/main/java/com/google/api/generator/engine/ast",
Expand Down
11 changes: 1 addition & 10 deletions src/main/java/com/google/api/generator/debug/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "debug_files",
srcs = glob(["*.java"]),
)

java_library(
name = "debug",
srcs = [
":debug_files",
],
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/api/generator",
"//src/main/java/com/google/api/generator/engine",
Expand Down
12 changes: 1 addition & 11 deletions src/main/java/com/google/api/generator/engine/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,16 +1,6 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "engine_files",
srcs = [
"//src/main/java/com/google/api/generator/engine/ast:ast_files",
"//src/main/java/com/google/api/generator/engine/escaper:escaper_files",
"//src/main/java/com/google/api/generator/engine/lexicon:lexicon_files",
"//src/main/java/com/google/api/generator/engine/writer:writer_files",
],
)
package(default_visibility = ["//:__subpackages__"])

java_library(
name = "engine",
Expand Down
11 changes: 2 additions & 9 deletions src/main/java/com/google/api/generator/engine/ast/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "ast_files",
srcs = glob(["*.java"]),
)
package(default_visibility = ["//:__subpackages__"])

java_library(
name = "ast",
srcs = [
":ast_files",
],
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/api/generator:autovalue",
"//src/main/java/com/google/api/generator/engine/escaper",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "escaper_files",
srcs = glob(["*.java"]),
)
package(default_visibility = ["//:__subpackages__"])

java_library(
name = "escaper",
srcs = [
":escaper_files",
],
srcs = glob(["*.java"]),
deps = [
"@com_google_guava_guava//jar",
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "lexicon_files",
srcs = glob(["*.java"]),
)
package(default_visibility = ["//:__subpackages__"])

java_library(
name = "lexicon",
srcs = [
":lexicon_files",
],
srcs = glob(["*.java"]),
deps = [
"@com_google_guava_guava//jar",
],
Expand Down
11 changes: 2 additions & 9 deletions src/main/java/com/google/api/generator/engine/writer/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "writer_files",
srcs = glob(["*.java"]),
)
package(default_visibility = ["//:__subpackages__"])

java_library(
name = "writer",
srcs = [
":writer_files",
],
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/api/generator/engine/ast",
"@com_google_guava_guava//jar",
Expand Down
22 changes: 1 addition & 21 deletions src/main/java/com/google/api/generator/gapic/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,26 +1,6 @@
load("@rules_java//java:defs.bzl", "java_library", "java_proto_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "gapic_files",
srcs = glob(["*.java"]) + [
"//src/main/java/com/google/api/generator/gapic/composer:composer_files",
"//src/main/java/com/google/api/generator/gapic/composer/comment:comment_files",
"//src/main/java/com/google/api/generator/gapic/composer/defaultvalue:defaultvalue_files",
"//src/main/java/com/google/api/generator/gapic/composer/grpc:grpc_files",
"//src/main/java/com/google/api/generator/gapic/composer/grpcrest:grpcrest_files",
"//src/main/java/com/google/api/generator/gapic/composer/resourcename:resourcename_files",
"//src/main/java/com/google/api/generator/gapic/composer/rest:rest_files",
"//src/main/java/com/google/api/generator/gapic/composer/samplecode:samplecode_files",
"//src/main/java/com/google/api/generator/gapic/composer/store:store_files",
"//src/main/java/com/google/api/generator/gapic/composer/utils:utils_files",
"//src/main/java/com/google/api/generator/gapic/model:model_files",
"//src/main/java/com/google/api/generator/gapic/protoparser:protoparser_files",
"//src/main/java/com/google/api/generator/gapic/protowriter:protowriter_files",
"//src/main/java/com/google/api/generator/gapic/utils:utils_files",
],
)
package(default_visibility = ["//:__subpackages__"])

java_proto_library(
name = "status_java_proto",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "composer_files",
srcs = glob(["*.java"]),
)
package(default_visibility = ["//:__subpackages__"])

java_library(
name = "composer",
srcs = [
":composer_files",
],
srcs = glob(["*.java"]),
deps = [
"//:service_config_java_proto",
"//src/main/java/com/google/api/generator/engine/ast",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
load("@rules_java//java:defs.bzl", "java_library")

package(default_visibility = ["//visibility:public"])

filegroup(
name = "comment_files",
srcs = glob(["*.java"]),
)
package(default_visibility = ["//:__subpackages__"])

java_library(
name = "comment",
srcs = [
":comment_files",
],
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/api/generator/engine/ast",
"//src/main/java/com/google/api/generator/gapic/composer/utils",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ protected TypeNode getOperationsStubType(Service service) {
return operationsStubType;
}


private TypeStore createTypes(Service service) {
List<Class<?>> concreteClazzes =
Arrays.asList(
Expand Down
Loading

0 comments on commit 99b3da7

Please sign in to comment.