Fixes for avb flags in soong-generated partitions
The logic for what variables control what aspect of AVB is pretty
complicated, this brings make and soong closer together.
Bug: 381120092
Test: m out/soong/.intermediates/build/soong/fsgen/aosp_cf_x86_64_phone_generated_system_image/android_common/prop, diff with make's prop file
Change-Id: I6ada90262f4578d120c35b7f8d38c06c2b1d9c27
diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index fbc8089..b6b4cb7 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -53,6 +53,12 @@
})
}
+// Remember to add referenced files to implicits!
+var textFileProcessorRule = pctx.AndroidStaticRule("text_file_processing", blueprint.RuleParams{
+ Command: "build/soong/scripts/text_file_processor.py $in $out",
+ CommandDeps: []string{"build/soong/scripts/text_file_processor.py"},
+})
+
type filesystem struct {
android.ModuleBase
android.PackagingBase
@@ -107,6 +113,9 @@
// avbtool. Default used by avbtool is sha1.
Avb_hash_algorithm *string
+ // Whether or not to use forward-error-correction codes when signing with AVB. Defaults to true.
+ Use_fec *bool
+
// The index used to prevent rollback of the image. Only used if use_avb is true.
Rollback_index *int64
@@ -563,11 +572,21 @@
FlagWithArg("--out_system=", rootDir.String()+"/system")
propFile, toolDeps := f.buildPropFile(ctx)
+
+ // Most of the time, if build_image were to call a host tool, it accepts the path to the
+ // host tool in a field in the prop file. However, it doesn't have that option for fec, which
+ // it expects to just be on the PATH. Add fec to the PATH.
+ fec := ctx.Config().HostToolPath(ctx, "fec")
+ pathToolDirs := []string{filepath.Dir(fec.String())}
+
output := android.PathForModuleOut(ctx, f.installFileName())
- builder.Command().BuiltTool("build_image").
+ builder.Command().
+ Textf("PATH=%s:$PATH", strings.Join(pathToolDirs, ":")).
+ BuiltTool("build_image").
Text(rootDir.String()). // input directory
Input(propFile).
Implicits(toolDeps).
+ Implicit(fec).
Output(output).
Text(rootDir.String()) // directory where to find fs_config_files|dirs
@@ -634,10 +653,15 @@
addPath("avb_avbtool", ctx.Config().HostToolPath(ctx, "avbtool"))
algorithm := proptools.StringDefault(f.properties.Avb_algorithm, "SHA256_RSA4096")
addStr("avb_algorithm", algorithm)
- key := android.PathForModuleSrc(ctx, proptools.String(f.properties.Avb_private_key))
- addPath("avb_key_path", key)
+ if f.properties.Avb_private_key != nil {
+ key := android.PathForModuleSrc(ctx, *f.properties.Avb_private_key)
+ addPath("avb_key_path", key)
+ }
addStr("partition_name", f.partitionName())
- avb_add_hashtree_footer_args := "--do_not_generate_fec"
+ avb_add_hashtree_footer_args := ""
+ if !proptools.BoolDefault(f.properties.Use_fec, true) {
+ avb_add_hashtree_footer_args += " --do_not_generate_fec"
+ }
if hashAlgorithm := proptools.String(f.properties.Avb_hash_algorithm); hashAlgorithm != "" {
avb_add_hashtree_footer_args += " --hash_algorithm " + hashAlgorithm
}
@@ -648,9 +672,9 @@
}
avb_add_hashtree_footer_args += " --rollback_index " + strconv.Itoa(rollbackIndex)
}
- securityPatchKey := "com.android.build." + f.partitionName() + ".security_patch"
- securityPatchValue := ctx.Config().PlatformSecurityPatch()
- avb_add_hashtree_footer_args += " --prop " + securityPatchKey + ":" + securityPatchValue
+ avb_add_hashtree_footer_args += fmt.Sprintf(" --prop com.android.build.%s.os_version:%s", f.partitionName(), ctx.Config().PlatformVersionLastStable())
+ avb_add_hashtree_footer_args += fmt.Sprintf(" --prop com.android.build.%s.fingerprint:{CONTENTS_OF:%s}", f.partitionName(), ctx.Config().BuildFingerprintFile(ctx))
+ avb_add_hashtree_footer_args += fmt.Sprintf(" --prop com.android.build.%s.security_patch:%s", f.partitionName(), ctx.Config().PlatformSecurityPatch())
addStr("avb_add_hashtree_footer_args", avb_add_hashtree_footer_args)
addStr("avb_salt", f.salt())
}
@@ -694,8 +718,15 @@
}
f.checkFsTypePropertyError(ctx, fst, fsTypeStr(fst))
+ propFilePreProcessing := android.PathForModuleOut(ctx, "prop_pre_processing")
+ android.WriteFileRuleVerbatim(ctx, propFilePreProcessing, propFileString.String())
propFile := android.PathForModuleOut(ctx, "prop")
- android.WriteFileRuleVerbatim(ctx, propFile, propFileString.String())
+ ctx.Build(pctx, android.BuildParams{
+ Rule: textFileProcessorRule,
+ Input: propFilePreProcessing,
+ Output: propFile,
+ Implicit: ctx.Config().BuildFingerprintFile(ctx),
+ })
return propFile, deps
}
diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go
index 72a5211..86496eb 100644
--- a/filesystem/filesystem_test.go
+++ b/filesystem/filesystem_test.go
@@ -593,7 +593,7 @@
`)
partition := result.ModuleForTests("erofs_partition", "android_common")
- buildImageConfig := android.ContentFromFileRuleForTests(t, result.TestContext, partition.Output("prop"))
+ buildImageConfig := android.ContentFromFileRuleForTests(t, result.TestContext, partition.Output("prop_pre_processing"))
android.AssertStringDoesContain(t, "erofs fs type", buildImageConfig, "fs_type=erofs")
android.AssertStringDoesContain(t, "erofs fs type compress algorithm", buildImageConfig, "erofs_default_compressor=lz4hc,9")
android.AssertStringDoesContain(t, "erofs fs type compress hint", buildImageConfig, "erofs_default_compress_hints=compress_hints.txt")
@@ -609,7 +609,7 @@
`)
partition := result.ModuleForTests("f2fs_partition", "android_common")
- buildImageConfig := android.ContentFromFileRuleForTests(t, result.TestContext, partition.Output("prop"))
+ buildImageConfig := android.ContentFromFileRuleForTests(t, result.TestContext, partition.Output("prop_pre_processing"))
android.AssertStringDoesContain(t, "f2fs fs type", buildImageConfig, "fs_type=f2fs")
android.AssertStringDoesContain(t, "f2fs fs type sparse", buildImageConfig, "f2fs_sparse_flag=-S")
}
diff --git a/fsgen/filesystem_creator.go b/fsgen/filesystem_creator.go
index cd29dfd..6aec90a 100644
--- a/fsgen/filesystem_creator.go
+++ b/fsgen/filesystem_creator.go
@@ -18,6 +18,7 @@
"crypto/sha256"
"fmt"
"path/filepath"
+ "slices"
"strconv"
"strings"
@@ -750,6 +751,7 @@
fsProps.Avb_algorithm = avbInfo.avbAlgorithm
// BOARD_AVB_SYSTEM_ROLLBACK_INDEX
fsProps.Rollback_index = avbInfo.avbRollbackIndex
+ fsProps.Avb_hash_algorithm = avbInfo.avbHashAlgorithm
fsProps.Partition_name = proptools.StringPtr(partitionType)
@@ -783,6 +785,7 @@
avbAlgorithm *string
avbRollbackIndex *int64
avbMode *string
+ avbHashAlgorithm *string
}
func getAvbInfo(config android.Config, partitionType string) avbInfo {
@@ -792,10 +795,23 @@
boardAvbEnable := partitionVars.BoardAvbEnable
if boardAvbEnable {
result.avbEnable = proptools.BoolPtr(true)
+ // There are "global" and "specific" copies of a lot of these variables. Sometimes they
+ // choose the specific and then fall back to the global one if it's not set, other times
+ // the global one actually only applies to the vbmeta partition.
+ if partitionType == "vbmeta" {
+ if partitionVars.BoardAvbKeyPath != "" {
+ result.avbKeyPath = proptools.StringPtr(partitionVars.BoardAvbKeyPath)
+ }
+ if partitionVars.BoardAvbRollbackIndex != "" {
+ parsed, err := strconv.ParseInt(partitionVars.BoardAvbRollbackIndex, 10, 64)
+ if err != nil {
+ panic(fmt.Sprintf("Rollback index must be an int, got %s", partitionVars.BoardAvbRollbackIndex))
+ }
+ result.avbRollbackIndex = &parsed
+ }
+ }
if specificPartitionVars.BoardAvbKeyPath != "" {
result.avbKeyPath = proptools.StringPtr(specificPartitionVars.BoardAvbKeyPath)
- } else if partitionVars.BoardAvbKeyPath != "" {
- result.avbKeyPath = proptools.StringPtr(partitionVars.BoardAvbKeyPath)
}
if specificPartitionVars.BoardAvbAlgorithm != "" {
result.avbAlgorithm = proptools.StringPtr(specificPartitionVars.BoardAvbAlgorithm)
@@ -808,13 +824,24 @@
panic(fmt.Sprintf("Rollback index must be an int, got %s", specificPartitionVars.BoardAvbRollbackIndex))
}
result.avbRollbackIndex = &parsed
- } else if partitionVars.BoardAvbRollbackIndex != "" {
- parsed, err := strconv.ParseInt(partitionVars.BoardAvbRollbackIndex, 10, 64)
+ }
+ if specificPartitionVars.BoardAvbRollbackIndex != "" {
+ parsed, err := strconv.ParseInt(specificPartitionVars.BoardAvbRollbackIndex, 10, 64)
if err != nil {
- panic(fmt.Sprintf("Rollback index must be an int, got %s", partitionVars.BoardAvbRollbackIndex))
+ panic(fmt.Sprintf("Rollback index must be an int, got %s", specificPartitionVars.BoardAvbRollbackIndex))
}
result.avbRollbackIndex = &parsed
}
+
+ // Make allows you to pass arbitrary arguments to avbtool via this variable, but in practice
+ // it's only used for --hash_algorithm. The soong module has a dedicated property for the
+ // hashtree algorithm, and doesn't allow custom arguments, so just extract the hashtree
+ // algorithm out of the arbitrary arguments.
+ addHashtreeFooterArgs := strings.Split(specificPartitionVars.BoardAvbAddHashtreeFooterArgs, " ")
+ if i := slices.Index(addHashtreeFooterArgs, "--hash_algorithm"); i >= 0 {
+ result.avbHashAlgorithm = &addHashtreeFooterArgs[i+1]
+ }
+
result.avbMode = proptools.StringPtr("make_legacy")
}
if result.avbKeyPath != nil {
diff --git a/scripts/text_file_processor.py b/scripts/text_file_processor.py
new file mode 100755
index 0000000..10186ce
--- /dev/null
+++ b/scripts/text_file_processor.py
@@ -0,0 +1,46 @@
+#!/usr/bin/env python
+#
+# Copyright (C) 2024 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the 'License');
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an 'AS IS' BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import argparse
+import re
+
+def main():
+ parser = argparse.ArgumentParser(description='This script looks for '
+ '`{CONTENTS_OF:path/to/file}` markers in the input file and replaces them with the actual '
+ 'contents of that file, with leading/trailing whitespace stripped. The idea is that this '
+ 'script could be extended to support more types of markers in the future.')
+ parser.add_argument('input')
+ parser.add_argument('output')
+ args = parser.parse_args()
+
+ with open(args.input, 'r') as f:
+ contents = f.read()
+
+ i = 0
+ replacedContents = ''
+ for m in re.finditer(r'{CONTENTS_OF:([a-zA-Z0-9 _/.-]+)}', contents):
+ replacedContents += contents[i:m.start()]
+ with open(m.group(1), 'r') as f:
+ replacedContents += f.read().strip()
+ i = m.end()
+ replacedContents += contents[i:]
+
+ with open(args.output, 'w') as f:
+ f.write(replacedContents)
+
+
+if __name__ == '__main__':
+ main()