From 16fe8e1cf108c95ee48776121c4e2d45d9e3fd50 Mon Sep 17 00:00:00 2001 From: Vinh Tran Date: Tue, 16 Aug 2022 16:45:44 -0400 Subject: [PATCH] Fix ProcessBazelQueryResponse of filegroup In mixed builds currently, filegroup doesn't use path prop when creating the paths to the srcs. It defaults to ModuleDir. Hence, when java.genAidlIncludeFlags [1] calls srcFile.Rel() to eventually create the AIDL include dir for AIDL flags, srcFile.Rel() returns the filepath relative to the module directory instead. This CL appends path prop to module dir when creating relativeRoot. This fixes the bridge between converted filegroup that set path prop (e.g. libbinder_aidl) to unconverted module (for example, droidstubs). The fix is needed for the child CL aosp/2186599 to convert libbinder_aidl to Bazel. Without this fix, module-lib-api-stubs-docs-non-updatable (unconverted module that depends on libbinder_aidl) can't be built in mixed builds. [1]: https://cs.android.com/android/platform/superproject/+/master:build/soong/java/gen.go;l=123?q=java%2Fgen.go Test: go test Bug: 243010121 Change-Id: Ic2dd2ab9199c62010303a5b8c611d722f4a4118d --- android/filegroup.go | 15 ++++++---- android/filegroup_test.go | 58 +++++++++++++++++++++++++++++++++++++++ java/java_test.go | 35 +++++++++++++++++++++++ 3 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 android/filegroup_test.go diff --git a/android/filegroup.go b/android/filegroup.go index 9e5769a82..504223de9 100644 --- a/android/filegroup.go +++ b/android/filegroup.go @@ -15,6 +15,7 @@ package android import ( + "path/filepath" "strings" "android/soong/bazel" @@ -164,12 +165,17 @@ func (fg *fileGroup) IsMixedBuildSupported(ctx BaseModuleContext) bool { } func (fg *fileGroup) ProcessBazelQueryResponse(ctx ModuleContext) { - fg.srcs = PathsForModuleSrcExcludes(ctx, fg.properties.Srcs, fg.properties.Exclude_srcs) + bazelCtx := ctx.Config().BazelContext + // This is a short-term solution because we rely on info from Android.bp to handle + // a converted module. This will block when we want to remove Android.bp for all + // converted modules at some point. + // TODO(b/242847534): Implement a long-term solution in which we don't need to rely + // on info form Android.bp for modules that are already converted to Bazel + relativeRoot := ctx.ModuleDir() if fg.properties.Path != nil { - fg.srcs = PathsWithModuleSrcSubDir(ctx, fg.srcs, String(fg.properties.Path)) + relativeRoot = filepath.Join(relativeRoot, *fg.properties.Path) } - bazelCtx := ctx.Config().BazelContext filePaths, err := bazelCtx.GetOutputFiles(fg.GetBazelLabel(ctx, fg), configKey{Common.String(), CommonOS}) if err != nil { ctx.ModuleErrorf(err.Error()) @@ -178,8 +184,7 @@ func (fg *fileGroup) ProcessBazelQueryResponse(ctx ModuleContext) { bazelOuts := make(Paths, 0, len(filePaths)) for _, p := range filePaths { - bazelOuts = append(bazelOuts, PathForBazelOutRelative(ctx, ctx.ModuleDir(), p)) + bazelOuts = append(bazelOuts, PathForBazelOutRelative(ctx, relativeRoot, p)) } - fg.srcs = bazelOuts } diff --git a/android/filegroup_test.go b/android/filegroup_test.go new file mode 100644 index 000000000..a7ea8054c --- /dev/null +++ b/android/filegroup_test.go @@ -0,0 +1,58 @@ +package android + +import ( + "path/filepath" + "testing" +) + +func TestFileGroupWithPathProp(t *testing.T) { + outBaseDir := "outputbase" + pathPrefix := outBaseDir + "/execroot/__main__" + expectedOutputfile := filepath.Join(pathPrefix, "a/b/c/d/test.aidl") + + testCases := []struct { + bp string + rel string + }{ + { + bp: ` + filegroup { + name: "baz", + srcs: ["a/b/c/d/test.aidl"], + path: "a/b", + bazel_module: { label: "//:baz" }, + } +`, + rel: "c/d/test.aidl", + }, + { + bp: ` + filegroup { + name: "baz", + srcs: ["a/b/c/d/test.aidl"], + bazel_module: { label: "//:baz" }, + } +`, + rel: "a/b/c/d/test.aidl", + }, + } + + for _, testCase := range testCases { + outBaseDir := "outputbase" + result := GroupFixturePreparers( + PrepareForTestWithFilegroup, + FixtureModifyConfig(func(config Config) { + config.BazelContext = MockBazelContext{ + OutputBaseDir: outBaseDir, + LabelToOutputFiles: map[string][]string{ + "//:baz": []string{"a/b/c/d/test.aidl"}, + }, + } + }), + ).RunTestWithBp(t, testCase.bp) + + fg := result.Module("baz", "").(*fileGroup) + AssertStringEquals(t, "src relativeRoot", testCase.rel, fg.srcs[0].Rel()) + AssertStringEquals(t, "src full path", expectedOutputfile, fg.srcs[0].String()) + } +} diff --git a/java/java_test.go b/java/java_test.go index 9e5cf0cf2..bfd97eb0d 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -1287,6 +1287,41 @@ func TestAidlExportIncludeDirsFromImports(t *testing.T) { } } +func TestAidlIncludeDirFromConvertedFileGroupWithPathPropInMixedBuilds(t *testing.T) { + bp := ` + filegroup { + name: "foo_aidl", + srcs: ["aidl/foo/IFoo.aidl"], + path: "aidl/foo", + bazel_module: { label: "//:foo_aidl" }, + } + java_library { + name: "foo", + srcs: [":foo_aidl"], + } +` + + outBaseDir := "out/bazel/output" + result := android.GroupFixturePreparers( + prepareForJavaTest, + android.PrepareForTestWithFilegroup, + android.FixtureModifyConfig(func(config android.Config) { + config.BazelContext = android.MockBazelContext{ + OutputBaseDir: outBaseDir, + LabelToOutputFiles: map[string][]string{ + "//:foo_aidl": []string{"aidl/foo/IFoo.aidl"}, + }, + } + }), + ).RunTestWithBp(t, bp) + + aidlCommand := result.ModuleForTests("foo", "android_common").Rule("aidl").RuleParams.Command + expectedAidlFlag := "-I" + outBaseDir + "/execroot/__main__/aidl/foo" + if !strings.Contains(aidlCommand, expectedAidlFlag) { + t.Errorf("aidl command %q does not contain %q", aidlCommand, expectedAidlFlag) + } +} + func TestAidlFlagsArePassedToTheAidlCompiler(t *testing.T) { ctx, _ := testJava(t, ` java_library {