feat: preserve some values when regenerating BUILD.bazel (#3237)

* feat: preserve some values when regenerating BUILD.bazel

* pr feedback

* use bazel version v2.2.0

* added comment to the generated BUILD.bazel

* pr feedback

* fix: accept buildozer path as a parameter

* pr feedback

bazel/BUILD.bazel

@@ -1,7 +1,16 @@
+genrule(
+    name = "buildozer_binary",
+    srcs = ["@com_github_bazelbuild_buildtools//buildozer:buildozer"],
+    outs = ["buildozer.bin"],
+    cmd = "cp $(location @com_github_bazelbuild_buildtools//buildozer:buildozer) \"$@\" && chmod +x \"$@\"",
+)
+
 java_binary(
     name = "build_file_generator",
     srcs = glob(["src/main/java/**/*.java"]),
     resources = glob(["src/main/java/**/*.mustache"]),
+    data = [":buildozer_binary"],
+    args = ["--buildozer=$(location :buildozer_binary)"],
     create_executable = True,
     javacopts = ["-source", "1.8", "-target", "1.8"],
     jvm_flags = ["-Xmx1024m"],
@@ -13,6 +22,7 @@ java_test(
     name = "build_file_generator_test",
     srcs = glob(["src/test/java/**/*.java"]),
     deps = [":build_file_generator"],
-    data = glob(["src/test/data/**/*.*"]),
+    data = glob(["src/test/data/**/*.*"]) + [":buildozer_binary"],
     test_class = "com.google.api.codegen.bazel.BuildFileGeneratorTest",
+    jvm_flags = ["-Dbuildozer=$(location :buildozer_binary)"],
 )

bazel/src/main/java/com/google/api/codegen/bazel/ApiVersionedDir.java

@@ -1,7 +1,11 @@
 package com.google.api.codegen.bazel;
 
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
@@ -40,6 +44,18 @@ class ApiVersionedDir {
 
   private static String CLOUD_AUTH_SCOPE = "https://www.googleapis.com/auth/cloud-platform";
 
+  private static final String[] PRESERVED_PROTO_LIBRARY_STRING_ATTRIBUTES = {
+    // TypeScript:
+    "package_name", "main_service", "bundle_config", "iam_service",
+    // Other languages: add below
+  };
+
+  private static final String[] PRESERVED_PROTO_LIBRARY_LIST_ATTRIBUTES = {
+    // All languages:
+    "extra_protoc_parameters", "extra_protoc_file_parameters",
+    // Other languages: add below
+  };
+
   // A reference to the object representing the parent dir of this versioned API dir.
   // For example: google/example/library.
   private ApiDir parent;
@@ -123,6 +139,13 @@ class ApiVersionedDir {
   //         https://www.googleapis.com/auth/cloud-platform
   private boolean cloudScope;
 
+  // Names of *_gapic_assembly_* rules (since they may be overridden by the user)
+  private final Map<String, String> assemblyPkgRulesNames = new HashMap<>();
+
+  // Attributes of *_gapic_library rules to be overridden
+  private final Map<String, Map<String, String>> overriddenStringAttributes = new HashMap<>();
+  private final Map<String, Map<String, List<String>>> overriddenListAttributes = new HashMap<>();
+
   void setParent(ApiDir parent) {
     this.parent = parent;
   }
@@ -183,6 +206,18 @@ boolean getCloudScope() {
     return cloudScope;
   }
 
+  Map<String, Map<String, String>> getOverriddenStringAttributes() {
+    return overriddenStringAttributes;
+  }
+
+  Map<String, Map<String, List<String>>> getOverriddenListAttributes() {
+    return overriddenListAttributes;
+  }
+
+  Map<String, String> getAssemblyPkgRulesNames() {
+    return assemblyPkgRulesNames;
+  }
+
   void parseYamlFile(String fileName, String fileBody) {
     // It is a gapic yaml
     Matcher m = GAPIC_YAML_TYPE.matcher(fileBody);
@@ -285,6 +320,59 @@ void parseJsonFile(String fileName, String fileBody) {
     }
   }
 
+  void parseBazelBuildFile(Path file) {
+    try {
+      Buildozer buildozer = Buildozer.getInstance();
+
+      // We cannot and we do not want to preserve all the content of the file.
+      // We will let the user edit just the following:
+      // - names of the final targets (*_gapic_assembly_*) because they are user-facing;
+      // - extra protoc plugin parameters for *_gapic_library rules.
+      List<String> allRules = buildozer.execute(file, "print kind name", "*");
+      for (String rule : allRules) {
+        String[] split = rule.split(" ");
+        if (split.length != 2) {
+          // some rules e.g. package() don't have "name" attribute, just skip them
+          continue;
+        }
+        String kind = split[0];
+        String name = split[1];
+        if (kind.contains("_gapic_assembly_")) {
+          if (this.assemblyPkgRulesNames.containsKey(kind)) {
+            // Duplicated rule of the same kind will break our logic for preserving rule name.
+            System.err.println("There are more than one rule of kind " + kind + ".");
+            System.err.println(
+                "Bazel build file generator does not support regenerating BUILD.bazel in this case.");
+            System.err.println(
+                "Please run it with --overwrite option to overwrite the existing BUILD.bazel completely.");
+            throw new RuntimeException("Duplicated rule " + kind);
+          }
+          this.assemblyPkgRulesNames.put(kind, name);
+        } else if (kind.endsWith("_gapic_library")) {
+          this.overriddenStringAttributes.put(name, new HashMap<>());
+          this.overriddenListAttributes.put(name, new HashMap<>());
+          for (String attr : ApiVersionedDir.PRESERVED_PROTO_LIBRARY_STRING_ATTRIBUTES) {
+            String value = buildozer.getAttribute(file, name, attr);
+            if (value != null) {
+              this.overriddenStringAttributes.get(name).put(attr, value);
+            }
+          }
+          for (String attr : ApiVersionedDir.PRESERVED_PROTO_LIBRARY_LIST_ATTRIBUTES) {
+            String value = buildozer.getAttribute(file, name, attr);
+            if (value != null && value.startsWith("[") && value.endsWith("]")) {
+              value = value.substring(1, value.length() - 1);
+              String[] values = value.split(" ");
+              this.overriddenListAttributes.get(name).put(attr, Arrays.asList(values));
+            }
+          }
+        }
+      }
+    } catch (IOException exception) {
+      System.err.println(
+          "Error parsing BUILD.bazel file in " + file.toString() + ": " + exception.toString());
+    }
+  }
+
   void injectFieldsFromTopLevel() {
     if (parent == null || serviceYamlPath != null) {
       return;

bazel/src/main/java/com/google/api/codegen/bazel/ApisVisitor.java

@@ -25,6 +25,7 @@ interface FileWriter {
   private final BazelBuildFileTemplate rawApiTempl;
   private final Path srcDir;
   private final Path destDir;
+  private final boolean overwrite;
   private boolean writerMode;
   private final FileWriter fileWriter;
 
@@ -34,12 +35,14 @@ interface FileWriter {
       String gapicApiTempl,
       String rootApiTempl,
       String rawApiTempl,
+      boolean overwrite,
       FileWriter fileWriter) {
     this.gapicApiTempl = new BazelBuildFileTemplate(gapicApiTempl);
     this.rootApiTempl = new BazelBuildFileTemplate(rootApiTempl);
     this.rawApiTempl = new BazelBuildFileTemplate(rawApiTempl);
     this.srcDir = srcDir.normalize();
     this.destDir = destDir.normalize();
+    this.overwrite = overwrite;
     this.writerMode = false;
     this.fileWriter =
         (fileWriter != null)
@@ -98,9 +101,10 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
     } else if (fileName.endsWith(".proto")) {
       bp.parseProtoFile(fileName, readFile(file));
     } else if (fileName.endsWith(".bazel")) {
-      // Consider merging BUILD.bazel files if it becomes necessary (i.e. people will be doing many
-      // valuable manual edits in their BUILD.bazel files). This will complicate the whole logic
-      // so not doing it for now, hoping it will not be required.
+      // if --overwrite is given, we don't care what's in the existing BUILD.bazel file.
+      if (!overwrite) {
+        bp.parseBazelBuildFile(file);
+      }
     } else if (fileName.endsWith(".json")) {
       bp.parseJsonFile(fileName, readFile(file));
     }

bazel/src/main/java/com/google/api/codegen/bazel/ArgsParser.java

@@ -15,6 +15,10 @@ class ArgsParser {
   ArgsParser(String[] args) {
     for (String arg : args) {
       String[] argNameVal = arg.split("=");
+      if (argNameVal.length == 1) {
+        parsedArgs.put(argNameVal[0], "true");
+        continue;
+      }
       if (argNameVal.length != 2) {
         System.out.println("WARNING: Ignoring unrecognized argument: " + arg);
         continue;
@@ -29,15 +33,38 @@ class ArgsParser {
               + "The required arguments are: "
               + required;
       System.out.println(msg);
+      ArgsParser.printUsage();
       throw new IllegalArgumentException(msg);
     }
   }
 
+  static void printUsage() {
+    String helpMessage =
+        "Usage (when running from googleapis folder):\n"
+            + "  bazel run //:build_gen -- --src=rules_gapic/bazel/src/test/data/googleapis\n"
+            + "\n"
+            + "Command line options:\n"
+            + "  --src=path: location of googleapis directory\n"
+            + "  --dest=path: destination folder, defaults to the value of --src\n"
+            + "  --overwrite: do not preserve any of the manually changed values in the generated BUILD.bazel files\n";
+    System.out.println(helpMessage);
+  }
+
   ApisVisitor createApisVisitor(ApisVisitor.FileWriter fileWriter, String relativePathPrefix)
       throws IOException {
+    if (parsedArgs.get("--help") != null) {
+      ArgsParser.printUsage();
+      throw new IllegalArgumentException();
+    }
+
+    String buildozerPath = parsedArgs.get("--buildozer");
     String gapicApiTemplPath = parsedArgs.get("--gapic_api_templ");
     String rootApiTemplPath = parsedArgs.get("--root_api_templ");
     String rawApiTempl = parsedArgs.get("--raw_api_templ");
+    String overwrite = parsedArgs.get("--overwrite");
+    if (overwrite == null) {
+      overwrite = "false";
+    }
 
     Path srcPath = Paths.get(parsedArgs.get("--src")).normalize();
     Path destPath = srcPath;
@@ -55,6 +82,14 @@ ApisVisitor createApisVisitor(ApisVisitor.FileWriter fileWriter, String relative
       }
     }
 
+    if (buildozerPath == null && !overwrite.equals("true")) {
+      System.err.println("This tool requires Buildozer tool to parse BUILD.bazel files.");
+      System.err.println("Please use --buildozer=/path/to/buildozer to point to Buildozer,");
+      System.err.println("or use --overwrite if you want to rewrite all BUILD.bazel files.");
+      throw new IllegalArgumentException();
+    }
+    Buildozer.setBinaryPath(buildozerPath);
+
     return new ApisVisitor(
         srcPath,
         destPath,
@@ -67,6 +102,7 @@ ApisVisitor createApisVisitor(ApisVisitor.FileWriter fileWriter, String relative
         rawApiTempl == null
             ? readResource("BUILD.bazel.raw_api.mustache")
             : ApisVisitor.readFile(rawApiTempl),
+        overwrite.equals("true"),
         fileWriter);
   }
 

bazel/src/main/java/com/google/api/codegen/bazel/BUILD.bazel.gapic_api.mustache

@@ -1,4 +1,13 @@
 # This file was automatically generated by BuildFileGenerator
+# https://github.com/googleapis/gapic-generator/tree/master/rules_gapic/bazel
+
+# Most of the manual changes to this file will be overwritten.
+# It's **only** allowed to change the following rule attribute values:
+# - names of *_gapic_assembly_* rules
+# - certain parameters of *_gapic_library rules, including but not limited to:
+#    * extra_protoc_parameters
+#    * extra_protoc_file_parameters
+# The complete list of preserved parameters can be found in the source code.
 
 # This is an API workspace, having public visibility by default makes perfect sense.
 package(default_visibility = ["//visibility:public"])

bazel/src/main/java/com/google/api/codegen/bazel/BazelBuildFileTemplate.java

@@ -1,5 +1,11 @@
 package com.google.api.codegen.bazel;
 
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -32,7 +38,69 @@ private String expand(Map<String, String> tokens) {
     return builder.toString();
   }
 
-  String expand(BazelBuildFileView bpv) {
-    return this.expand(bpv.getTokens());
+  String expand(BazelBuildFileView bpv) throws IOException {
+    String expandedTemplate = this.expand(bpv.getTokens());
+
+    // Apply overrides
+    Map<String, Map<String, String>> overriddenStringAttributes =
+        bpv.getOverriddenStringAttributes();
+    Map<String, Map<String, List<String>>> overriddenListAttributes =
+        bpv.getOverriddenListAttributes();
+    Map<String, String> assemblyPkgRulesNames = bpv.getAssemblyPkgRulesNames();
+    if (overriddenStringAttributes.size() == 0
+        && overriddenListAttributes.size() == 0
+        && assemblyPkgRulesNames.size() == 0) {
+      // nothing to override
+      return expandedTemplate;
+    }
+
+    // write the content of the build file to a temporary directory and fix it with Buildozer
+    File tempdir = Files.createTempDirectory("build_file_generator_").toFile();
+    File buildBazel = new File(tempdir, "BUILD.bazel");
+    Path buildBazelPath = buildBazel.toPath();
+    Files.write(buildBazelPath, expandedTemplate.getBytes(StandardCharsets.UTF_8));
+
+    Buildozer buildozer = Buildozer.getInstance();
+
+    // First of all, rename the rules
+    for (Map.Entry<String, String> entry : assemblyPkgRulesNames.entrySet()) {
+      String kind = entry.getKey();
+      String newName = entry.getValue();
+      String currentName = buildozer.getAttribute(buildBazelPath, "%" + kind, "name");
+      if (!currentName.equals(newName)) {
+        buildozer.batchSetAttribute(buildBazelPath, currentName, "name", newName);
+      }
+    }
+    buildozer.commit();
+
+    // Apply preserved string attribute values
+    for (Map.Entry<String, Map<String, String>> entry : overriddenStringAttributes.entrySet()) {
+      String ruleName = entry.getKey();
+      for (Map.Entry<String, String> subentry : entry.getValue().entrySet()) {
+        String attr = subentry.getKey();
+        String value = subentry.getValue();
+        buildozer.batchSetAttribute(buildBazelPath, ruleName, attr, value);
+      }
+    }
+    // Apply preserved list attribute values
+    for (Map.Entry<String, Map<String, List<String>>> entry : overriddenListAttributes.entrySet()) {
+      String ruleName = entry.getKey();
+      for (Map.Entry<String, List<String>> subentry : entry.getValue().entrySet()) {
+        String attr = subentry.getKey();
+        List<String> values = subentry.getValue();
+        buildozer.batchRemoveAttribute(buildBazelPath, ruleName, attr);
+        for (String value : values) {
+          buildozer.batchAddAttribute(buildBazelPath, ruleName, attr, value);
+        }
+      }
+    }
+    buildozer.commit();
+
+    String updatedContent = new String(Files.readAllBytes(buildBazelPath), StandardCharsets.UTF_8);
+
+    buildBazel.delete();
+    tempdir.delete();
+
+    return updatedContent;
   }
 }

bazel/src/main/java/com/google/api/codegen/bazel/BazelBuildFileView.java

@@ -12,6 +12,9 @@
 class BazelBuildFileView {
   private static final Pattern LABEL_NAME = Pattern.compile(":\\w+$");
   private final Map<String, String> tokens = new HashMap<>();
+  private final Map<String, Map<String, String>> overriddenStringAttributes = new HashMap<>();
+  private final Map<String, Map<String, List<String>>> overriddenListAttributes = new HashMap<>();
+  private final Map<String, String> assemblyPkgRulesNames = new HashMap<>();
 
   BazelBuildFileView(ApiVersionedDir bp) {
     if (bp.getProtoPackage() == null) {
@@ -91,6 +94,10 @@ class BazelBuildFileView {
     tokens.put("go_gapic_importpath", goImport);
     tokens.put("go_gapic_test_importpath", goImport.split(";")[0]);
     tokens.put("go_gapic_deps", joinSetWithIndentationNl(mapGoGapicDeps(actualImports)));
+
+    overriddenStringAttributes.putAll(bp.getOverriddenStringAttributes());
+    overriddenListAttributes.putAll(bp.getOverriddenListAttributes());
+    assemblyPkgRulesNames.putAll(bp.getAssemblyPkgRulesNames());
   }
 
   private String assembleGoImportPath(boolean isCloud, String protoPkg, String goPkg) {
@@ -258,4 +265,16 @@ private Set<String> mapGoGapicDeps(Set<String> protoImports) {
   Map<String, String> getTokens() {
     return Collections.unmodifiableMap(this.tokens);
   }
+
+  Map<String, Map<String, String>> getOverriddenStringAttributes() {
+    return overriddenStringAttributes;
+  }
+
+  Map<String, Map<String, List<String>>> getOverriddenListAttributes() {
+    return overriddenListAttributes;
+  }
+
+  Map<String, String> getAssemblyPkgRulesNames() {
+    return assemblyPkgRulesNames;
+  }
 }