From 885ee7ad86b9ae00d76e417475f02ed359498d97 Mon Sep 17 00:00:00 2001 From: Jingwen Chen Date: Tue, 26 Jan 2021 03:16:49 -0500 Subject: [PATCH] bp2build: support genrule $(location) and $(locations) Soong genrules support $(location) and $(locations) cmd variable shortcuts without labels. The shortcut is for the location of the first tool module from the concatenation of the tools and tool_files properties. Bazel doesn't support this shortcut, so the bp2build mapping needs to support it. Documentation: https://cs.android.com/android/platform/superproject/+/master:build/soong/genrule/genrule.go;l=95-96;drc=316e07c593ab66599c74725cf482dedbb32b2875 Code: https://cs.android.com/android/platform/superproject/+/master:build/soong/genrule/genrule.go;l=236-246;drc=316e07c593ab66599c74725cf482dedbb32b2875 Test: build_conversion_test.go Test: TH Change-Id: I8aa98ae460af3a3fbb0a7835572518680dc7ade1 --- bp2build/build_conversion_test.go | 95 ++++++++++++++++++++++++++++--- genrule/genrule.go | 10 +++- 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/bp2build/build_conversion_test.go b/bp2build/build_conversion_test.go index be325304c..367e13f75 100644 --- a/bp2build/build_conversion_test.go +++ b/bp2build/build_conversion_test.go @@ -274,8 +274,10 @@ func TestModuleTypeBp2Build(t *testing.T) { moduleTypeUnderTestFactory android.ModuleFactory bp string expectedBazelTarget string + description string }{ { + description: "filegroup with no srcs", moduleTypeUnderTest: "filegroup", moduleTypeUnderTestFactory: android.FileGroupFactory, bp: `filegroup { @@ -289,6 +291,7 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { + description: "filegroup with srcs", moduleTypeUnderTest: "filegroup", moduleTypeUnderTestFactory: android.FileGroupFactory, bp: `filegroup { @@ -304,18 +307,19 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { + description: "genrule with command line variable replacements", moduleTypeUnderTest: "genrule", moduleTypeUnderTestFactory: genrule.GenRuleFactory, bp: `genrule { name: "foo", out: ["foo.out"], srcs: ["foo.in"], - tool_files: [":foo.tool"], - cmd: "$(location :foo.tool) arg $(in) $(out)", + tools: [":foo.tool"], + cmd: "$(location :foo.tool) --genDir=$(genDir) arg $(in) $(out)", }`, expectedBazelTarget: `genrule( name = "foo", - cmd = "$(location :foo.tool) arg $(SRCS) $(OUTS)", + cmd = "$(location :foo.tool) --genDir=$(GENDIR) arg $(SRCS) $(OUTS)", outs = [ "foo.out", ], @@ -328,18 +332,44 @@ func TestModuleTypeBp2Build(t *testing.T) { )`, }, { + description: "genrule using $(locations :label)", moduleTypeUnderTest: "genrule", moduleTypeUnderTestFactory: genrule.GenRuleFactory, bp: `genrule { name: "foo", out: ["foo.out"], srcs: ["foo.in"], - tools: [":foo.tool"], - cmd: "$(location :foo.tool) --out-dir=$(genDir) $(in)", + tools: [":foo.tools"], + cmd: "$(locations :foo.tools) -s $(out) $(in)", }`, expectedBazelTarget: `genrule( name = "foo", - cmd = "$(location :foo.tool) --out-dir=$(GENDIR) $(SRCS)", + cmd = "$(locations :foo.tools) -s $(OUTS) $(SRCS)", + outs = [ + "foo.out", + ], + srcs = [ + "foo.in", + ], + tools = [ + ":foo.tools", + ], +)`, + }, + { + description: "genrule using $(location) label should substitute first tool label automatically", + moduleTypeUnderTest: "genrule", + moduleTypeUnderTestFactory: genrule.GenRuleFactory, + bp: `genrule { + name: "foo", + out: ["foo.out"], + srcs: ["foo.in"], + tool_files: [":foo.tool", ":other.tool"], + cmd: "$(location) -s $(out) $(in)", +}`, + expectedBazelTarget: `genrule( + name = "foo", + cmd = "$(location :foo.tool) -s $(OUTS) $(SRCS)", outs = [ "foo.out", ], @@ -348,6 +378,54 @@ func TestModuleTypeBp2Build(t *testing.T) { ], tools = [ ":foo.tool", + ":other.tool", + ], +)`, + }, + { + description: "genrule using $(locations) label should substitute first tool label automatically", + moduleTypeUnderTest: "genrule", + moduleTypeUnderTestFactory: genrule.GenRuleFactory, + bp: `genrule { + name: "foo", + out: ["foo.out"], + srcs: ["foo.in"], + tools: [":foo.tool", ":other.tool"], + cmd: "$(locations) -s $(out) $(in)", +}`, + expectedBazelTarget: `genrule( + name = "foo", + cmd = "$(locations :foo.tool) -s $(OUTS) $(SRCS)", + outs = [ + "foo.out", + ], + srcs = [ + "foo.in", + ], + tools = [ + ":foo.tool", + ":other.tool", + ], +)`, + }, + { + description: "genrule without tools or tool_files can convert successfully", + moduleTypeUnderTest: "genrule", + moduleTypeUnderTestFactory: genrule.GenRuleFactory, + bp: `genrule { + name: "foo", + out: ["foo.out"], + srcs: ["foo.in"], + cmd: "cp $(in) $(out)", +}`, + expectedBazelTarget: `genrule( + name = "foo", + cmd = "cp $(SRCS) $(OUTS)", + outs = [ + "foo.out", + ], + srcs = [ + "foo.in", ], )`, }, @@ -367,13 +445,14 @@ func TestModuleTypeBp2Build(t *testing.T) { bazelTargets := GenerateSoongModuleTargets(ctx.Context.Context, Bp2Build)[dir] if actualCount, expectedCount := len(bazelTargets), 1; actualCount != expectedCount { - t.Fatalf("Expected %d bazel target, got %d", expectedCount, actualCount) + t.Fatalf("%s: Expected %d bazel target, got %d", testCase.description, expectedCount, actualCount) } actualBazelTarget := bazelTargets[0] if actualBazelTarget.content != testCase.expectedBazelTarget { t.Errorf( - "Expected generated Bazel target to be '%s', got '%s'", + "%s: Expected generated Bazel target to be '%s', got '%s'", + testCase.description, testCase.expectedBazelTarget, actualBazelTarget.content, ) diff --git a/genrule/genrule.go b/genrule/genrule.go index 1f47deca4..66e5652f7 100644 --- a/genrule/genrule.go +++ b/genrule/genrule.go @@ -797,12 +797,19 @@ func BazelGenruleFactory() android.Module { func bp2buildMutator(ctx android.TopDownMutatorContext) { if m, ok := ctx.Module().(*Module); ok { name := "__bp2build__" + m.Name() + // Bazel only has the "tools" attribute. + tools := append(m.properties.Tools, m.properties.Tool_files...) + // Replace in and out variables with $< and $@ var cmd string if m.properties.Cmd != nil { cmd = strings.Replace(*m.properties.Cmd, "$(in)", "$(SRCS)", -1) cmd = strings.Replace(cmd, "$(out)", "$(OUTS)", -1) cmd = strings.Replace(cmd, "$(genDir)", "$(GENDIR)", -1) + if len(tools) > 0 { + cmd = strings.Replace(cmd, "$(location)", fmt.Sprintf("$(location %s)", tools[0]), -1) + cmd = strings.Replace(cmd, "$(locations)", fmt.Sprintf("$(locations %s)", tools[0]), -1) + } } // The Out prop is not in an immediately accessible field @@ -816,9 +823,6 @@ func bp2buildMutator(ctx android.TopDownMutatorContext) { } } - // Bazel only has the "tools" attribute. - tools := append(m.properties.Tools, m.properties.Tool_files...) - // Create the BazelTargetModule. ctx.CreateModule(BazelGenruleFactory, &bazelGenruleAttributes{ Name: proptools.StringPtr(name),