Merge pull request #102 from cgruber/configurable_jetifier

Make jetifier configurable.
square · May 9, 2020 · 327ca5a · 327ca5a
2 parents c9ea4b9 + a1971e9
commit 327ca5a
Show file tree

Hide file tree

Showing 10 changed files with 280 additions and 148 deletions.
diff --git a/.bazelignore b/.bazelignore
@@ -1 +1,2 @@
 test/test_workspace
+kramer
diff --git a/.bazelversion b/.bazelversion
@@ -0,0 +1 @@
+3.1.0
diff --git a/.travis.yml b/.travis.yml
@@ -11,9 +11,8 @@ addons:
       - pkg-config
 
 before_install:
-  - wget https://github.com/bazelbuild/bazel/releases/download/0.21.0/bazel_0.21.0-linux-x86_64.deb
-  - sha256sum -c tools/bazel_0.21.0-linux-x86_64.deb.sha256
-  - sudo dpkg -i bazel_0.21.0-linux-x86_64.deb
+  - wget https://github.com/bazelbuild/bazelisk/releases/download/v1.4.0/bazelisk-linux-amd64
+  - sudo install bazelisk-linux-amd64 /usr/local/bin/bazel
 
 script:
   - # Test the maven integration's internal unit tests

diff --git a/maven/jetifier.bzl b/maven/jetifier.bzl
diff --git a/maven/maven.bzl b/maven/maven.bzl
@@ -19,7 +19,13 @@ load(":jvm.bzl", "raw_jvm_import")
 load(":poms.bzl", "poms")
 load(":sets.bzl", "sets")
 load(":utils.bzl", "dicts", "paths", "strings")
-load(":jetifier.bzl", "jetifier_init", "jetify")
+load(
+    ":jetifier.bzl",
+    "DEFAULT_JETIFIER_EXCLUDED_ARTIFACTS",
+    "jetifier_init",
+    "jetify",
+    "jetify_utils",
+)
 
 #enum
 artifact_config_properties = struct(
@@ -293,6 +299,7 @@ def _generate_maven_repository_impl(ctx):
         manifests[m.workspace_name] = m
 
     # Generate the per-group_id BUILD.bazel files.
+    jetifier_excludes = jetify_utils.prepare_jetifier_excludes(ctx)
     build_snippets = ctx.attr.build_snippets
     target_substitutes = dicts.decode_nested(ctx.attr.dependency_target_substitutes)
     processed_artifacts = sets.new()
@@ -340,14 +347,18 @@ def _generate_maven_repository_impl(ctx):
                         spec,
                         list(unregistered_deps),
                     ))
-
+                use_jetifier = jetify_utils.should_use_jetifier(
+                    coordinates = coordinates,
+                    enabled = ctx.attr.use_jetifier,
+                    excludes = jetifier_excludes,
+                )
                 if artifact.packaging == "aar":
                     aar_params = _aar_template_params(
                         ctx,
                         artifact,
                         normalized_deps,
                         manifests[artifact.maven_target_name],
-                        ctx.attr.use_jetifier)
+                        use_jetifier)
                     target_definitions.append(
                         _MAVEN_REPO_AAR_TARGET_TEMPLATE.format(**aar_params),
                     )
@@ -357,15 +368,13 @@ def _generate_maven_repository_impl(ctx):
                             target = artifact.third_party_target_name,
                             deps = _deps_string(normalized_deps),
                             artifact_coordinates = artifact.original_spec,
-                            use_jetifier = "\n    use_jetifier = True," if ctx.attr.use_jetifier else ""
+                            use_jetifier = "\n    use_jetifier = True," if use_jetifier else ""
                         ),
                     )
         file = "%s/BUILD.bazel" % group_path
         content = "\n".join([prefix] + target_definitions)
         ctx.file(file, content)
 
-# build_files[build_file] = build_content
-
 _generate_maven_repository = repository_rule(
     implementation = _generate_maven_repository_impl,
     attrs = {
@@ -375,23 +384,11 @@ _generate_maven_repository = repository_rule(
         "dependency_target_substitutes": attr.string_list_dict(mandatory = True),
         "build_snippets": attr.string_dict(mandatory = True),
         "use_jetifier": attr.bool(default = False),
+        "jetifier_excludes": attr.string_list(mandatory = True),
         "manifests" : attr.label_list()
     },
 )
 
-JETIFIER_EXCLUDED_ARTIFACTS = [
-    "javax.annotation:jsr250-api",
-    "com.google.code.findbugs:jsr305",
-    "com.google.errorprone:javac-shaded",
-    "com.google.googlejavaformat:google-java-format",
-    "com.squareup:javapoet",
-    "com.google.dagger:dagger-compiler",
-    "com.google.dagger:dagger-producers",
-    "com.google.dagger:dagger-spi",
-    "com.google.dagger:dagger",
-    "javax.inject:javax.inject",
-]
-
 # Implementation of the maven_jvm_artifact rule.
 def _maven_jvm_artifact(artifact_spec, name, visibility, deps = [], use_jetifier = False, **kwargs):
     artifact = artifacts.annotate(artifacts.parse_spec(artifact_spec))
@@ -400,18 +397,11 @@ def _maven_jvm_artifact(artifact_spec, name, visibility, deps = [], use_jetifier
     target_name = name if name else artifact.third_party_target_name
     coordinate = "%s:%s" % (artifact.group_id, artifact.artifact_id)
 
-    should_jetify = (use_jetifier and
-        coordinate not in JETIFIER_EXCLUDED_ARTIFACTS and
-        artifact.group_id != "org.bouncycastle" and
-        artifact.group_id != "com.android.tools" and
-        artifact.group_id != "com.android.tools.build" and
-        not artifact.group_id.startswith("androidx"))
-
-    if should_jetify:
+    if use_jetifier:
         target = target_name + "_jetified"
         jetify(
             name = target,
-            srcs = [maven_target]
+            srcs = [maven_target],
         )
     else:
         target = maven_target
@@ -507,11 +497,12 @@ def _handle_legacy_specifications(artifact_declarations, insecure_artifacts, bui
 #
 def _maven_repository_specification(
         name,
+        use_jetifier,
+        jetifier_excludes,
         artifact_declarations = {},
         insecure_artifacts = [],
         build_substitutes = {},
         dependency_target_substitutes = {},
-        use_jetifier = False,
         repository_urls = ["https://repo1.maven.org/maven2"]):
     _handle_legacy_specifications(artifact_declarations, insecure_artifacts, build_substitutes)
 
@@ -559,6 +550,7 @@ def _maven_repository_specification(
         dependency_target_substitutes = dependency_target_substitutes_rewritten,
         build_snippets = build_snippets,
         use_jetifier = use_jetifier,
+        jetifier_excludes = jetifier_excludes,
         manifests = manifests,
     )
 
@@ -613,13 +605,19 @@ def maven_repository_specification(
         # "@myreponame//path/to/package:target": "@myrepotarget//path/to/package:alternate"
         dependency_target_substitutes = {},
 
-        #  TODO: Add suppot for opting out from jetifier
-        # use_jetifier = False,
+        # Activate jetifier on maven artifacts, except where excluded
+        use_jetifier = False,
+
+        # A list of artifacts to be excluded from jetifier processing, in the form "group:artifact"
+        # Note this is an exact group/artifact match. Future versions may support wildcards.
+        jetifier_excludes = DEFAULT_JETIFIER_EXCLUDED_ARTIFACTS,
 
         # Optional list of repositories which the build rule will attempt to fetch maven artifacts and metadata.
         repository_urls = ["https://repo1.maven.org/maven2"]):
     # Define repository rule for the jetifier tooling
-    jetifier_init()
+    if (use_jetifier):
+        jetifier_init()
+
     # Redirected to _maven_repository_specification to allow the public parameter "artifacts" without conflicting
     # with the artifact utility struct.
     _maven_repository_specification(
@@ -628,6 +626,7 @@ def maven_repository_specification(
         insecure_artifacts = insecure_artifacts,
         build_substitutes = build_substitutes,
         dependency_target_substitutes = dependency_target_substitutes,
-        use_jetifier = True,
+        use_jetifier = use_jetifier,
         repository_urls = repository_urls,
+        jetifier_excludes = jetifier_excludes,
     )
diff --git a/test/test_workspace/.bazelversion b/test/test_workspace/.bazelversion
@@ -0,0 +1 @@
+3.1.0
diff --git a/test/test_workspace/BUILD.bazel b/test/test_workspace/BUILD.bazel
@@ -1,3 +1,12 @@
 exports_files([
     "build_substitution_templates.bzl",
 ])
+
+load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "define_kt_toolchain")
+
+define_kt_toolchain(
+    name = "kotlin_toolchain",
+    api_version = "1.3",
+    jvm_target = "1.8",
+    language_version = "1.3",
+)
diff --git a/test/test_workspace/WORKSPACE b/test/test_workspace/WORKSPACE
@@ -14,29 +14,41 @@ load(
 )
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
-KOTLIN_VERSION = "1.2.70"
+KOTLIN_VERSION = "1.3.71"
 
-rules_kotlin_version = "porque_no_les_dos"
+KOTLINC_RELEASE_SHA = "7adb77dad99c6d2f7bde9f8bafe4c6244a04587a8e36e62b074d00eda9f8e74a"
+
+KOTLINC_RELEASE_URL = "https://github.com/JetBrains/kotlin/releases/download/v{v}/kotlin-compiler-{v}.zip".format(v = KOTLIN_VERSION)
+
+KOTLIN_RULES_VERSION = "legacy-1.3.0"
+
+KOTLIN_RULES_SHA = "4fd769fb0db5d3c6240df8a9500515775101964eebdf85a3f9f0511130885fde"
+
+KOTLIN_RULES_URL = "https://github.com/bazelbuild/rules_kotlin/archive/%s.zip" % KOTLIN_RULES_VERSION
 
 http_archive(
     name = "io_bazel_rules_kotlin",
-    strip_prefix = "rules_kotlin-%s" % rules_kotlin_version,
+    sha256 = KOTLIN_RULES_SHA,
+    strip_prefix = "rules_kotlin-%s" % KOTLIN_RULES_VERSION,
     type = "zip",
-    urls = ["https://github.com/cgruber/rules_kotlin/archive/%s.zip" % rules_kotlin_version],
+    urls = [KOTLIN_RULES_URL],
 )
 
 load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kotlin_repositories", "kt_register_toolchains")
 
-kotlin_repositories()
+kotlin_repositories(compiler_release = {
+    "urls": [KOTLINC_RELEASE_URL],
+    "sha256": KOTLINC_RELEASE_SHA,
+})
 
-kt_register_toolchains()
+register_toolchains("//:kotlin_toolchain")
 
 maven_repository_specification(
     name = "maven",
     artifacts = {
         # This is the proper way to specify an artifact.  It contains the artifact, plus a configuration dictionary.
         # The config dictionary contains a sha256 hash. This both ensures the file downloaded is the expected one, but
-        # also caches the file in bazel's "content addressable" cache, which survives build clean. 
+        # also caches the file in bazel's "content addressable" cache, which survives build clean.
         "com.google.guava:guava:25.0-jre": {"sha256": "3fd4341776428c7e0e5c18a7c10de129475b69ab9d30aeafbb5c277bb6074fa9"},
         "com.google.dagger:dagger:2.20": {
             "sha256": "d37a556d8d57e2428c20e222b95346512d11fcf2174d581489a69a1439b886fb",
@@ -73,7 +85,10 @@ maven_repository_specification(
         "org.checkerframework:checker-qual:2.5.3": {"insecure": True},
         "com.googlecode.java-diff-utils:diffutils:1.3.0": {"insecure": True},
         "com.google.auto.value:auto-value-annotations:1.6.3": {"insecure": True},
-        "com.google.auto.value:auto-value:1.6.3": { "insecure": True, "build_snippet": AUTO_VALUE_BUILD_SNIPPET_WITH_PLUGIN.format(version = "1.6.3"), },
+        "com.google.auto.value:auto-value:1.6.3": {
+            "insecure": True,
+            "build_snippet": AUTO_VALUE_BUILD_SNIPPET_WITH_PLUGIN.format(version = "1.6.3"),
+        },
     },
     # Because these apply to all targets within a group, it's specified separately from the artifact list.
     dependency_target_substitutes = {
@@ -83,4 +98,22 @@ maven_repository_specification(
         # "groupId": { "full bazel target": "full alternate target" }
         "com.google.dagger": {"@maven//com/google/dagger:dagger": "@maven//com/google/dagger:dagger_api"},
     },
+    jetifier_excludes = [
+        "javax.*:jsr250-api",
+        "com.google.code.findbugs:jsr305",
+        "com.google.errorprone:javac-shaded",
+        "com.google.googlejavaformat:google-java-format",
+        "com.google.truth:truth",
+        "com.squareup:javapoet",
+        "com.google.dagger:*",
+        "javax.inject:javax.inject",
+        "*:rxjava",
+        "junit:junit",
+        "org.jetbrains.kotlin*:*",
+        # "*:*", # Don't use this, just set use_jetifier=False
+        # "foo:foo*", # Artifacts must be literal or only the "*" wildcard. No partial matches.
+        # "foo.*.bar:baz", # Groups may only have "startswith" style globbing.
+        # "*.bar:baz",  # Groups may only have "startswith" style globbing.
+    ],
+    use_jetifier = True,
 )
diff --git a/test/test_workspace/javatests/foo/BUILD.bazel b/test/test_workspace/javatests/foo/BUILD.bazel
@@ -18,6 +18,7 @@ java_test(
     srcs = ["FooTest.java"],
     deps = [
         "//java/foo",
+        "@maven//com/google/truth",
         "@maven//junit",
     ],
 )
diff --git a/test/test_workspace/javatests/foo/FooTest.java b/test/test_workspace/javatests/foo/FooTest.java
@@ -3,13 +3,16 @@
 import foo.Foo.FooComponent;
 import org.junit.Test;
 
+import static com.google.common.truth.Truth.assertThat;
+
 public class FooTest {
   @Test public void testFoo() {
     Foo foo = new Foo();
     FooComponent fooComponent = FooComponent.create();
     fooComponent.bar();
 
-    Foo.Inner.builder().setFoo(foo).build();
+    Foo.Inner inner = Foo.Inner.builder().setFoo(foo).build();
+    assertThat(inner.foo()).isSameAs(foo);
   }
 
 }