diff --git a/tools/metadata/Android.bp b/tools/metadata/Android.bp index b2fabecb96..77d106d705 100644 --- a/tools/metadata/Android.bp +++ b/tools/metadata/Android.bp @@ -6,6 +6,8 @@ blueprint_go_binary { 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 e970e1708f..d328876ed2 100644 --- a/tools/metadata/generator.go +++ b/tools/metadata/generator.go @@ -10,6 +10,8 @@ import ( "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 @@ func (kl *keyToLocksMap) GetLockForKey(key string) *sync.Mutex { 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 @@ func getSortedKeys(syncMap *sync.Map) []string { 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 writeNewlineToOutputFile(outputFile string) { } 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 @@ func processTestSpecProtobuf( 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 @@ func processTestSpecProtobuf( } } +// 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 @@ func main() { 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 @@ func main() { 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 23875daf3d..f2cdf8ec98 100644 --- a/tools/metadata/go.work +++ b/tools/metadata/go.work @@ -4,7 +4,8 @@ use ( . ../../../../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 0000000000..755cf40a30 --- /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 0000000000..d8de06457d --- /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 0000000000..9c7cdcd505 --- /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 0000000000..d8de06457d --- /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 0000000000..a931690022 --- /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 0000000000..755cf40a30 --- /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 0000000000..755cf40a30 --- /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 0000000000..7a81b7d523 --- /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 0000000000..26668e44a9 --- /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 71856fe606..314add352f 100644 --- a/tools/metadata/testdata/metadata_test.go +++ b/tools/metadata/testdata/metadata_test.go @@ -87,3 +87,33 @@ func TestEmptyInputFile(t *testing.T) { 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") + } +}