From 6a20eeda429406b90219ee3e906aca7b8cb8bca9 Mon Sep 17 00:00:00 2001
From: Remi NGUYEN VAN <reminv@google.com>
Date: Mon, 31 Jan 2022 11:21:08 +0900
Subject: [PATCH] Reorganize connectivity framework dependencies

Allow framework-connectivity to depend on framework-connectivity-t
stubs, and framework-connectivity-t to depend on prebuilt (to avoid
circular dependencies) framework-connectivity stubs to compile its own
stubs, and framework-connectivity.impl to compile its implementation.

Also reorganize jarjar rules so that service and framework jar can use
static libraries in framework-connectivity without packaging their own,
reducing duplicate code.

Bug: 204830222
Test: m
Change-Id: I75c34986e7c479de23cdb2e9b360fa1fede018c9
---
 Tethering/apex/Android.bp                     |  2 +-
 framework-t/Android.bp                        | 27 ++++++++++--
 framework-t/jarjar-rules.txt                  |  1 -
 framework/Android.bp                          | 42 +++++++++++++++----
 framework/jarjar-rules.txt                    |  3 --
 service-t/Android.bp                          |  8 ++--
 service/Android.bp                            | 12 +++---
 service/jarjar-rules.txt                      | 16 ++++---
 .../com_android_net_module_util/onload.cpp    |  4 +-
 tests/integration/Android.bp                  |  2 +-
 10 files changed, 82 insertions(+), 35 deletions(-)
 delete mode 100644 framework-t/jarjar-rules.txt
 delete mode 100644 framework/jarjar-rules.txt

diff --git a/Tethering/apex/Android.bp b/Tethering/apex/Android.bp
index 721f05e184..ea3f8d63b8 100644
--- a/Tethering/apex/Android.bp
+++ b/Tethering/apex/Android.bp
@@ -52,7 +52,7 @@ apex {
         first: {
             jni_libs: [
                 "libservice-connectivity",
-                "libcom_android_connectivity_com_android_net_module_util_jni",
+                "libandroid_net_connectivity_com_android_net_module_util_jni",
             ],
             native_shared_libs: ["libnetd_updatable"],
         },
diff --git a/framework-t/Android.bp b/framework-t/Android.bp
index 8d621af30f..a6700e518e 100644
--- a/framework-t/Android.bp
+++ b/framework-t/Android.bp
@@ -26,12 +26,13 @@ java_defaults {
 // The above defaults can be used to disable framework-connectivity t
 // targets while minimizing merge conflicts in the build rules.
 
-
+// SDK library for connectivity bootclasspath classes that were part of the non-updatable API before
+// T, and were moved to the module in T. Other bootclasspath classes in connectivity should go to
+// framework-connectivity.
 java_sdk_library {
     name: "framework-connectivity-tiramisu",
     sdk_version: "module_current",
     min_sdk_version: "Tiramisu",
-    jarjar_rules: "jarjar-rules.txt",
     defaults: [
         "framework-module-defaults",
         "enable-framework-connectivity-t-targets",
@@ -40,13 +41,31 @@ java_sdk_library {
         ":framework-connectivity-tiramisu-updatable-sources",
         ":framework-nearby-java-sources",
     ],
-    static_libs: [
-        "modules-utils-preconditions",
+    // Do not add static_libs to this library: put them in framework-connectivity instead.
+    // The jarjar rules are only so that references to jarjared utils in
+    // framework-connectivity-pre-jarjar match at runtime.
+    jarjar_rules: ":connectivity-jarjar-rules",
+    stub_only_libs: [
+        // Use prebuilt framework-connectivity stubs to avoid circular dependencies
+        "sdk_module-lib_current_framework-connectivity",
     ],
     libs: [
         "unsupportedappusage",
         "app-compat-annotations",
+        "sdk_module-lib_current_framework-connectivity",
+    ],
+    impl_only_libs: [
+        // The build system will use framework-bluetooth module_current stubs, because
+        // of sdk_version: "module_current" above.
         "framework-bluetooth",
+        // Compile against the entire implementation of framework-connectivity,
+        // including hidden methods. This is safe because if framework-connectivity-t is
+        // on the bootclasspath (i.e., T), then framework-connectivity is also on the
+        // bootclasspath (because it shipped in S).
+        //
+        // This compiles against the pre-jarjar target so that this code can use
+        // non-jarjard names of widely-used packages such as com.android.net.module.util.
+        "framework-connectivity-pre-jarjar",
     ],
     permitted_packages: [
         "android.net",
diff --git a/framework-t/jarjar-rules.txt b/framework-t/jarjar-rules.txt
deleted file mode 100644
index 0b3621578d..0000000000
--- a/framework-t/jarjar-rules.txt
+++ /dev/null
@@ -1 +0,0 @@
-rule com.android.internal.util.** com.android.connectivity.tiramisu.@0
diff --git a/framework/Android.bp b/framework/Android.bp
index de505c7905..1545264646 100644
--- a/framework/Android.bp
+++ b/framework/Android.bp
@@ -55,12 +55,11 @@ filegroup {
     ],
 }
 
-java_sdk_library {
-    name: "framework-connectivity",
+java_defaults {
+    name: "framework-connectivity-defaults",
+    defaults: ["framework-module-defaults"],
     sdk_version: "module_current",
     min_sdk_version: "30",
-    defaults: ["framework-module-defaults"],
-    installable: true,
     srcs: [
         ":framework-connectivity-sources",
         ":net-utils-framework-common-srcs",
@@ -76,6 +75,9 @@ java_sdk_library {
             "frameworks/native/aidl/binder", // For PersistableBundle.aidl
         ],
     },
+    stub_only_libs: [
+        "framework-connectivity-tiramisu.stubs.module_lib",
+    ],
     impl_only_libs: [
         "framework-tethering.stubs.module_lib",
         "framework-wifi.stubs.module_lib",
@@ -83,16 +85,42 @@ java_sdk_library {
     ],
     static_libs: [
         "modules-utils-build",
+        "modules-utils-preconditions",
     ],
     libs: [
+        "framework-connectivity-tiramisu.stubs.module_lib",
         "unsupportedappusage",
     ],
-    jarjar_rules: "jarjar-rules.txt",
+    apex_available: [
+        "com.android.tethering",
+    ],
+    lint: { strict_updatability_linting: true },
+}
+
+java_library {
+    name: "framework-connectivity-pre-jarjar",
+    defaults: ["framework-connectivity-defaults"],
+    libs: [
+        // This cannot be in the defaults clause above because if it were, it would be used
+        // to generate the connectivity stubs. That would create a circular dependency
+        // because the tethering stubs depend on the connectivity stubs (e.g.,
+        // TetheringRequest depends on LinkAddress).
+        "framework-tethering.stubs.module_lib",
+    ],
+    visibility: ["//packages/modules/Connectivity:__subpackages__"]
+}
+
+java_sdk_library {
+    name: "framework-connectivity",
+    defaults: ["framework-connectivity-defaults"],
+    installable: true,
+    jarjar_rules: ":connectivity-jarjar-rules",
     permitted_packages: ["android.net"],
     impl_library_visibility: [
         "//packages/modules/Connectivity/Tethering/apex",
         // In preparation for future move
         "//packages/modules/Connectivity/apex",
+        "//packages/modules/Connectivity/framework-t",
         "//packages/modules/Connectivity/service",
         "//packages/modules/Connectivity/service-t",
         "//frameworks/base/packages/Connectivity/service",
@@ -111,10 +139,6 @@ java_sdk_library {
         "//packages/modules/NetworkStack/tests:__subpackages__",
         "//packages/modules/Wifi/service/tests/wifitests",
     ],
-    apex_available: [
-        "com.android.tethering",
-    ],
-    lint: { strict_updatability_linting: true },
 }
 
 cc_library_shared {
diff --git a/framework/jarjar-rules.txt b/framework/jarjar-rules.txt
deleted file mode 100644
index ae6b9c3337..0000000000
--- a/framework/jarjar-rules.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-rule com.android.net.module.util.** android.net.connectivity.framework.util.@1
-rule com.android.modules.utils.** android.net.connectivity.framework.modules.utils.@1
-rule android.net.NetworkFactory* android.net.connectivity.framework.NetworkFactory@1
diff --git a/service-t/Android.bp b/service-t/Android.bp
index 48c74c688d..f33be63452 100644
--- a/service-t/Android.bp
+++ b/service-t/Android.bp
@@ -36,15 +36,17 @@ java_library {
     ],
     libs: [
         "framework-annotations-lib",
-        "framework-connectivity.impl",
+        "framework-connectivity-pre-jarjar",
         "framework-connectivity-tiramisu.impl",
         "service-connectivity-pre-jarjar",
         "unsupportedappusage",
     ],
     static_libs: [
-        "modules-utils-build",
+        // Do not add static_libs here if they are already included in framework-connectivity
+        // or in service-connectivity. They are not necessary (included via
+        // service-connectivity-pre-jarjar), and in the case of code that is already in
+        // framework-connectivity, the classes would be included in the apex twice.
         "modules-utils-statemachine",
-        "net-utils-framework-common",
     ],
     apex_available: [
         "com.android.tethering",
diff --git a/service/Android.bp b/service/Android.bp
index ef969ac747..6d187ba67e 100644
--- a/service/Android.bp
+++ b/service/Android.bp
@@ -20,9 +20,9 @@ package {
 }
 
 // The library name match the service-connectivity jarjar rules that put the JNI utils in the
-// com.android.connectivity.com.android.net.module.util package.
+// android.net.connectivity.com.android.net.module.util package.
 cc_library_shared {
-    name: "libcom_android_connectivity_com_android_net_module_util_jni",
+    name: "libandroid_net_connectivity_com_android_net_module_util_jni",
     min_sdk_version: "30",
     cflags: [
         "-Wall",
@@ -98,20 +98,20 @@ java_library {
     ],
     libs: [
         "framework-annotations-lib",
-        "framework-connectivity.impl",
+        "framework-connectivity-pre-jarjar",
         "framework-tethering.stubs.module_lib",
         "framework-wifi.stubs.module_lib",
         "unsupportedappusage",
         "ServiceConnectivityResources",
     ],
     static_libs: [
+        // Do not add libs here if they are already included
+        // in framework-connectivity
         "dnsresolver_aidl_interface-V9-java",
-        "modules-utils-build",
         "modules-utils-shell-command-handler",
         "net-utils-device-common",
         "net-utils-device-common-bpf",
         "net-utils-device-common-netlink",
-        "net-utils-framework-common",
         "netd-client",
         "networkstack-client",
         "PlatformProperties",
@@ -159,7 +159,7 @@ java_library {
         "service-connectivity-tiramisu-pre-jarjar",
         "service-nearby",
     ],
-    jarjar_rules: "jarjar-rules.txt",
+    jarjar_rules: ":connectivity-jarjar-rules",
     apex_available: [
         "com.android.tethering",
     ],
diff --git a/service/jarjar-rules.txt b/service/jarjar-rules.txt
index f658a5e18f..e3b26fd928 100644
--- a/service/jarjar-rules.txt
+++ b/service/jarjar-rules.txt
@@ -1,6 +1,15 @@
+# Classes in framework-connectivity are restricted to the android.net package.
+# This cannot be changed because it is harcoded in ART in S.
+# Any missing jarjar rule for framework-connectivity would be caught by the
+# build as an unexpected class outside of the android.net package.
+rule com.android.net.module.util.** android.net.connectivity.@0
+rule com.android.modules.utils.** android.net.connectivity.@0
+rule android.net.NetworkFactory* android.net.connectivity.@0
+
+# From modules-utils-preconditions
+rule com.android.internal.util.Preconditions* android.net.connectivity.@0
+
 rule android.sysprop.** com.android.connectivity.@0
-rule com.android.net.module.util.** com.android.connectivity.@0
-rule com.android.modules.utils.** com.android.connectivity.@0
 
 # internal util classes from framework-connectivity-shared-srcs
 rule android.util.LocalLog* com.android.connectivity.@0
@@ -23,9 +32,6 @@ rule android.net.ResolverParamsParcel* com.android.connectivity.@0
 rule android.net.ResolverParamsParcel* com.android.connectivity.@0
 # Also includes netd event listener AIDL, but this is handled by netd-client rules
 
-# From net-utils-device-common
-rule android.net.NetworkFactory* com.android.connectivity.@0
-
 # From netd-client (newer AIDLs should go to android.net.netd.aidl)
 rule android.net.netd.aidl.** com.android.connectivity.@0
 # Avoid including android.net.INetdEventCallback, used in tests but not part of the module
diff --git a/service/jni/com_android_net_module_util/onload.cpp b/service/jni/com_android_net_module_util/onload.cpp
index 07ae31c623..2f09e55864 100644
--- a/service/jni/com_android_net_module_util/onload.cpp
+++ b/service/jni/com_android_net_module_util/onload.cpp
@@ -30,10 +30,10 @@ extern "C" jint JNI_OnLoad(JavaVM* vm, void*) {
     }
 
     if (register_com_android_net_module_util_BpfMap(env,
-            "com/android/connectivity/com/android/net/module/util/BpfMap") < 0) return JNI_ERR;
+            "android/net/connectivity/com/android/net/module/util/BpfMap") < 0) return JNI_ERR;
 
     if (register_com_android_net_module_util_TcUtils(env,
-            "com/android/connectivity/com/android/net/module/util/TcUtils") < 0) return JNI_ERR;
+            "android/net/connectivity/com/android/net/module/util/TcUtils") < 0) return JNI_ERR;
 
     return JNI_VERSION_1_6;
 }
diff --git a/tests/integration/Android.bp b/tests/integration/Android.bp
index 4d4e7b9217..eb7c2d1b03 100644
--- a/tests/integration/Android.bp
+++ b/tests/integration/Android.bp
@@ -53,7 +53,7 @@ android_test {
         // android_library does not include JNI libs: include NetworkStack dependencies here
         "libnativehelper_compat_libc++",
         "libnetworkstackutilsjni",
-        "libcom_android_connectivity_com_android_net_module_util_jni",
+        "libandroid_net_connectivity_com_android_net_module_util_jni",
     ],
     jarjar_rules: ":connectivity-jarjar-rules",
 }
-- 
GitLab