AGP 8.7.0 - Lint False Positive RestrictedApi on NavOptionsBuilder.popUpTo [370694831]

Fixed

Bug

Status Update

No update yet.

Description

ey...@gmail.com

created issue #1

Oct 1, 2024 07:15PM

DESCRIBE THE ISSUE IN DETAIL:

After updating to 8.7.0 I am getting a false positive RestrictedApi error saying that I cannot call NavOptionsBuilder.popUpTo because it is restricted to the library scope.

My call:

navController.navigate(Routes.Onboarding) {
  popUpTo(navController.rootRoute()) {
    inclusive = true
  }
}

popUpTo resolves to:

@Suppress("BuilderSetStyle", "MissingJvmstatic")
public fun <T : Any> popUpTo(route: T, popUpToBuilder: PopUpToBuilder.() -> Unit = {}) {
    popUpToRouteObject = route
    popUpToId = -1
    popUpToRoute = null
    val builder = PopUpToBuilder().apply(popUpToBuilder)
    inclusive = builder.inclusive
    saveState = builder.saveState
}

which does not have a RestrictTo annotation. There is an overload that does though:

// this restricted public is needed so that the public reified [popUpTo] can call
// private popUpToRouteClass setter
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
public fun <T : Any> popUpTo(klass: KClass<T>, popUpToBuilder: PopUpToBuilder.() -> Unit) {
    popUpToRouteClass = klass
    popUpToId = -1
    popUpToRoute = null
    val builder = PopUpToBuilder().apply(popUpToBuilder)
    inclusive = builder.inclusive
    saveState = builder.saveState
}

Comments

hu...@google.com <hu...@google.com> Oct 1, 2024 09:09PM

Assigned to an...@google.com.

js...@google.com <js...@google.com> #2Oct 2, 2024 06:54AM

Reassigned to js...@google.com.

This would really help with performance testing of Compose

ey...@gmail.com <ey...@gmail.com> #3Oct 2, 2024 11:44AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit a42f3e66b63a9303bdc7350302bbd83fc9eb77b0
Author: Chris Craik <ccraik@google.com>
Date: Fri Jun 19 14:25:22 2020

Enable profileable for all benchmarks, and benchmark tests

Test: ./gradlew benchmark:b-b:cC
Bug: 158303822

This is a prerequisite for simpleperf on user builds.

Change-Id: Idcb0024aad12d56edcf36427fa65235b3eab74ec

M ads/ads-identifier-benchmark/src/androidTest/AndroidManifest.xml
M appcompat/appcompat-benchmark/src/androidTest/AndroidManifest.xml
M benchmark/benchmark/src/androidTest/AndroidManifest.xml
M benchmark/integration-tests/startup-benchmark/src/androidTest/AndroidManifest.xml
M collection/collection-benchmark/src/androidTest/AndroidManifest.xml
M compose/compose-runtime/compose-runtime-benchmark/src/androidTest/AndroidManifest.xml
M navigation/benchmark/src/androidTest/AndroidManifest.xml
M recyclerview/recyclerview-benchmark/src/androidTest/AndroidManifest.xml
M room/benchmark/src/androidTest/AndroidManifest.xml
M slices/benchmark/src/androidTest/AndroidManifest.xml
M ui/integration-tests/benchmark/src/androidTest/AndroidManifest.xml
M work/workmanager-benchmark/src/androidTest/AndroidManifest.xml

https://android-review.googlesource.com/1344979

js...@google.com <js...@google.com> #4Oct 4, 2024 06:20AM

Accepted by js...@google.com.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit dfae3689d8dd651c25cbd2bd42393fc402503cb9
Author: Chris Craik <ccraik@google.com>
Date: Wed Jun 17 11:32:08 2020

Create Profiler abstraction

Bug: 158303822
Test: ProfilerTest
Test: ./gradlew benchmark:b-c:cC benchmark:b-j:cC

Allows us to factor significant functionality out of BenchmarkState,
and will let us support additional profiling modes (simpleperf!).

Additionally, renames all tracing params for clarity, while
maintaining backwards compatibility.

Change-Id: Id5b3fb034638077a97c9d30c7f9f12f0ef31102c

M benchmark/common/api/1.1.0-alpha02.txt
M benchmark/common/api/current.txt
M benchmark/common/api/public_plus_experimental_1.1.0-alpha02.txt
M benchmark/common/api/public_plus_experimental_current.txt
M benchmark/common/api/restricted_1.1.0-alpha02.txt
M benchmark/common/api/restricted_current.txt
M benchmark/common/src/androidTest/java/androidx/benchmark/ArgumentInjectingApplication.kt
A benchmark/common/src/androidTest/java/androidx/benchmark/ProfilerTest.kt
D benchmark/common/src/androidTest/java/androidx/benchmark/ProfilingModeTest.kt
M benchmark/common/src/main/java/androidx/benchmark/Arguments.kt
M benchmark/common/src/main/java/androidx/benchmark/BenchmarkState.kt
M benchmark/common/src/main/java/androidx/benchmark/Errors.kt
A benchmark/common/src/main/java/androidx/benchmark/Profiler.kt
D benchmark/common/src/main/java/androidx/benchmark/ProfilingMode.kt

https://android-review.googlesource.com/1350809

js...@google.com <js...@google.com> Oct 4, 2024 03:24PM

Marked as fixed.

an...@google.com <an...@google.com> #5Oct 14, 2024 09:08PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 42d424831e41ad853d8d4ee3e0c7e83302a181f6
Author: Chris Craik <ccraik@google.com>
Date: Wed Jun 17 11:32:08 2020

Simpleperf method sampling - initial support

Bug: 158303822
Test: ./gradlew benchmark:b-c:cC benchmark:b-j:cC
Test: ProfilerTest

Remaining simpleperf work to do:
- Additionally spit out .trace file for opening in Studio
- Use additionalOutputDir
- Enable this mode by default on supported devices (when methodSampling is selected)
- Avoid need for python prepare script (can just call ADB directly)

Additionally adds args for sample freq, and sampling duration.

The 'simpleperf' package is copied from the simpleperf app_api sample:

https://cs.android.com/android/platform/superproject/+/master:system/extras/simpleperf/app_api/

Change-Id: I5ccef4b434710ae6561c63c5ae1cd5d87ed77b9f

M benchmark/common/src/androidTest/java/androidx/benchmark/ProfilerTest.kt
M benchmark/common/src/main/java/androidx/benchmark/Arguments.kt
M benchmark/common/src/main/java/androidx/benchmark/BenchmarkState.kt
M benchmark/common/src/main/java/androidx/benchmark/Errors.kt
M benchmark/common/src/main/java/androidx/benchmark/Profiler.kt
A benchmark/common/src/main/java/androidx/benchmark/simpleperf/ProfileSession.java
A benchmark/common/src/main/java/androidx/benchmark/simpleperf/RecordOptions.java

https://android-review.googlesource.com/1343923

ey...@gmail.com <ey...@gmail.com> #6Oct 20, 2024 09:18AM

Note that androidx.benchmark.ProfilerTest#methodSamplingSimpleperf has been 100% failing in CI https://android-build.googleplex.com/builds/tests/view?invocationId=I87700005330476092&testResultId=TR24710817326417073

java.lang.Error: linux perf events aren't enabled on the device. Please run api_profiler.py.
	at androidx.benchmark.simpleperf.ProfileSession.checkIfPerfEnabled(ProfileSession.java:296)
	at androidx.benchmark.simpleperf.ProfileSession.startRecording(ProfileSession.java:142)
	at androidx.benchmark.simpleperf.ProfileSession.startRecording(ProfileSession.java:125)
	at androidx.benchmark.MethodSamplingSimpleperf.start(Profiler.kt:174)
	at androidx.benchmark.ProfilerTest.verifyProfiler(ProfilerTest.kt:56)
	at androidx.benchmark.ProfilerTest.methodSamplingSimpleperf(ProfilerTest.kt:80)
	at java.lang.reflect.Method.invoke(Native Method)

js...@google.com <js...@google.com> #7Oct 20, 2024 05:44PM

With the patch in #5, simpleperf is supported, but it's cumbersome, and doesn't support outputting a file that Studio can read. A few things remain to make this easier to use, so we can ship externally:

1) Create Studio .trace file

Simpleperf docs explain how to convert the output <name>.data file output by simpeperf on device to a .trace file Studio can open. See Simpleperf docs on creating .trace file. That simpleperf executable is currently used by ProfileSession.java, so this would just be extra functionality there.

Currently, the trace file is output to /data/data/<package>/simpleperf_data/***.data. We can leave that file there for now, so that it can be copied by simpleperf/scripts/api_profiler.py collect, but we should put a copy of it in additionalTestOutputDir/.

2) Replicate api_profiler.py prepare functionality

See http://go/androidx-bench-dev#initial-setup, specifically api_profiler.py Looks like it just runs a bunch of ADB setprop commands, so should be fairly straightforward. It does also copy an updated version of simpleperf onto the device, but since running that version isn't supported on user builds (it has to be build into system image), we'll skip that part.

3) Switch MethodTracing to use simpleperf by default on supported devices

Once the above 2 are resolved, rename MethodTracing -> MethodTracingLegacy, and MethodTracingSimpleperf -> MethodTracing.

Use simpleperf by default on API 28+, but let people opt into the old version by passing "MethodTracingLegacy".

js...@google.com <js...@google.com> #8Oct 21, 2024 06:08PM

@Aurimas - that failure is because this functionality requires running a host-side script before it will work, but fixing 2) above will fix that.

I marked the test @FlakyTest, but if there's a better way to make it not run at all in pre/postsubmit, happy to make that change.

an...@google.com <an...@google.com> #9Oct 31, 2024 07:41PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 04e4b033916342deb8dc343f5dacc41f12597d0d
Author: Louis Pullen-Freilich <lpf@google.com>
Date: Thu Aug 20 00:39:26 2020

Changes ProfilerTest#methodSamplingSimpleperf to be @Ignore instead of @FlakyTest

Since this test fails consistently, there's no value to running it in postsubmit, as it just pollutes the testing dashboard.

Bug: 158303822
Test: ProfilerTest
Change-Id: I0c9f1ffd57d0058816b838e75502807f66fef7ed

M benchmark/common/src/androidTest/java/androidx/benchmark/ProfilerTest.kt

https://android-review.googlesource.com/1404517

du...@gmail.com <du...@gmail.com> #10
Restricted+
Nov 22, 2024 02:37AM

Comment has been deleted.

deleted

Restricted+

0 B

ch...@gmail.com <ch...@gmail.com> #11Jan 2, 2025 04:07PM