From 38e4fb2ce65fb3503b527f1457587c6c06c15335 Mon Sep 17 00:00:00 2001
From: Jaewoong Jung <jungjw@google.com>
Date: Wed, 12 Dec 2018 09:01:34 -0800
Subject: [PATCH] Add buildDir to java patch-module paths.

This solves a problem where javac fails to find patch-module targets
when a build diretory is not under the project root.

Bug: 117770838
Test: java_test.go + manual build
Change-Id: If70d0d388a3f6c87b3f78a927df2063616d50c8a
---
 android/module.go | 13 ++++++++++
 java/java.go      | 11 +++++++--
 java/java_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/android/module.go b/android/module.go
index dc0c85603..d2f84ce52 100644
--- a/android/module.go
+++ b/android/module.go
@@ -190,6 +190,7 @@ type Module interface {
 	GetProperties() []interface{}
 
 	BuildParamsForTests() []BuildParams
+	VariablesForTests() map[string]string
 }
 
 type nameProperties struct {
@@ -473,6 +474,7 @@ type ModuleBase struct {
 
 	// For tests
 	buildParams []BuildParams
+	variables   map[string]string
 
 	prefer32 func(ctx BaseModuleContext, base *ModuleBase, class OsClass) bool
 }
@@ -489,6 +491,10 @@ func (a *ModuleBase) BuildParamsForTests() []BuildParams {
 	return a.buildParams
 }
 
+func (a *ModuleBase) VariablesForTests() map[string]string {
+	return a.variables
+}
+
 func (a *ModuleBase) Prefer32(prefer32 func(ctx BaseModuleContext, base *ModuleBase, class OsClass) bool) {
 	a.prefer32 = prefer32
 }
@@ -781,6 +787,7 @@ func (a *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext)
 		installDeps:            a.computeInstallDeps(blueprintCtx),
 		installFiles:           a.installFiles,
 		missingDeps:            blueprintCtx.GetMissingDependencies(),
+		variables:              make(map[string]string),
 	}
 
 	desc := "//" + ctx.ModuleDir() + ":" + ctx.ModuleName() + " "
@@ -842,6 +849,7 @@ func (a *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext)
 	}
 
 	a.buildParams = ctx.buildParams
+	a.variables = ctx.variables
 }
 
 type androidBaseContextImpl struct {
@@ -864,6 +872,7 @@ type androidModuleContext struct {
 
 	// For tests
 	buildParams []BuildParams
+	variables   map[string]string
 }
 
 func (a *androidModuleContext) ninjaError(desc string, outputs []string, err error) {
@@ -928,6 +937,10 @@ func convertBuildParams(params BuildParams) blueprint.BuildParams {
 }
 
 func (a *androidModuleContext) Variable(pctx PackageContext, name, value string) {
+	if a.config.captureBuild {
+		a.variables[name] = value
+	}
+
 	a.ModuleContext.Variable(pctx.PackageContext, name, value)
 }
 
diff --git a/java/java.go b/java/java.go
index 71967a867..c02ccd347 100644
--- a/java/java.go
+++ b/java/java.go
@@ -1011,8 +1011,15 @@ func (j *Module) collectBuilderFlags(ctx android.ModuleContext, deps deps) javaB
 	}
 
 	if j.properties.Patch_module != nil && flags.javaVersion == "1.9" {
-		patchClasspath := ".:" + flags.classpath.FormJavaClassPath("")
-		javacFlags = append(javacFlags, "--patch-module="+String(j.properties.Patch_module)+"="+patchClasspath)
+		// Manually specify build directory in case it is not under the repo root.
+		// (javac doesn't seem to expand into symbolc links when searching for patch-module targets, so
+		// just adding a symlink under the root doesn't help.)
+		patchPaths := ".:" + ctx.Config().BuildDir()
+		classPath := flags.classpath.FormJavaClassPath("")
+		if classPath != "" {
+			patchPaths += ":" + classPath
+		}
+		javacFlags = append(javacFlags, "--patch-module="+String(j.properties.Patch_module)+"="+patchPaths)
 	}
 
 	// systemModules
diff --git a/java/java_test.go b/java/java_test.go
index 4d4b83618..2a54f699c 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -1185,3 +1185,65 @@ func TestCompilerFlags(t *testing.T) {
 		}
 	}
 }
+
+// TODO(jungjw): Consider making this more robust by ignoring path order.
+func checkPatchModuleFlag(t *testing.T, ctx *android.TestContext, moduleName string, expected string) {
+	variables := ctx.ModuleForTests(moduleName, "android_common").Module().VariablesForTests()
+	flags := strings.Split(variables["javacFlags"], " ")
+	got := ""
+	for _, flag := range flags {
+		keyEnd := strings.Index(flag, "=")
+		if keyEnd > -1 && flag[:keyEnd] == "--patch-module" {
+			got = flag[keyEnd+1:]
+			break
+		}
+	}
+	if expected != got {
+		t.Errorf("Unexpected patch-module flag for module %q - expected %q, but got %q", moduleName, expected, got)
+	}
+}
+
+func TestPatchModule(t *testing.T) {
+	bp := `
+		java_library {
+			name: "foo",
+			srcs: ["a.java"],
+		}
+
+		java_library {
+			name: "bar",
+			srcs: ["b.java"],
+			no_standard_libs: true,
+			system_modules: "none",
+			patch_module: "java.base",
+		}
+
+		java_library {
+			name: "baz",
+			srcs: ["c.java"],
+			patch_module: "java.base",
+		}
+	`
+
+	t.Run("1.8", func(t *testing.T) {
+		// Test default javac 1.8
+		ctx := testJava(t, bp)
+
+		checkPatchModuleFlag(t, ctx, "foo", "")
+		checkPatchModuleFlag(t, ctx, "bar", "")
+		checkPatchModuleFlag(t, ctx, "baz", "")
+	})
+
+	t.Run("1.9", func(t *testing.T) {
+		// Test again with javac 1.9
+		config := testConfig(map[string]string{"EXPERIMENTAL_USE_OPENJDK9": "true"})
+		ctx := testContext(config, bp, nil)
+		run(t, ctx, config)
+
+		checkPatchModuleFlag(t, ctx, "foo", "")
+		expected := "java.base=.:" + buildDir
+		checkPatchModuleFlag(t, ctx, "bar", expected)
+		expected = "java.base=" + strings.Join([]string{".", buildDir, moduleToPath("ext"), moduleToPath("framework")}, ":")
+		checkPatchModuleFlag(t, ctx, "baz", expected)
+	})
+}