Fix issue where a jetified maven artifact did not also jetify the deps (

#386)

* Fix issue where a jetified maven artifact did not also jetify the deps

* Add support for second-pass coursier fetch

* Add a basic build_test to verify build targets after jetification

* Add more tests for jetifier.bzl, add some small perf improvements

* Add _test

* Add tests for extract_jetify_artifacts

* Remove jetified build targets for versioned deps

* Add comments on how to generate the jetifier maven map

* Remove refactorings and inline added functions

* Rewrote tests to accomodate inlining

* Revert more refactoring

* Add docs and report progress for additional fetch for jetifier

README.md

@@ -811,6 +811,9 @@ Enable jetification by specifying `jetify = True` in `maven_install.`
 Control which artifacts to jetify with `jetify_include_list` — list of artifacts that need to be jetified in `groupId:artifactId` format. 
 By default all artifacts are jetified if `jetify` is set to True.
 
+NOTE: There is a performance penalty to using jetifier due to modifying fetched binaries, fetching
+additional `AndroidX` artifacts, and modifying the maven dependency graph.
+
 ```python
 maven_install(
     artifacts = [

WORKSPACE

@@ -321,7 +321,8 @@ maven_install(
     name = "jetify_all_test",
     artifacts = [
         "com.google.guava:guava:27.0-jre",
-        "com.android.support:appcompat-v7:28.0.0"
+        "com.android.support:appcompat-v7:28.0.0",
+        "com.android.support:swiperefreshlayout:28.0.0",
     ],
     repositories = [
         "https://jcenter.bintray.com/",

coursier.bzl

@@ -12,7 +12,8 @@
 # See the License for the specific language governing permissions and
 
 load("//third_party/bazel_json/lib:json_parser.bzl", "json_parse")
-load("//:specs.bzl", "utils")
+load("//private/rules:jetifier.bzl", "jetify_artifact_dependencies", "jetify_maven_coord")
+load("//:specs.bzl", "maven", "parse", "utils")
 load("//:private/proxy.bzl", "get_java_proxy_args")
 load("//:private/dependency_tree_parser.bzl", "JETIFY_INCLUDE_LIST_JETIFY_ALL", "parser")
 load("//:private/coursier_utilities.bzl", "SUPPORTED_PACKAGING_TYPES", "escape")
@@ -443,47 +444,25 @@ def get_coursier_cache_or_default(env_dict, use_unsafe_shared_cache):
         # This is an absolute path.
         return location
 
-def _coursier_fetch_impl(repository_ctx):
-    # Not using maven_install.json, so we resolve and fetch from scratch.
-    # This takes significantly longer as it doesn't rely on any local
-    # caches and uses Coursier's own download mechanisms.
-
-    # Download Coursier's standalone (deploy) jar from Maven repositories.
-    repository_ctx.download([
-        COURSIER_CLI_GITHUB_ASSET_URL,
-        COURSIER_CLI_BAZEL_MIRROR_URL,
-    ], "coursier", sha256 = COURSIER_CLI_SHA256, executable = True)
-
-    # Try running coursier once
-    exec_result = repository_ctx.execute(
-        _generate_java_jar_command(repository_ctx, repository_ctx.path("coursier")),
-    )
-    if exec_result.return_code != 0:
-        fail("Unable to run coursier: " + exec_result.stderr)
-
-    _windows_check(repository_ctx)
-
-    # Deserialize the spec blobs
-    repositories = []
-    for repository in repository_ctx.attr.repositories:
-        repositories.append(json_parse(repository))
-
-    artifacts = []
-    for a in repository_ctx.attr.artifacts:
-        artifacts.append(json_parse(a))
-
-    excluded_artifacts = []
-    for a in repository_ctx.attr.excluded_artifacts:
-        excluded_artifacts.append(json_parse(a))
-
-    artifact_coordinates = []
-
+def make_coursier_dep_tree(
+        repository_ctx,
+        artifacts,
+        excluded_artifacts,
+        repositories,
+        version_conflict_policy,
+        fail_on_missing_checksum,
+        fetch_sources,
+        use_unsafe_shared_cache,
+        timeout,
+        report_progress_prefix="",
+):
     # Set up artifact exclusion, if any. From coursier fetch --help:
     #
     # Path to the local exclusion file. Syntax: <org:name>--<org:name>. `--` means minus. Example file content:
     # com.twitter.penguin:korean-text--com.twitter:util-tunable-internal_2.11
     # org.apache.commons:commons-math--com.twitter.search:core-query-nodes
     # Behavior: If root module A excludes module X, but root module B requires X, module X will still be fetched.
+    artifact_coordinates = []
     exclusion_lines = []
     for a in artifacts:
         artifact_coordinates.append(utils.artifact_coordinate(a))
@@ -495,8 +474,9 @@ def _coursier_fetch_impl(repository_ctx):
 
     cmd = _generate_java_jar_command(repository_ctx, repository_ctx.path("coursier"))
     cmd.extend(["fetch"])
+
     cmd.extend(artifact_coordinates)
-    if repository_ctx.attr.version_conflict_policy == "pinned":
+    if version_conflict_policy == "pinned":
         for coord in artifact_coordinates:
             # Undo any `,classifier=` suffix from `utils.artifact_coordinate`.
             cmd.extend(["--force-version", coord.split(",classifier=")[0]])
@@ -505,7 +485,7 @@ def _coursier_fetch_impl(repository_ctx):
     cmd.append("--no-default")
     cmd.extend(["--json-output-file", "dep-tree.json"])
 
-    if repository_ctx.attr.fail_on_missing_checksum:
+    if fail_on_missing_checksum:
         cmd.extend(["--checksum", "SHA-1,MD5"])
     else:
         cmd.extend(["--checksum", "SHA-1,MD5,None"])
@@ -520,16 +500,16 @@ def _coursier_fetch_impl(repository_ctx):
     for a in excluded_artifacts:
         cmd.extend(["--exclude", ":".join([a["group"], a["artifact"]])])
 
-    if repository_ctx.attr.fetch_sources:
+    if fetch_sources:
         cmd.append("--sources")
         cmd.append("--default=true")
 
     environment = {}
     coursier_cache_location = get_coursier_cache_or_default(
         repository_ctx.os.environ,
-        repository_ctx.attr.use_unsafe_shared_cache,
+        use_unsafe_shared_cache,
     )
-    if not repository_ctx.attr.use_unsafe_shared_cache:
+    if not use_unsafe_shared_cache:
         cmd.extend(["--cache", coursier_cache_location])  # Download into $output_base/external/$maven_repo_name/v1
 
         # If not using the shared cache and the user did not specify a COURSIER_CACHE, set the default
@@ -538,18 +518,98 @@ def _coursier_fetch_impl(repository_ctx):
         # https://github.com/coursier/coursier/blob/1cbbf39b88ee88944a8d892789680cdb15be4714/modules/paths/src/main/java/coursier/paths/CoursierPaths.java#L29-L56
         environment = {"COURSIER_CACHE": str(repository_ctx.path(coursier_cache_location))}
 
-    repository_ctx.report_progress("Resolving and fetching the transitive closure of %s artifact(s).." % len(artifact_coordinates))
-    exec_result = repository_ctx.execute(cmd, timeout = repository_ctx.attr.resolve_timeout, environment = environment)
+    repository_ctx.report_progress(
+        "%sResolving and fetching the transitive closure of %s artifact(s).." % (
+            report_progress_prefix, len(artifact_coordinates)))
+    exec_result = repository_ctx.execute(cmd, timeout = timeout, environment = environment)
     if (exec_result.return_code != 0):
         fail("Error while fetching artifact with coursier: " + exec_result.stderr)
 
+    return _deduplicate_artifacts(json_parse(repository_ctx.read(repository_ctx.path(
+        "dep-tree.json"))))
+
+
+def _coursier_fetch_impl(repository_ctx):
+    # Not using maven_install.json, so we resolve and fetch from scratch.
+    # This takes significantly longer as it doesn't rely on any local
+    # caches and uses Coursier's own download mechanisms.
+
+    # Download Coursier's standalone (deploy) jar from Maven repositories.
+    repository_ctx.download([
+        COURSIER_CLI_GITHUB_ASSET_URL,
+        COURSIER_CLI_BAZEL_MIRROR_URL,
+    ], "coursier", sha256 = COURSIER_CLI_SHA256, executable = True)
+
+    # Try running coursier once
+    exec_result = repository_ctx.execute(
+        _generate_java_jar_command(repository_ctx, repository_ctx.path("coursier")),
+    )
+    if exec_result.return_code != 0:
+        fail("Unable to run coursier: " + exec_result.stderr)
+
+    _windows_check(repository_ctx)
+
+    # Deserialize the spec blobs
+    repositories = []
+    for repository in repository_ctx.attr.repositories:
+        repositories.append(json_parse(repository))
+
+    artifacts = []
+    for a in repository_ctx.attr.artifacts:
+        artifacts.append(json_parse(a))
+
+    excluded_artifacts = []
+    for a in repository_ctx.attr.excluded_artifacts:
+        excluded_artifacts.append(json_parse(a))
+
     # Once coursier finishes a fetch, it generates a tree of artifacts and their
     # transitive dependencies in a JSON file. We use that as the source of truth
     # to generate the repository's BUILD file.
     #
     # Coursier generates duplicate artifacts sometimes. Deduplicate them using
     # the file name value as the key.
-    dep_tree = _deduplicate_artifacts(json_parse(repository_ctx.read(repository_ctx.path("dep-tree.json"))))
+    dep_tree = make_coursier_dep_tree(
+        repository_ctx,
+        artifacts,
+        excluded_artifacts,
+        repositories,
+        repository_ctx.attr.version_conflict_policy,
+        repository_ctx.attr.fail_on_missing_checksum,
+        repository_ctx.attr.fetch_sources,
+        repository_ctx.attr.use_unsafe_shared_cache,
+        repository_ctx.attr.resolve_timeout,
+    )
+
+    # Fetch all possible jetified artifacts. We will wire them up later.
+    if repository_ctx.attr.jetify:
+        extra_jetify_artifacts = []
+        for artifact in dep_tree["dependencies"]:
+            artifact_coord = parse.parse_maven_coordinate(artifact["coord"])
+            jetify_coord_tuple = jetify_maven_coord(
+                artifact_coord["group"],
+                artifact_coord["artifact"],
+                artifact_coord["version"],
+            )
+            if jetify_coord_tuple:
+                artifact_coord["group"] = jetify_coord_tuple[0]
+                artifact_coord["artifact"] = jetify_coord_tuple[1]
+                artifact_coord["version"] = jetify_coord_tuple[2]
+                extra_jetify_artifacts.append(artifact_coord)
+        dep_tree = make_coursier_dep_tree(
+            repository_ctx,
+            # Order is important due to version conflict resolution. "pinned" will take the last
+            # version that is provided so having the explicit artifacts last makes those versions
+            # stick.
+            extra_jetify_artifacts + artifacts,
+            excluded_artifacts,
+            repositories,
+            repository_ctx.attr.version_conflict_policy,
+            repository_ctx.attr.fail_on_missing_checksum,
+            repository_ctx.attr.fetch_sources,
+            repository_ctx.attr.use_unsafe_shared_cache,
+            repository_ctx.attr.resolve_timeout,
+            report_progress_prefix = "Second pass for Jetified Artifacts: ",
+        )
 
     # Reconstruct the original URLs from the relative path to the artifact,
     # which encodes the URL components for the protocol, domain, and path to
@@ -560,13 +620,22 @@ def _coursier_fetch_impl(repository_ctx):
         repository_ctx.path(repository_ctx.attr._sha256_hasher),
     )
     files_to_hash = []
+    jetify_include_dict = {k: None for k in repository_ctx.attr.jetify_include_list}
+    jetify_all = repository_ctx.attr.jetify and repository_ctx.attr.jetify_include_list == JETIFY_INCLUDE_LIST_JETIFY_ALL
 
     for artifact in dep_tree["dependencies"]:
         # Some artifacts don't contain files; they are just parent artifacts
         # to other artifacts.
         if artifact["file"] == None:
             continue
 
+        coord_split = artifact["coord"].split(":")
+        coord_unversioned = "{}:{}".format(coord_split[0], coord_split[1])
+        should_jetify = jetify_all or (repository_ctx.attr.jetify and coord_unversioned in jetify_include_dict)
+        if should_jetify :
+            artifact["directDependencies"] = jetify_artifact_dependencies(artifact["directDependencies"])
+            artifact["dependencies"] = jetify_artifact_dependencies(artifact["dependencies"])
+
         # Normalize paths in place here.
         artifact.update({"file": _normalize_to_unix_path(artifact["file"])})
 

private/dependency_tree_parser.bzl

@@ -177,9 +177,10 @@ def _generate_imports(repository_ctx, dep_tree, explicit_artifacts, neverlink_ar
             # Dedupe dependencies here. Sometimes coursier will return "x.y:z:aar:version" and "x.y:z:version" in the
             # same list of dependencies.
             target_import_labels = []
-            for dep in artifact["dependencies"]:
+            for dep in artifact["directDependencies"]:
                 if get_packaging(dep) == "json":
                     continue
+                stripped_dep = strip_packaging_and_classifier_and_version(dep)
                 dep_target_label = escape(strip_packaging_and_classifier_and_version(dep))
 
                 # Coursier returns cyclic dependencies sometimes. Handle it here.

private/rules/jetifier.bzl

@@ -1,3 +1,5 @@
+load("//:specs.bzl", "maven", "parse")
+load(":jetifier_maven_map.bzl", "jetifier_maven_map")
 load(":jvm_import.bzl", "jvm_import")
 
 def _jetify_impl(ctx):
@@ -57,3 +59,37 @@ def jetify_jvm_import(name, jars, **kwargs):
         jars = [":jetified_" + name],
         **kwargs
     )
+
+def jetify_maven_coord(group, artifact, version):
+    """
+    Looks up support -> androidx artifact mapping, returns None if no mapping found.
+    """
+    if (group, artifact) not in jetifier_maven_map:
+        return None
+
+    return jetifier_maven_map[(group, artifact)].get(version, None)
+
+def jetify_artifact_dependencies(deps):
+    """Takes in list of maven coordinates and returns a list of jetified maven coordinates"""
+    ret = []
+    for coord_str in deps:
+        artifact = parse.parse_maven_coordinate(coord_str)
+        jetify_coord_tuple = jetify_maven_coord(
+            artifact["group"],
+            artifact["artifact"],
+            artifact["version"],
+        )
+        if jetify_coord_tuple:
+            artifact["group"] = jetify_coord_tuple[0]
+            artifact["artifact"] = jetify_coord_tuple[1]
+            artifact["version"] = jetify_coord_tuple[2]
+            ret.append("{}:{}{}{}:{}".format(
+                artifact["group"],
+                artifact["artifact"],
+                (":" + artifact["packaging"]) if "packaging" in artifact else "",
+                (":" + artifact["classifier"]) if "classifier" in artifact else "",
+                artifact["version"],
+            ))
+        else:
+            ret.append(coord_str)
+    return ret