From 767dbd9d3b74e341dc89d203b9868568931eaee8 Mon Sep 17 00:00:00 2001
From: Jiyong Park <jiyong@google.com>
Date: Thu, 4 Mar 2021 13:03:10 +0900
Subject: [PATCH] Friendly error message on apex_available and min_sdk_version
 checks

1) suggest a fix at the end of the message
2) add new lines around the dependency path so that they are visually
separated from rest of the error message

Bug: N/A
Test: m with an intentional break
error: bionic/apex/Android.bp:32:1: module "com.android.runtime" variant "android_common_com.android.runtime_image": "libutils_headers" requires "libsystem_headers" that doesn't list the APEX under 'apex_available'.

Dependency path:
           via tag apex.dependencyTag: { name:executable payload:true}
    -> crash_dump{os:android,image:,arch:arm_armv8-a,sdk:,apex:apex10000}
           via tag cc.libraryDependencyTag: { Kind:staticLibraryDependency Order:normalLibraryDependency wholeStatic:false reexportFlags:false explicitlyVersioned:false dataLib:false ndk:false staticUnwinder:false makeSuffix: skipApexAllowedDependenciesCheck:false excludeInApex:false}
    -> libtombstoned_client_static{os:android,image:,arch:arm_armv8-a,sdk:,link:static,apex:apex10000}
           via tag cc.libraryDependencyTag: { Kind:staticLibraryDependency Order:normalLibraryDependency wholeStatic:true reexportFlags:true explicitlyVersioned:false dataLib:false ndk:false staticUnwinder:false makeSuffix: skipApexAllowedDependenciesCheck:false excludeInApex:false}
    -> libcutils{os:android,image:,arch:arm_armv8-a,sdk:,link:static,asan:,apex:apex10000}
           via tag cc.libraryDependencyTag: { Kind:headerLibraryDependency Order:normalLibraryDependency wholeStatic:false reexportFlags:false explicitlyVersioned:false dataLib:false ndk:false staticUnwinder:false makeSuffix: skipApexAllowedDependenciesCheck:false excludeInApex:false}
    -> libutils_headers{os:android,image:,arch:arm_armv8-a,sdk:,asan:,apex:apex10000}
           via tag cc.libraryDependencyTag: { Kind:headerLibraryDependency Order:normalLibraryDependency wholeStatic:false reexportFlags:true explicitlyVersioned:false dataLib:false ndk:false staticUnwinder:false makeSuffix: skipApexAllowedDependenciesCheck:false excludeInApex:false}
    -> libsystem_headers{os:android,image:,arch:arm_armv8-a,sdk:,asan:,apex:apex10000}

Consider adding "com.android.runtime" to 'apex_available' property of "libsystem_headers"

Change-Id: I09f92c3086ea433780133a33ba0ad73baee6dc41
---
 android/apex.go   | 8 ++++++--
 apex/apex.go      | 6 ++++--
 apex/apex_test.go | 2 +-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/android/apex.go b/android/apex.go
index 81f8c867f..a014592ef 100644
--- a/android/apex.go
+++ b/android/apex.go
@@ -849,8 +849,12 @@ func CheckMinSdkVersion(m UpdatableModule, ctx ModuleContext, minSdkVersion ApiL
 		if err := to.ShouldSupportSdkVersion(ctx, minSdkVersion); err != nil {
 			toName := ctx.OtherModuleName(to)
 			if ver, ok := minSdkVersionAllowlist[toName]; !ok || ver.GreaterThan(minSdkVersion) {
-				ctx.OtherModuleErrorf(to, "should support min_sdk_version(%v) for %q: %v. Dependency path: %s",
-					minSdkVersion, ctx.ModuleName(), err.Error(), ctx.GetPathString(false))
+				ctx.OtherModuleErrorf(to, "should support min_sdk_version(%v) for %q: %v."+
+					"\n\nDependency path: %s\n\n"+
+					"Consider adding 'min_sdk_version: %q' to %q",
+					minSdkVersion, ctx.ModuleName(), err.Error(),
+					ctx.GetPathString(false),
+					minSdkVersion, ctx.ModuleName())
 				return false
 			}
 		}
diff --git a/apex/apex.go b/apex/apex.go
index 662bbbd0b..65260cc00 100644
--- a/apex/apex.go
+++ b/apex/apex.go
@@ -2246,8 +2246,10 @@ func (a *apexBundle) checkApexAvailability(ctx android.ModuleContext) {
 		if to.AvailableFor(apexName) || baselineApexAvailable(apexName, toName) {
 			return true
 		}
-		ctx.ModuleErrorf("%q requires %q that doesn't list the APEX under 'apex_available'. Dependency path:%s",
-			fromName, toName, ctx.GetPathString(true))
+		ctx.ModuleErrorf("%q requires %q that doesn't list the APEX under 'apex_available'."+
+			"\n\nDependency path:%s\n\n"+
+			"Consider adding %q to 'apex_available' property of %q",
+			fromName, toName, ctx.GetPathString(true), apexName, toName)
 		// Visit this module's dependencies to check and report any issues with their availability.
 		return true
 	})
diff --git a/apex/apex_test.go b/apex/apex_test.go
index 3f5604741..c90b92806 100644
--- a/apex/apex_test.go
+++ b/apex/apex_test.go
@@ -5184,7 +5184,7 @@ func TestApexAvailable_DirectDep(t *testing.T) {
 
 func TestApexAvailable_IndirectDep(t *testing.T) {
 	// libbbaz is an indirect dep
-	testApexError(t, `requires "libbaz" that doesn't list the APEX under 'apex_available'. Dependency path:
+	testApexError(t, `requires "libbaz" that doesn't list the APEX under 'apex_available'.\n\nDependency path:
 .*via tag apex\.dependencyTag.*name:sharedLib.*
 .*-> libfoo.*link:shared.*
 .*via tag cc\.libraryDependencyTag.*Kind:sharedLibraryDependency.*