From 375058f67d23150e8e2f2c9bd390420705f7f698 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 29 Nov 2019 20:17:53 +0000 Subject: [PATCH] Use static build rules in snapshot generation It is easier to extract information out of static build rules than it is out of custom build rules built using the builder as the former provides access to the in/out and args separate from the rule whereas the latter only provides access to in/out. Also, cleans up some warnings that appear in Intellij. There is a lot of duplication in the testing code. That will be resolved in a follow up change. Bug: 143678475 Test: m conscrypt-module-sdk Change-Id: I973bc0c90b0affd84487f1b222dd3e6c22c07ec0 --- sdk/sdk.go | 2 ++ sdk/sdk_test.go | 74 +++++++++++++++++++++++++------------------ sdk/update.go | 83 +++++++++++++++++++++++++++++++++---------------- 3 files changed, 102 insertions(+), 57 deletions(-) diff --git a/sdk/sdk.go b/sdk/sdk.go index 431ace9a5..18b0040b9 100644 --- a/sdk/sdk.go +++ b/sdk/sdk.go @@ -31,6 +31,8 @@ import ( func init() { pctx.Import("android/soong/android") + pctx.Import("android/soong/java/config") + android.RegisterModuleType("sdk", ModuleFactory) android.RegisterModuleType("sdk_snapshot", SnapshotModuleFactory) android.PreDepsMutators(RegisterPreDepsMutators) diff --git a/sdk/sdk_test.go b/sdk/sdk_test.go index 5435ef658..6c2065998 100644 --- a/sdk/sdk_test.go +++ b/sdk/sdk_test.go @@ -27,7 +27,7 @@ import ( "android/soong/java" ) -func testSdkContext(t *testing.T, bp string) (*android.TestContext, android.Config) { +func testSdkContext(bp string) (*android.TestContext, android.Config) { config := android.TestArchConfig(buildDir, nil) ctx := android.NewTestArchContext() @@ -114,7 +114,7 @@ func testSdkContext(t *testing.T, bp string) (*android.TestContext, android.Conf } func testSdk(t *testing.T, bp string) (*android.TestContext, android.Config) { - ctx, config := testSdkContext(t, bp) + ctx, config := testSdkContext(bp) _, errs := ctx.ParseFileList(".", []string{"Android.bp"}) android.FailIfErrored(t, errs) _, errs = ctx.PrepareBuildActions(config) @@ -124,7 +124,7 @@ func testSdk(t *testing.T, bp string) (*android.TestContext, android.Config) { func testSdkError(t *testing.T, pattern, bp string) { t.Helper() - ctx, config := testSdkContext(t, bp) + ctx, config := testSdkContext(bp) _, errs := ctx.ParseFileList(".", []string{"Android.bp"}) if len(errs) > 0 { android.FailIfNoMatchingErrors(t, pattern, errs) @@ -147,7 +147,7 @@ func ensureListContains(t *testing.T, result []string, expected string) { } func pathsToStrings(paths android.Paths) []string { - ret := []string{} + var ret []string for _, p := range paths { ret = append(ret, p.String()) } @@ -554,14 +554,23 @@ sdk_snapshot { var copySrcs []string var copyDests []string buildParams := sdk.BuildParamsForTests() - var zipBp android.BuildParams + var mergeZipInputs []string + var intermediateZip string + var outputZip string for _, bp := range buildParams { ruleString := bp.Rule.String() - if ruleString == "android/soong/android.Cp" { + if ruleString == android.Cp.String() { copySrcs = append(copySrcs, bp.Input.String()) copyDests = append(copyDests, bp.Output.Rel()) // rooted at the snapshot root - } else if ruleString == ":m.mysdk_android_common.snapshot" { - zipBp = bp + } else if ruleString == zipFiles.String() { + intermediateZip = bp.Output.String() + } else if ruleString == mergeZips.String() { + input := bp.Input.String() + if intermediateZip != input { + t.Errorf("Intermediate zip %s is not an input to merge_zips, %s is used instead", intermediateZip, input) + } + mergeZipInputs = bp.Inputs.Strings() + outputZip = bp.Output.String() } } @@ -582,17 +591,14 @@ sdk_snapshot { ensureListContains(t, copyDests, "java/myjavalib.jar") ensureListContains(t, copyDests, "arm64/lib/mynativelib.so") + expectedOutputZip := filepath.Join(buildDir, ".intermediates/mysdk/android_common/mysdk-current.zip") + expectedRepackagedZip := filepath.Join(buildDir, ".intermediates/mysdk/android_common/tmp/java/myjavaapistubs_stubs_sources.zip") + // Ensure that the droidstubs .srcjar as repackaged into a temporary zip file // and then merged together with the intermediate snapshot zip. - snapshotCreationInputs := zipBp.Implicits.Strings() - ensureListContains(t, snapshotCreationInputs, - filepath.Join(buildDir, ".intermediates/mysdk/android_common/tmp/java/myjavaapistubs_stubs_sources.zip")) - ensureListContains(t, snapshotCreationInputs, - filepath.Join(buildDir, ".intermediates/mysdk/android_common/mysdk-current.unmerged.zip")) - actual := zipBp.Output.String() - expected := filepath.Join(buildDir, ".intermediates/mysdk/android_common/mysdk-current.zip") - if actual != expected { - t.Errorf("Expected snapshot output to be %q but was %q", expected, actual) + ensureListContains(t, mergeZipInputs, expectedRepackagedZip) + if outputZip != expectedOutputZip { + t.Errorf("Expected snapshot output to be %q but was %q", expectedOutputZip, outputZip) } } @@ -749,14 +755,23 @@ sdk_snapshot { var copySrcs []string var copyDests []string buildParams := sdk.BuildParamsForTests() - var zipBp android.BuildParams + var mergeZipInputs []string + var intermediateZip string + var outputZip string for _, bp := range buildParams { ruleString := bp.Rule.String() - if ruleString == "android/soong/android.Cp" { + if ruleString == android.Cp.String() { copySrcs = append(copySrcs, bp.Input.String()) copyDests = append(copyDests, bp.Output.Rel()) // rooted at the snapshot root - } else if ruleString == ":m.mysdk_linux_glibc_common.snapshot" { - zipBp = bp + } else if ruleString == zipFiles.String() { + intermediateZip = bp.Output.String() + } else if ruleString == mergeZips.String() { + input := bp.Input.String() + if intermediateZip != input { + t.Errorf("Intermediate zip %s is not an input to merge_zips, %s is used instead", intermediateZip, input) + } + mergeZipInputs = bp.Inputs.Strings() + outputZip = bp.Output.String() } } @@ -777,17 +792,14 @@ sdk_snapshot { ensureListContains(t, copyDests, "java/myjavalib.jar") ensureListContains(t, copyDests, "x86_64/lib/mynativelib.so") + expectedOutputZip := filepath.Join(buildDir, ".intermediates/mysdk/linux_glibc_common/mysdk-current.zip") + expectedRepackagedZip := filepath.Join(buildDir, ".intermediates/mysdk/linux_glibc_common/tmp/java/myjavaapistubs_stubs_sources.zip") + // Ensure that the droidstubs .srcjar as repackaged into a temporary zip file // and then merged together with the intermediate snapshot zip. - snapshotCreationInputs := zipBp.Implicits.Strings() - ensureListContains(t, snapshotCreationInputs, - filepath.Join(buildDir, ".intermediates/mysdk/linux_glibc_common/tmp/java/myjavaapistubs_stubs_sources.zip")) - ensureListContains(t, snapshotCreationInputs, - filepath.Join(buildDir, ".intermediates/mysdk/linux_glibc_common/mysdk-current.unmerged.zip")) - actual := zipBp.Output.String() - expected := filepath.Join(buildDir, ".intermediates/mysdk/linux_glibc_common/mysdk-current.zip") - if actual != expected { - t.Errorf("Expected snapshot output to be %q but was %q", expected, actual) + ensureListContains(t, mergeZipInputs, expectedRepackagedZip) + if outputZip != expectedOutputZip { + t.Errorf("Expected snapshot output to be %q but was %q", expectedOutputZip, outputZip) } } @@ -810,7 +822,7 @@ func setUp() { } func tearDown() { - os.RemoveAll(buildDir) + _ = os.RemoveAll(buildDir) } func TestMain(m *testing.M) { diff --git a/sdk/update.go b/sdk/update.go index 8159d3b4d..63bb1b73b 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -20,6 +20,7 @@ import ( "reflect" "strings" + "github.com/google/blueprint" "github.com/google/blueprint/proptools" "android/soong/android" @@ -29,6 +30,36 @@ import ( var pctx = android.NewPackageContext("android/soong/sdk") +var ( + repackageZip = pctx.AndroidStaticRule("SnapshotRepackageZip", + blueprint.RuleParams{ + Command: `${config.Zip2ZipCmd} -i $in -o $out "**/*:$destdir"`, + CommandDeps: []string{ + "${config.Zip2ZipCmd}", + }, + }, + "destdir") + + zipFiles = pctx.AndroidStaticRule("SnapshotZipFiles", + blueprint.RuleParams{ + Command: `${config.SoongZipCmd} -C $basedir -l $out.rsp -o $out`, + CommandDeps: []string{ + "${config.SoongZipCmd}", + }, + Rspfile: "$out.rsp", + RspfileContent: "$in", + }, + "basedir") + + mergeZips = pctx.AndroidStaticRule("SnapshotMergeZips", + blueprint.RuleParams{ + Command: `${config.MergeZipsCmd} $out $in`, + CommandDeps: []string{ + "${config.MergeZipsCmd}", + }, + }) +) + type generatedContents struct { content strings.Builder indentLevel int @@ -316,41 +347,39 @@ func (s *sdk) buildSnapshot(ctx android.ModuleContext) android.OutputPath { // zip them all outputZipFile := android.PathForModuleOut(ctx, ctx.ModuleName()+"-current.zip").OutputPath - outputRuleName := "snapshot" outputDesc := "Building snapshot for " + ctx.ModuleName() // If there are no zips to merge then generate the output zip directly. // Otherwise, generate an intermediate zip file into which other zips can be // merged. var zipFile android.OutputPath - var ruleName string var desc string if len(builder.zipsToMerge) == 0 { zipFile = outputZipFile - ruleName = outputRuleName desc = outputDesc } else { zipFile = android.PathForModuleOut(ctx, ctx.ModuleName()+"-current.unmerged.zip").OutputPath - ruleName = "intermediate snapshot" desc = "Building intermediate snapshot for " + ctx.ModuleName() } - rb := android.NewRuleBuilder() - rb.Command(). - BuiltTool(ctx, "soong_zip"). - FlagWithArg("-C ", builder.snapshotDir.String()). - FlagWithRspFileInputList("-l ", filesToZip). - FlagWithOutput("-o ", zipFile) - rb.Build(pctx, ctx, ruleName, desc) + ctx.Build(pctx, android.BuildParams{ + Description: desc, + Rule: zipFiles, + Inputs: filesToZip, + Output: zipFile, + Args: map[string]string{ + "basedir": builder.snapshotDir.String(), + }, + }) if len(builder.zipsToMerge) != 0 { - rb := android.NewRuleBuilder() - rb.Command(). - BuiltTool(ctx, "merge_zips"). - Output(outputZipFile). - Input(zipFile). - Inputs(builder.zipsToMerge) - rb.Build(pctx, ctx, outputRuleName, outputDesc) + ctx.Build(pctx, android.BuildParams{ + Description: outputDesc, + Rule: mergeZips, + Input: zipFile, + Inputs: builder.zipsToMerge, + Output: outputZipFile, + }) } return outputZipFile @@ -534,14 +563,16 @@ func (s *snapshotBuilder) UnzipToSnapshot(zipPath android.Path, destDir string) // Repackage the zip file so that the entries are in the destDir directory. // This will allow the zip file to be merged into the snapshot. tmpZipPath := android.PathForModuleOut(ctx, "tmp", destDir+".zip").OutputPath - rb := android.NewRuleBuilder() - rb.Command(). - BuiltTool(ctx, "zip2zip"). - FlagWithInput("-i ", zipPath). - FlagWithOutput("-o ", tmpZipPath). - Flag("**/*:" + destDir) - rb.Build(pctx, ctx, "repackaging "+destDir, - "Repackaging zip file "+destDir+" for snapshot "+ctx.ModuleName()) + + ctx.Build(pctx, android.BuildParams{ + Description: "Repackaging zip file " + destDir + " for snapshot " + ctx.ModuleName(), + Rule: repackageZip, + Input: zipPath, + Output: tmpZipPath, + Args: map[string]string{ + "destdir": destDir, + }, + }) // Add the repackaged zip file to the files to merge. s.zipsToMerge = append(s.zipsToMerge, tmpZipPath)