Merge "Add --dedup flag to generate-partition-aconfig-flag-file" into main
diff --git a/core/config.mk b/core/config.mk
index fbf6764..f8a9879 100644
--- a/core/config.mk
+++ b/core/config.mk
@@ -436,16 +436,16 @@
# Boolean variable determining if AOSP is page size agnostic. This means
# that AOSP can use a kernel configured with 4k/16k/64k PAGE SIZES.
-TARGET_PAGE_SIZE_AGNOSTIC := false
-ifdef PRODUCT_PAGE_SIZE_AGNOSTIC
- TARGET_PAGE_SIZE_AGNOSTIC := $(PRODUCT_PAGE_SIZE_AGNOSTIC)
- ifeq ($(TARGET_PAGE_SIZE_AGNOSTIC),true)
+TARGET_NO_BIONIC_PAGE_SIZE_MACRO := false
+ifdef PRODUCT_NO_BIONIC_PAGE_SIZE_MACRO
+ TARGET_NO_BIONIC_PAGE_SIZE_MACRO := $(PRODUCT_NO_BIONIC_PAGE_SIZE_MACRO)
+ ifeq ($(TARGET_NO_BIONIC_PAGE_SIZE_MACRO),true)
ifneq ($(TARGET_MAX_PAGE_SIZE_SUPPORTED),65536)
$(error TARGET_MAX_PAGE_SIZE_SUPPORTED has to be 65536 to support page size agnostic)
endif
endif
endif
-.KATI_READONLY := TARGET_PAGE_SIZE_AGNOSTIC
+.KATI_READONLY := TARGET_NO_BIONIC_PAGE_SIZE_MACRO
# Pruned directory options used when using findleaves.py
# See envsetup.mk for a description of SCAN_EXCLUDE_DIRS
diff --git a/core/product.mk b/core/product.mk
index 91b811d..5515a8a 100644
--- a/core/product.mk
+++ b/core/product.mk
@@ -35,7 +35,7 @@
# Indicates that AOSP can use a kernel configured with 4k/16k/64k page sizes.
# The possible values are true or false.
-_product_single_value_vars += PRODUCT_PAGE_SIZE_AGNOSTIC
+_product_single_value_vars += PRODUCT_NO_BIONIC_PAGE_SIZE_MACRO
# The resource configuration options to use for this product.
_product_list_vars += PRODUCT_LOCALES
diff --git a/core/soong_config.mk b/core/soong_config.mk
index 5fdf416..193ac18 100644
--- a/core/soong_config.mk
+++ b/core/soong_config.mk
@@ -159,7 +159,7 @@
$(call add_json_bool, Malloc_pattern_fill_contents, $(MALLOC_PATTERN_FILL_CONTENTS))
$(call add_json_str, Override_rs_driver, $(OVERRIDE_RS_DRIVER))
$(call add_json_str, DeviceMaxPageSizeSupported, $(TARGET_MAX_PAGE_SIZE_SUPPORTED))
-$(call add_json_bool, DevicePageSizeAgnostic, $(filter true,$(TARGET_PAGE_SIZE_AGNOSTIC)))
+$(call add_json_bool, DeviceNoBionicPageSizeMacro, $(filter true,$(TARGET_NO_BIONIC_PAGE_SIZE_MACRO)))
$(call add_json_bool, UncompressPrivAppDex, $(call invert_bool,$(filter true,$(DONT_UNCOMPRESS_PRIV_APPS_DEXS))))
$(call add_json_list, ModulesLoadedByPrivilegedModules, $(PRODUCT_LOADED_BY_PRIVILEGED_MODULES))
diff --git a/tools/aconfig/protos/aconfig.proto b/tools/aconfig/protos/aconfig.proto
index 1a80b04..ed4b24c 100644
--- a/tools/aconfig/protos/aconfig.proto
+++ b/tools/aconfig/protos/aconfig.proto
@@ -41,8 +41,23 @@
repeated string bug = 4;
optional bool is_fixed_read_only = 5;
optional bool is_exported = 6;
+ optional flag_metadata metadata = 7;
};
+// Optional metadata about the flag, such as its purpose and its intended form factors.
+// Can influence the applied policies and testing strategy.
+message flag_metadata {
+ enum flag_purpose {
+ PURPOSE_UNSPECIFIED = 0;
+ PURPOSE_FEATURE = 1;
+ PURPOSE_BUGFIX = 2;
+ }
+
+ optional flag_purpose purpose = 1;
+
+ // TODO(b/315025930): Add field to designate intended target device form factor(s), such as phone, watch or other.
+}
+
message flag_declarations {
optional string package = 1;
repeated flag_declaration flag = 2;
@@ -81,6 +96,7 @@
optional bool is_fixed_read_only = 9;
optional bool is_exported = 10;
optional string container = 11;
+ optional flag_metadata metadata = 12;
}
message parsed_flags {
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index 37ee79e..1e80d26 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -24,7 +24,8 @@
use crate::codegen_java::generate_java_code;
use crate::codegen_rust::generate_rust_code;
use crate::protos::{
- ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag, ProtoParsedFlags, ProtoTracepoint,
+ ProtoFlagMetadata, ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag, ProtoParsedFlags,
+ ProtoTracepoint,
};
pub struct Input {
@@ -118,6 +119,11 @@
tracepoint.set_permission(flag_permission);
parsed_flag.trace.push(tracepoint);
+ let mut metadata = ProtoFlagMetadata::new();
+ let purpose = flag_declaration.metadata.purpose();
+ metadata.set_purpose(purpose);
+ parsed_flag.metadata = Some(metadata).into();
+
// verify ParsedFlag looks reasonable
crate::protos::parsed_flag::verify_fields(&parsed_flag)?;
@@ -264,7 +270,11 @@
Textproto,
}
-pub fn dump_parsed_flags(mut input: Vec<Input>, format: DumpFormat, dedup: bool) -> Result<Vec<u8>> {
+pub fn dump_parsed_flags(
+ mut input: Vec<Input>,
+ format: DumpFormat,
+ dedup: bool,
+) -> Result<Vec<u8>> {
let individually_parsed_flags: Result<Vec<ProtoParsedFlags>> =
input.iter_mut().map(|i| i.try_parse_flags()).collect();
let parsed_flags: ProtoParsedFlags =
@@ -325,6 +335,7 @@
#[cfg(test)]
mod tests {
use super::*;
+ use crate::protos::ProtoFlagPurpose;
#[test]
fn test_parse_flags() {
@@ -339,6 +350,7 @@
assert_eq!("This flag is ENABLED + READ_ONLY", enabled_ro.description());
assert_eq!(ProtoFlagState::ENABLED, enabled_ro.state());
assert_eq!(ProtoFlagPermission::READ_ONLY, enabled_ro.permission());
+ assert_eq!(ProtoFlagPurpose::PURPOSE_BUGFIX, enabled_ro.metadata.purpose());
assert_eq!(3, enabled_ro.trace.len());
assert!(!enabled_ro.is_fixed_read_only());
assert_eq!("tests/test.aconfig", enabled_ro.trace[0].source());
@@ -511,6 +523,41 @@
}
#[test]
+ fn test_parse_flags_metadata() {
+ let metadata_flag = r#"
+ package: "com.first"
+ flag {
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of this feature flag."
+ bug: "123"
+ metadata {
+ purpose: PURPOSE_FEATURE
+ }
+ }
+ "#;
+ let declaration = vec![Input {
+ source: "memory".to_string(),
+ reader: Box::new(metadata_flag.as_bytes()),
+ }];
+ let value: Vec<Input> = vec![];
+
+ let flags_bytes = crate::commands::parse_flags(
+ "com.first",
+ None,
+ declaration,
+ value,
+ ProtoFlagPermission::READ_ONLY,
+ )
+ .unwrap();
+ let parsed_flags =
+ crate::protos::parsed_flags::try_from_binary_proto(&flags_bytes).unwrap();
+ assert_eq!(1, parsed_flags.parsed_flag.len());
+ let parsed_flag = parsed_flags.parsed_flag.first().unwrap();
+ assert_eq!(ProtoFlagPurpose::PURPOSE_FEATURE, parsed_flag.metadata.purpose());
+ }
+
+ #[test]
fn test_create_device_config_defaults() {
let input = parse_test_flags_as_input();
let bytes = create_device_config_defaults(input).unwrap();
diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs
index 3d9089c..37eb67d 100644
--- a/tools/aconfig/src/protos.rs
+++ b/tools/aconfig/src/protos.rs
@@ -29,8 +29,10 @@
// ---- When building with the Android tool-chain ----
#[cfg(not(feature = "cargo"))]
mod auto_generated {
+ pub use aconfig_protos::aconfig::flag_metadata::Flag_purpose as ProtoFlagPurpose;
pub use aconfig_protos::aconfig::Flag_declaration as ProtoFlagDeclaration;
pub use aconfig_protos::aconfig::Flag_declarations as ProtoFlagDeclarations;
+ pub use aconfig_protos::aconfig::Flag_metadata as ProtoFlagMetadata;
pub use aconfig_protos::aconfig::Flag_permission as ProtoFlagPermission;
pub use aconfig_protos::aconfig::Flag_state as ProtoFlagState;
pub use aconfig_protos::aconfig::Flag_value as ProtoFlagValue;
@@ -47,8 +49,10 @@
// because this is only used during local development, and only if using cargo instead of the
// Android tool-chain, we allow it
include!(concat!(env!("OUT_DIR"), "/aconfig_proto/mod.rs"));
+ pub use aconfig::flag_metadata::Flag_purpose as ProtoFlagPurpose;
pub use aconfig::Flag_declaration as ProtoFlagDeclaration;
pub use aconfig::Flag_declarations as ProtoFlagDeclarations;
+ pub use aconfig::Flag_metadata as ProtoFlagMetadata;
pub use aconfig::Flag_permission as ProtoFlagPermission;
pub use aconfig::Flag_state as ProtoFlagState;
pub use aconfig::Flag_value as ProtoFlagValue;
@@ -938,11 +942,13 @@
assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.first.first (defined in flags.declarations and flags.declarations)");
// two conflicting flags with dedup disabled
- let error = parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], false).unwrap_err();
+ let error =
+ parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], false).unwrap_err();
assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.second.second (defined in flags.declarations and duplicate/flags.declarations)");
// two conflicting flags with dedup enabled
- let error = parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], true).unwrap_err();
+ let error =
+ parsed_flags::merge(vec![second.clone(), second_duplicate.clone()], true).unwrap_err();
assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.second.second (defined in flags.declarations and duplicate/flags.declarations)");
// valid cases
@@ -950,10 +956,22 @@
assert!(parsed_flags::merge(vec![], true).unwrap().parsed_flag.is_empty());
assert_eq!(first, parsed_flags::merge(vec![first.clone()], false).unwrap());
assert_eq!(first, parsed_flags::merge(vec![first.clone()], true).unwrap());
- assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()], false).unwrap());
- assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()], true).unwrap());
- assert_eq!(expected, parsed_flags::merge(vec![second.clone(), first.clone()], false).unwrap());
- assert_eq!(expected, parsed_flags::merge(vec![second.clone(), first.clone()], true).unwrap());
+ assert_eq!(
+ expected,
+ parsed_flags::merge(vec![first.clone(), second.clone()], false).unwrap()
+ );
+ assert_eq!(
+ expected,
+ parsed_flags::merge(vec![first.clone(), second.clone()], true).unwrap()
+ );
+ assert_eq!(
+ expected,
+ parsed_flags::merge(vec![second.clone(), first.clone()], false).unwrap()
+ );
+ assert_eq!(
+ expected,
+ parsed_flags::merge(vec![second.clone(), first.clone()], true).unwrap()
+ );
// two identical flags with dedup enabled
assert_eq!(first, parsed_flags::merge(vec![first.clone(), first.clone()], true).unwrap());
diff --git a/tools/aconfig/src/test.rs b/tools/aconfig/src/test.rs
index 8df4493..2c63fef 100644
--- a/tools/aconfig/src/test.rs
+++ b/tools/aconfig/src/test.rs
@@ -44,6 +44,9 @@
is_fixed_read_only: false
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -61,6 +64,9 @@
is_fixed_read_only: false
is_exported: true
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -83,6 +89,9 @@
is_fixed_read_only: false
is_exported: true
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -105,6 +114,9 @@
is_fixed_read_only: false
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -127,6 +139,9 @@
is_fixed_read_only: true
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -154,6 +169,9 @@
is_fixed_read_only: false
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
}
parsed_flag {
package: "com.android.aconfig.test"
@@ -176,6 +194,9 @@
is_fixed_read_only: false
is_exported: false
container: "system"
+ metadata {
+ purpose: PURPOSE_UNSPECIFIED
+ }
}
"#;
diff --git a/tools/aconfig/tests/test.aconfig b/tools/aconfig/tests/test.aconfig
index e44d154..310d0a6 100644
--- a/tools/aconfig/tests/test.aconfig
+++ b/tools/aconfig/tests/test.aconfig
@@ -10,6 +10,9 @@
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_ONLY"
bug: "abc"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
}
# This flag's final value is calculated from:
diff --git a/tools/metadata/Android.bp b/tools/metadata/Android.bp
index b2fabec..77d106d 100644
--- a/tools/metadata/Android.bp
+++ b/tools/metadata/Android.bp
@@ -6,6 +6,8 @@
name: "metadata",
deps: [
"soong-testing-test_spec_proto",
+ "soong-testing-code_metadata_proto",
+ "soong-testing-code_metadata_internal_proto",
"golang-protobuf-proto",
],
srcs: [
diff --git a/tools/metadata/generator.go b/tools/metadata/generator.go
index e970e17..d328876 100644
--- a/tools/metadata/generator.go
+++ b/tools/metadata/generator.go
@@ -10,6 +10,8 @@
"strings"
"sync"
+ "android/soong/testing/code_metadata_internal_proto"
+ "android/soong/testing/code_metadata_proto"
"android/soong/testing/test_spec_proto"
"google.golang.org/protobuf/proto"
)
@@ -23,6 +25,13 @@
return mutex.(*sync.Mutex)
}
+// Define a struct to hold the combination of team ID and multi-ownership flag for validation
+type sourceFileAttributes struct {
+ TeamID string
+ MultiOwnership bool
+ Path string
+}
+
func getSortedKeys(syncMap *sync.Map) []string {
var allKeys []string
syncMap.Range(
@@ -36,14 +45,9 @@
return allKeys
}
-func writeOutput(
- outputFile string,
- allMetadata []*test_spec_proto.TestSpec_OwnershipMetadata,
-) {
- testSpec := &test_spec_proto.TestSpec{
- OwnershipMetadataList: allMetadata,
- }
- data, err := proto.Marshal(testSpec)
+// writeProtoToFile marshals a protobuf message and writes it to a file
+func writeProtoToFile(outputFile string, message proto.Message) {
+ data, err := proto.Marshal(message)
if err != nil {
log.Fatal(err)
}
@@ -88,8 +92,8 @@
}
func processTestSpecProtobuf(
- filePath string, ownershipMetadataMap *sync.Map, keyLocks *keyToLocksMap,
- errCh chan error, wg *sync.WaitGroup,
+ filePath string, ownershipMetadataMap *sync.Map, keyLocks *keyToLocksMap,
+ errCh chan error, wg *sync.WaitGroup,
) {
defer wg.Done()
@@ -117,7 +121,7 @@
if metadata.GetTrendyTeamId() != existing.GetTrendyTeamId() {
errCh <- fmt.Errorf(
"Conflicting trendy team IDs found for %s at:\n%s with teamId"+
- ": %s,\n%s with teamId: %s",
+ ": %s,\n%s with teamId: %s",
key,
metadata.GetPath(), metadata.GetTrendyTeamId(), existing.GetPath(),
existing.GetTrendyTeamId(),
@@ -141,10 +145,86 @@
}
}
+// processCodeMetadataProtobuf processes CodeMetadata protobuf files
+func processCodeMetadataProtobuf(
+ filePath string, ownershipMetadataMap *sync.Map, sourceFileMetadataMap *sync.Map, keyLocks *keyToLocksMap,
+ errCh chan error, wg *sync.WaitGroup,
+) {
+ defer wg.Done()
+
+ fileContent := strings.TrimRight(readFileToString(filePath), "\n")
+ internalCodeData := code_metadata_internal_proto.CodeMetadataInternal{}
+ err := proto.Unmarshal([]byte(fileContent), &internalCodeData)
+ if err != nil {
+ errCh <- err
+ return
+ }
+
+ // Process each TargetOwnership entry
+ for _, internalMetadata := range internalCodeData.GetTargetOwnershipList() {
+ key := internalMetadata.GetTargetName()
+ lock := keyLocks.GetLockForKey(key)
+ lock.Lock()
+
+ for _, srcFile := range internalMetadata.GetSourceFiles() {
+ srcFileKey := srcFile
+ srcFileLock := keyLocks.GetLockForKey(srcFileKey)
+ srcFileLock.Lock()
+ attributes := sourceFileAttributes{
+ TeamID: internalMetadata.GetTrendyTeamId(),
+ MultiOwnership: internalMetadata.GetMultiOwnership(),
+ Path: internalMetadata.GetPath(),
+ }
+
+ existingAttributes, exists := sourceFileMetadataMap.Load(srcFileKey)
+ if exists {
+ existing := existingAttributes.(sourceFileAttributes)
+ if attributes.TeamID != existing.TeamID && (!attributes.MultiOwnership || !existing.MultiOwnership) {
+ errCh <- fmt.Errorf(
+ "Conflict found for source file %s covered at %s with team ID: %s. Existing team ID: %s and path: %s."+
+ " If multi-ownership is required, multiOwnership should be set to true in all test_spec modules using this target. "+
+ "Multiple-ownership in general is discouraged though as it make infrastructure around android relying on this information pick up a random value when it needs only one.",
+ srcFile, internalMetadata.GetPath(), attributes.TeamID, existing.TeamID, existing.Path,
+ )
+ srcFileLock.Unlock()
+ lock.Unlock()
+ return
+ }
+ } else {
+ // Store the metadata if no conflict
+ sourceFileMetadataMap.Store(srcFileKey, attributes)
+ }
+ srcFileLock.Unlock()
+ }
+
+ value, loaded := ownershipMetadataMap.LoadOrStore(
+ key, []*code_metadata_internal_proto.CodeMetadataInternal_TargetOwnership{internalMetadata},
+ )
+ if loaded {
+ existingMetadata := value.([]*code_metadata_internal_proto.CodeMetadataInternal_TargetOwnership)
+ isDuplicate := false
+ for _, existing := range existingMetadata {
+ if internalMetadata.GetTrendyTeamId() == existing.GetTrendyTeamId() && internalMetadata.GetPath() == existing.GetPath() {
+ isDuplicate = true
+ break
+ }
+ }
+ if !isDuplicate {
+ existingMetadata = append(existingMetadata, internalMetadata)
+ ownershipMetadataMap.Store(key, existingMetadata)
+ }
+ }
+
+ lock.Unlock()
+ }
+}
+
func main() {
inputFile := flag.String("inputFile", "", "Input file path")
outputFile := flag.String("outputFile", "", "Output file path")
- rule := flag.String("rule", "", "Metadata rule (Hint: test_spec or code_metadata)")
+ rule := flag.String(
+ "rule", "", "Metadata rule (Hint: test_spec or code_metadata)",
+ )
flag.Parse()
if *inputFile == "" || *outputFile == "" || *rule == "" {
@@ -167,7 +247,9 @@
case "test_spec":
for _, filePath := range filePaths {
wg.Add(1)
- go processTestSpecProtobuf(filePath, ownershipMetadataMap, keyLocks, errCh, &wg)
+ go processTestSpecProtobuf(
+ filePath, ownershipMetadataMap, keyLocks, errCh, &wg,
+ )
}
wg.Wait()
@@ -186,9 +268,51 @@
allMetadata = append(allMetadata, metadataList...)
}
- writeOutput(*outputFile, allMetadata)
+ testSpec := &test_spec_proto.TestSpec{
+ OwnershipMetadataList: allMetadata,
+ }
+ writeProtoToFile(*outputFile, testSpec)
break
case "code_metadata":
+ sourceFileMetadataMap := &sync.Map{}
+ for _, filePath := range filePaths {
+ wg.Add(1)
+ go processCodeMetadataProtobuf(
+ filePath, ownershipMetadataMap, sourceFileMetadataMap, keyLocks, errCh, &wg,
+ )
+ }
+
+ wg.Wait()
+ close(errCh)
+
+ for err := range errCh {
+ log.Fatal(err)
+ }
+
+ sortedKeys := getSortedKeys(ownershipMetadataMap)
+ allMetadata := make([]*code_metadata_proto.CodeMetadata_TargetOwnership, 0)
+ for _, key := range sortedKeys {
+ value, _ := ownershipMetadataMap.Load(key)
+ metadata := value.([]*code_metadata_internal_proto.CodeMetadataInternal_TargetOwnership)
+ for _, m := range metadata {
+ targetName := m.GetTargetName()
+ path := m.GetPath()
+ trendyTeamId := m.GetTrendyTeamId()
+
+ allMetadata = append(allMetadata, &code_metadata_proto.CodeMetadata_TargetOwnership{
+ TargetName: &targetName,
+ Path: &path,
+ TrendyTeamId: &trendyTeamId,
+ SourceFiles: m.GetSourceFiles(),
+ })
+ }
+ }
+
+ finalMetadata := &code_metadata_proto.CodeMetadata{
+ TargetOwnershipList: allMetadata,
+ }
+ writeProtoToFile(*outputFile, finalMetadata)
+ break
default:
log.Fatalf("No specific processing implemented for rule '%s'.\n", *rule)
}
diff --git a/tools/metadata/go.work b/tools/metadata/go.work
index 23875da..f2cdf8e 100644
--- a/tools/metadata/go.work
+++ b/tools/metadata/go.work
@@ -4,7 +4,8 @@
.
../../../../external/golang-protobuf
../../../soong/testing/test_spec_proto
-
+ ../../../soong/testing/code_metadata_proto
+ ../../../soong/testing/code_metadata_proto_internal
)
replace google.golang.org/protobuf v0.0.0 => ../../../../external/golang-protobuf
diff --git a/tools/metadata/testdata/expectedCodeMetadataOutput.txt b/tools/metadata/testdata/expectedCodeMetadataOutput.txt
new file mode 100644
index 0000000..755cf40
--- /dev/null
+++ b/tools/metadata/testdata/expectedCodeMetadataOutput.txt
@@ -0,0 +1,7 @@
+
+
+bar
+Android.bp12346"b.java
+
+foo
+Android.bp12345"a.java
\ No newline at end of file
diff --git a/tools/metadata/testdata/file5.txt b/tools/metadata/testdata/file5.txt
new file mode 100644
index 0000000..d8de064
--- /dev/null
+++ b/tools/metadata/testdata/file5.txt
@@ -0,0 +1,4 @@
+
+
+foo
+Android.bp12345"a.java
diff --git a/tools/metadata/testdata/file6.txt b/tools/metadata/testdata/file6.txt
new file mode 100644
index 0000000..9c7cdcd
--- /dev/null
+++ b/tools/metadata/testdata/file6.txt
@@ -0,0 +1,4 @@
+
+
+bar
+Android.bp12346"b.java
diff --git a/tools/metadata/testdata/file7.txt b/tools/metadata/testdata/file7.txt
new file mode 100644
index 0000000..d8de064
--- /dev/null
+++ b/tools/metadata/testdata/file7.txt
@@ -0,0 +1,4 @@
+
+
+foo
+Android.bp12345"a.java
diff --git a/tools/metadata/testdata/file8.txt b/tools/metadata/testdata/file8.txt
new file mode 100644
index 0000000..a931690
--- /dev/null
+++ b/tools/metadata/testdata/file8.txt
@@ -0,0 +1,4 @@
+
+
+foo
+Android.gp12346"a.java
diff --git a/tools/metadata/testdata/generatedCodeMetadataOutput.txt b/tools/metadata/testdata/generatedCodeMetadataOutput.txt
new file mode 100644
index 0000000..755cf40
--- /dev/null
+++ b/tools/metadata/testdata/generatedCodeMetadataOutput.txt
@@ -0,0 +1,7 @@
+
+
+bar
+Android.bp12346"b.java
+
+foo
+Android.bp12345"a.java
\ No newline at end of file
diff --git a/tools/metadata/testdata/generatedCodeMetadataOutputFile.txt b/tools/metadata/testdata/generatedCodeMetadataOutputFile.txt
new file mode 100644
index 0000000..755cf40
--- /dev/null
+++ b/tools/metadata/testdata/generatedCodeMetadataOutputFile.txt
@@ -0,0 +1,7 @@
+
+
+bar
+Android.bp12346"b.java
+
+foo
+Android.bp12345"a.java
\ No newline at end of file
diff --git a/tools/metadata/testdata/inputCodeMetadata.txt b/tools/metadata/testdata/inputCodeMetadata.txt
new file mode 100644
index 0000000..7a81b7d
--- /dev/null
+++ b/tools/metadata/testdata/inputCodeMetadata.txt
@@ -0,0 +1 @@
+file5.txt file6.txt
\ No newline at end of file
diff --git a/tools/metadata/testdata/inputCodeMetadataNegative.txt b/tools/metadata/testdata/inputCodeMetadataNegative.txt
new file mode 100644
index 0000000..26668e4
--- /dev/null
+++ b/tools/metadata/testdata/inputCodeMetadataNegative.txt
@@ -0,0 +1 @@
+file7.txt file8.txt
\ No newline at end of file
diff --git a/tools/metadata/testdata/metadata_test.go b/tools/metadata/testdata/metadata_test.go
index 71856fe..314add3 100644
--- a/tools/metadata/testdata/metadata_test.go
+++ b/tools/metadata/testdata/metadata_test.go
@@ -87,3 +87,33 @@
t.Errorf("Generated file contents do not match the expected output")
}
}
+
+func TestCodeMetadata(t *testing.T) {
+ cmd := exec.Command(
+ "metadata", "-rule", "code_metadata", "-inputFile", "./inputCodeMetadata.txt", "-outputFile",
+ "./generatedCodeMetadataOutputFile.txt",
+ )
+ stderr, err := cmd.CombinedOutput()
+ if err != nil {
+ t.Fatalf("Error running metadata command: %s. Error: %v", stderr, err)
+ }
+
+ // Read the contents of the expected output file
+ expectedOutput, err := ioutil.ReadFile("./expectedCodeMetadataOutput.txt")
+ if err != nil {
+ t.Fatalf("Error reading expected output file: %s", err)
+ }
+
+ // Read the contents of the generated output file
+ generatedOutput, err := ioutil.ReadFile("./generatedCodeMetadataOutputFile.txt")
+ if err != nil {
+ t.Fatalf("Error reading generated output file: %s", err)
+ }
+
+ fmt.Println()
+
+ // Compare the contents
+ if string(expectedOutput) != string(generatedOutput) {
+ t.Errorf("Generated file contents do not match the expected output")
+ }
+}
diff --git a/tools/releasetools/Android.bp b/tools/releasetools/Android.bp
index ee266b7..bd8ce14 100644
--- a/tools/releasetools/Android.bp
+++ b/tools/releasetools/Android.bp
@@ -333,6 +333,7 @@
srcs: [
"ota_utils.py",
"payload_signer.py",
+ "ota_signing_utils.py",
],
libs: [
"releasetools_common",
@@ -348,7 +349,6 @@
},
srcs: [
"merge_ota.py",
- "ota_signing_utils.py",
],
libs: [
"ota_metadata_proto",
@@ -501,7 +501,6 @@
name: "ota_from_raw_img",
srcs: [
"ota_from_raw_img.py",
- "ota_signing_utils.py",
],
main: "ota_from_raw_img.py",
defaults: [
@@ -552,6 +551,8 @@
defaults: ["releasetools_binary_defaults"],
srcs: [
"sign_target_files_apks.py",
+ "payload_signer.py",
+ "ota_signing_utils.py",
],
libs: [
"releasetools_add_img_to_target_files",
@@ -615,7 +616,6 @@
"sign_target_files_apks.py",
"validate_target_files.py",
"merge_ota.py",
- "ota_signing_utils.py",
":releasetools_merge_sources",
":releasetools_merge_tests",
diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py
index 63fd854..7451ccc 100644
--- a/tools/releasetools/common.py
+++ b/tools/releasetools/common.py
@@ -3105,6 +3105,34 @@
zip_file.writestr(zinfo, data)
zipfile.ZIP64_LIMIT = saved_zip64_limit
+def ZipExclude(input_zip, output_zip, entries, force=False):
+ """Deletes entries from a ZIP file.
+
+ Args:
+ zip_filename: The name of the ZIP file.
+ entries: The name of the entry, or the list of names to be deleted.
+ """
+ if isinstance(entries, str):
+ entries = [entries]
+ # If list is empty, nothing to do
+ if not entries:
+ shutil.copy(input_zip, output_zip)
+ return
+
+ with zipfile.ZipFile(input_zip, 'r') as zin:
+ if not force and len(set(zin.namelist()).intersection(entries)) == 0:
+ raise ExternalError(
+ "Failed to delete zip entries, name not matched: %s" % entries)
+
+ fd, new_zipfile = tempfile.mkstemp(dir=os.path.dirname(input_zip))
+ os.close(fd)
+ cmd = ["zip2zip", "-i", input_zip, "-o", new_zipfile]
+ for entry in entries:
+ cmd.append("-x")
+ cmd.append(entry)
+ RunAndCheckOutput(cmd)
+ os.replace(new_zipfile, output_zip)
+
def ZipDelete(zip_filename, entries, force=False):
"""Deletes entries from a ZIP file.
@@ -3119,20 +3147,7 @@
if not entries:
return
- with zipfile.ZipFile(zip_filename, 'r') as zin:
- if not force and len(set(zin.namelist()).intersection(entries)) == 0:
- raise ExternalError(
- "Failed to delete zip entries, name not matched: %s" % entries)
-
- fd, new_zipfile = tempfile.mkstemp(dir=os.path.dirname(zip_filename))
- os.close(fd)
- cmd = ["zip2zip", "-i", zip_filename, "-o", new_zipfile]
- for entry in entries:
- cmd.append("-x")
- cmd.append(entry)
- RunAndCheckOutput(cmd)
-
- os.replace(new_zipfile, zip_filename)
+ ZipExclude(zip_filename, zip_filename, entries, force)
def ZipClose(zip_file):
diff --git a/tools/releasetools/create_brick_ota.py b/tools/releasetools/create_brick_ota.py
index 44f0a95..f290323 100644
--- a/tools/releasetools/create_brick_ota.py
+++ b/tools/releasetools/create_brick_ota.py
@@ -59,9 +59,9 @@
parser.add_argument('otafile', metavar='PAYLOAD', type=str,
help='The output OTA package file.')
parser.add_argument('--product', type=str,
- help='The product name of the device, for example, bramble, redfin. This can be a comma separated list.', required=True)
+ help='The product name of the device, for example, bramble, redfin.', required=True)
parser.add_argument('--serialno', type=str,
- help='The serial number of devices that are allowed to install this OTA package. This can be a comma separated list.')
+ help='The serial number of devices that are allowed to install this OTA package. This can be a | separated list.')
parser.add_argument('--extra_wipe_partitions', type=str,
help='Additional partitions on device which should be wiped.')
parser.add_argument('-v', action="store_true",
diff --git a/tools/releasetools/ota_utils.py b/tools/releasetools/ota_utils.py
index 0a6ff39..ddd2d36 100644
--- a/tools/releasetools/ota_utils.py
+++ b/tools/releasetools/ota_utils.py
@@ -27,7 +27,8 @@
ZipWriteStr, BuildInfo, LoadDictionaryFromFile,
SignFile, PARTITIONS_WITH_BUILD_PROP, PartitionBuildProps,
GetRamdiskFormat, ParseUpdateEngineConfig)
-from payload_signer import PayloadSigner
+import payload_signer
+from payload_signer import PayloadSigner, AddSigningArgumentParse, GeneratePayloadProperties
logger = logging.getLogger(__name__)
@@ -785,8 +786,8 @@
class PayloadGenerator(object):
"""Manages the creation and the signing of an A/B OTA Payload."""
- PAYLOAD_BIN = 'payload.bin'
- PAYLOAD_PROPERTIES_TXT = 'payload_properties.txt'
+ PAYLOAD_BIN = payload_signer.PAYLOAD_BIN
+ PAYLOAD_PROPERTIES_TXT = payload_signer.PAYLOAD_PROPERTIES_TXT
SECONDARY_PAYLOAD_BIN = 'secondary/payload.bin'
SECONDARY_PAYLOAD_PROPERTIES_TXT = 'secondary/payload_properties.txt'
@@ -905,12 +906,7 @@
"""
assert self.payload_file is not None
# 4. Dump the signed payload properties.
- properties_file = common.MakeTempFile(prefix="payload-properties-",
- suffix=".txt")
- cmd = ["delta_generator",
- "--in_file=" + self.payload_file,
- "--properties_file=" + properties_file]
- self._Run(cmd)
+ properties_file = GeneratePayloadProperties(self.payload_file)
with open(properties_file, "a") as f:
diff --git a/tools/releasetools/payload_signer.py b/tools/releasetools/payload_signer.py
index a5d09e1..e85d64c 100644
--- a/tools/releasetools/payload_signer.py
+++ b/tools/releasetools/payload_signer.py
@@ -17,7 +17,12 @@
import common
import logging
import shlex
+import argparse
+import tempfile
+import zipfile
+import shutil
from common import OPTIONS, OptionHandler
+from ota_signing_utils import AddSigningArgumentParse
logger = logging.getLogger(__name__)
@@ -26,6 +31,8 @@
OPTIONS.payload_signer_maximum_signature_size = None
OPTIONS.package_key = None
+PAYLOAD_BIN = 'payload.bin'
+PAYLOAD_PROPERTIES_TXT = 'payload_properties.txt'
class SignerOptions(OptionHandler):
@@ -165,3 +172,52 @@
cmd = [self.signer] + self.signer_args + ['-in', in_file, '-out', out_file]
common.RunAndCheckOutput(cmd)
return out_file
+
+def GeneratePayloadProperties(payload_file):
+ properties_file = common.MakeTempFile(prefix="payload-properties-",
+ suffix=".txt")
+ cmd = ["delta_generator",
+ "--in_file=" + payload_file,
+ "--properties_file=" + properties_file]
+ common.RunAndCheckOutput(cmd)
+ return properties_file
+
+def SignOtaPackage(input_path, output_path):
+ payload_signer = PayloadSigner(
+ OPTIONS.package_key, OPTIONS.private_key_suffix,
+ None, OPTIONS.payload_signer, OPTIONS.payload_signer_args)
+ common.ZipExclude(input_path, output_path, [PAYLOAD_BIN, PAYLOAD_PROPERTIES_TXT])
+ with tempfile.NamedTemporaryFile() as unsigned_payload, zipfile.ZipFile(input_path, "r", allowZip64=True) as zfp:
+ with zfp.open("payload.bin") as payload_fp:
+ shutil.copyfileobj(payload_fp, unsigned_payload)
+ signed_payload = payload_signer.SignPayload(unsigned_payload.name)
+ properties_file = GeneratePayloadProperties(signed_payload)
+ with zipfile.ZipFile(output_path, "a", compression=zipfile.ZIP_STORED, allowZip64=True) as output_zfp:
+ common.ZipWrite(output_zfp, signed_payload, PAYLOAD_BIN)
+ common.ZipWrite(output_zfp, properties_file, PAYLOAD_PROPERTIES_TXT)
+
+
+def main(argv):
+ parser = argparse.ArgumentParser(
+ prog=argv[0], description="Given a series of .img files, produces a full OTA package that installs thoese images")
+ parser.add_argument("input_ota", type=str,
+ help="Input OTA for signing")
+ parser.add_argument('output_ota', type=str,
+ help='Output OTA for the signed package')
+ parser.add_argument("-v", action="store_true",
+ help="Enable verbose logging", dest="verbose")
+ AddSigningArgumentParse(parser)
+ args = parser.parse_args(argv[1:])
+ input_ota = args.input_ota
+ output_ota = args.output_ota
+ if args.verbose:
+ OPTIONS.verbose = True
+ common.InitLogging()
+ if args.package_key:
+ OPTIONS.package_key = args.package_key
+ logger.info("Re-signing OTA package {}".format(input_ota))
+ SignOtaPackage(input_ota, output_ota)
+
+if __name__ == "__main__":
+ import sys
+ main(sys.argv)
\ No newline at end of file
diff --git a/tools/releasetools/sign_target_files_apks.py b/tools/releasetools/sign_target_files_apks.py
index 5867c6f..4356394 100755
--- a/tools/releasetools/sign_target_files_apks.py
+++ b/tools/releasetools/sign_target_files_apks.py
@@ -146,6 +146,34 @@
--override_apex_keys <path>
Replace all APEX keys with this private key
+
+ -k (--package_key) <key>
+ Key to use to sign the package (default is the value of
+ default_system_dev_certificate from the input target-files's
+ META/misc_info.txt, or "build/make/target/product/security/testkey" if
+ that value is not specified).
+
+ For incremental OTAs, the default value is based on the source
+ target-file, not the target build.
+
+ --payload_signer <signer>
+ Specify the signer when signing the payload and metadata for A/B OTAs.
+ By default (i.e. without this flag), it calls 'openssl pkeyutl' to sign
+ with the package private key. If the private key cannot be accessed
+ directly, a payload signer that knows how to do that should be specified.
+ The signer will be supplied with "-inkey <path_to_key>",
+ "-in <input_file>" and "-out <output_file>" parameters.
+
+ --payload_signer_args <args>
+ Specify the arguments needed for payload signer.
+
+ --payload_signer_maximum_signature_size <signature_size>
+ The maximum signature size (in bytes) that would be generated by the given
+ payload signer. Only meaningful when custom payload signer is specified
+ via '--payload_signer'.
+ If the signer uses a RSA key, this should be the number of bytes to
+ represent the modulus. If it uses an EC key, this is the size of a
+ DER-encoded ECDSA signature.
"""
from __future__ import print_function
@@ -161,7 +189,6 @@
import re
import shutil
import stat
-import subprocess
import sys
import tempfile
import zipfile
@@ -170,6 +197,8 @@
import add_img_to_target_files
import apex_utils
import common
+import payload_signer
+from payload_signer import SignOtaPackage, PAYLOAD_BIN
if sys.hexversion < 0x02070000:
@@ -240,6 +269,20 @@
return filename.endswith(".apex") or filename.endswith(".capex")
+def IsOtaPackage(fp):
+ with zipfile.ZipFile(fp) as zfp:
+ if not PAYLOAD_BIN in zfp.namelist():
+ return False
+ with zfp.open(PAYLOAD_BIN, "r") as payload:
+ magic = payload.read(4)
+ return magic == b"CrAU"
+
+
+def IsEntryOtaPackage(input_zip, filename):
+ with input_zip.open(filename, "r") as fp:
+ return IsOtaPackage(fp)
+
+
def GetApexFilename(filename):
name = os.path.basename(filename)
# Replace the suffix for compressed apex
@@ -514,6 +557,7 @@
return data
+
def IsBuildPropFile(filename):
return filename in (
"SYSTEM/etc/prop.default",
@@ -540,7 +584,7 @@
filename.endswith("/prop.default")
-def ProcessTargetFiles(input_tf_zip, output_tf_zip, misc_info,
+def ProcessTargetFiles(input_tf_zip: zipfile.ZipFile, output_tf_zip, misc_info,
apk_keys, apex_keys, key_passwords,
platform_api_level, codename_to_api_level_map,
compressed_extension):
@@ -628,6 +672,15 @@
" (skipped due to special cert string)" % (name,))
common.ZipWriteStr(output_tf_zip, out_info, data)
+ elif filename.endswith(".zip") and IsEntryOtaPackage(input_tf_zip, filename):
+ logger.info("Re-signing OTA package {}".format(filename))
+ with tempfile.NamedTemporaryFile() as input_ota, tempfile.NamedTemporaryFile() as output_ota:
+ with input_tf_zip.open(filename, "r") as in_fp:
+ shutil.copyfileobj(in_fp, input_ota)
+ input_ota.flush()
+ SignOtaPackage(input_ota.name, output_ota.name)
+ common.ZipWrite(output_tf_zip, output_ota.name, filename,
+ compress_type=zipfile.ZIP_STORED)
# System properties.
elif IsBuildPropFile(filename):
print("Rewriting %s:" % (filename,))
@@ -1501,7 +1554,7 @@
"override_apk_keys=",
"override_apex_keys=",
],
- extra_option_handler=option_handler)
+ extra_option_handler=[option_handler, payload_signer.signer_options])
if len(args) != 2:
common.Usage(__doc__)
@@ -1515,6 +1568,10 @@
allowZip64=True)
misc_info = common.LoadInfoDict(input_zip)
+ if OPTIONS.package_key is None:
+ OPTIONS.package_key = misc_info.get(
+ "default_system_dev_certificate",
+ "build/make/target/product/security/testkey")
BuildKeyMap(misc_info, key_mapping_options)