From 38dfa0fa18aaa7c58cce10d37cc045c645786045 Mon Sep 17 00:00:00 2001 From: Ulya Trafimovich Date: Tue, 7 Jan 2020 16:37:02 +0000 Subject: [PATCH] Do not add jacocoagent to framework libraries in static coverage builds. Framework libraries need special handling in static coverage builds: they should not have static dependency on jacoco, otherwise there would be multiple conflicting definitions of the same jacoco classes coming from different bootclasspath jars. This CL does two things: - Move the code that enables instrumentation of framework libraries from AndroidGenerateBuildActions phase to the earlier DepsMutator phase. This is necessary because DepsMutator phase already does some things that depend on the instrumentation flag. - Explicitely exclude framework libraries from those libraries which have static dependency on jacoco. This CL does not fix any apparent build problems: prior to it the framework libraries were not excluded properly, but this was masked by wrong order of checking / setting instrumentation flag. Note that static coverage builds without framework coverage fail to boot, namely this build command: $ build/soong/soong_ui.bash --make-mode \ SKIP_ABI_CHECKS=true \ TARGET_PRODUCT=aosp_walleye TARGET_BUILD_VARIANT=userdebug droid \ EMMA_INSTRUMENT=true \ EMMA_INSTRUMENT_STATIC=true \ NATIVE_COVERAGE=true ..causes the following boot-time errors in logcat: 01-08 12:31:48.670 1252 1252 E System : java.lang.StackOverflowError: stack size 8192KB 01-08 12:31:48.670 1252 1252 E System : at org.jacoco.agent.rt.internal.Offline.$jacocoInit(Unknown Source:13) 01-08 12:31:48.670 1252 1252 E System : at org.jacoco.agent.rt.internal.Offline.getProbes(Unknown Source:0) Also note that static coverage with framework coverage failed to build prior to CL Iaa198b8505aaff36e6685559642ff721637ce55f (dex2oat failed to create boot image due to missing classes). Test: non-static coverage without framework coverage boots: $ build/soong/soong_ui.bash --make-mode \ SKIP_ABI_CHECKS=true \ TARGET_PRODUCT=aosp_walleye TARGET_BUILD_VARIANT=userdebug droid \ EMMA_INSTRUMENT=true \ NATIVE_COVERAGE=true Test: non-static coverage with framework coverage boots: $ build/soong/soong_ui.bash --make-mode \ SKIP_ABI_CHECKS=true \ TARGET_PRODUCT=aosp_walleye TARGET_BUILD_VARIANT=userdebug droid \ EMMA_INSTRUMENT=true \ EMMA_INSTRUMENT_FRAMEWORK=true \ NATIVE_COVERAGE=true Test: static coverage with framework coverage boots: $ build/soong/soong_ui.bash --make-mode \ SKIP_ABI_CHECKS=true \ TARGET_PRODUCT=aosp_walleye TARGET_BUILD_VARIANT=userdebug droid \ EMMA_INSTRUMENT=true \ EMMA_INSTRUMENT_FRAMEWORK=true \ EMMA_INSTRUMENT_STATIC=true \ NATIVE_COVERAGE=true Change-Id: I700f979a5d638ce632f5e8b920b9d0adb3c80248 --- java/java.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/java/java.go b/java/java.go index a48b5a300..051599348 100644 --- a/java/java.go +++ b/java/java.go @@ -651,7 +651,14 @@ func (j *Module) deps(ctx android.BottomUpMutatorContext) { } } - if j.shouldInstrumentStatic(ctx) { + // Framework libraries need special handling in static coverage builds: they should not have + // static dependency on jacoco, otherwise there would be multiple conflicting definitions of + // the same jacoco classes coming from different bootclasspath jars. + if inList(ctx.ModuleName(), config.InstrumentFrameworkModules) { + if ctx.Config().IsEnvTrue("EMMA_INSTRUMENT_FRAMEWORK") { + j.properties.Instrument = true + } + } else if j.shouldInstrumentStatic(ctx) { ctx.AddVariationDependencies(nil, staticLibTag, "jacocoagent") } } @@ -1454,12 +1461,6 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) { j.headerJarFile = j.implementationJarFile } - if ctx.Config().IsEnvTrue("EMMA_INSTRUMENT_FRAMEWORK") { - if inList(ctx.ModuleName(), config.InstrumentFrameworkModules) { - j.properties.Instrument = true - } - } - if j.shouldInstrument(ctx) { outputFile = j.instrument(ctx, flags, outputFile, jarName) }