Add support for name-less modules and property comments

Bug: 181569894
Test: m nothing
Change-Id: Ia4da1d2a55a924db82ae999da455adedbaca47c0

android/sdk.go

@@ -284,11 +284,20 @@ type BpPropertySet interface {
 	// Add a property set with the specified name and return so that additional
 	// properties can be added.
 	AddPropertySet(name string) BpPropertySet
+
+	// Add comment for property (or property set).
+	AddCommentForProperty(name, text string)
 }
 
 // A .bp module definition.
 type BpModule interface {
 	BpPropertySet
+
+	// ModuleType returns the module type of the module
+	ModuleType() string
+
+	// Name returns the name of the module or "" if no name has been specified.
+	Name() string
 }
 
 // An individual member of the SDK, includes all of the variants that the SDK

sdk/bp.go

@@ -25,6 +25,7 @@ import (
 type bpPropertySet struct {
 	properties map[string]interface{}
 	tags       map[string]android.BpPropertyTag
+	comments   map[string]string
 	order      []string
 }
 
@@ -133,10 +134,22 @@ func (s *bpPropertySet) getValue(name string) interface{} {
 	return s.properties[name]
 }
 
+func (s *bpPropertySet) getOptionalValue(name string) (interface{}, bool) {
+	value, ok := s.properties[name]
+	return value, ok
+}
+
 func (s *bpPropertySet) getTag(name string) interface{} {
 	return s.tags[name]
 }
 
+func (s *bpPropertySet) AddCommentForProperty(name, text string) {
+	if s.comments == nil {
+		s.comments = map[string]string{}
+	}
+	s.comments[name] = strings.TrimSpace(text)
+}
+
 func (s *bpPropertySet) transformContents(transformer bpPropertyTransformer) {
 	var newOrder []string
 	for _, name := range s.order {
@@ -222,6 +235,19 @@ type bpModule struct {
 	moduleType string
 }
 
+func (m *bpModule) ModuleType() string {
+	return m.moduleType
+}
+
+func (m *bpModule) Name() string {
+	name, hasName := m.getOptionalValue("name")
+	if hasName {
+		return name.(string)
+	} else {
+		return ""
+	}
+}
+
 var _ android.BpModule = (*bpModule)(nil)
 
 type bpPropertyTransformer interface {
@@ -352,16 +378,26 @@ type bpFile struct {
 // is unique within this file.
 func (f *bpFile) AddModule(module android.BpModule) {
 	m := module.(*bpModule)
-	if name, ok := m.getValue("name").(string); ok {
-		if f.modules[name] != nil {
-			panic(fmt.Sprintf("Module %q already exists in bp file", name))
-		}
-
-		f.modules[name] = m
-		f.order = append(f.order, m)
-	} else {
-		panic("Module does not have a name property, or it is not a string")
+	moduleType := module.ModuleType()
+	name := m.Name()
+	hasName := true
+	if name == "" {
+		// Use a prefixed module type as the name instead just in case this is something like a package
+		// of namespace module which does not require a name.
+		name = "#" + moduleType
+		hasName = false
 	}
+
+	if f.modules[name] != nil {
+		if hasName {
+			panic(fmt.Sprintf("Module %q already exists in bp file", name))
+		} else {
+			panic(fmt.Sprintf("Unnamed module type %q already exists in bp file", moduleType))
+		}
+	}
+
+	f.modules[name] = m
+	f.order = append(f.order, m)
 }
 
 func (f *bpFile) newModule(moduleType string) *bpModule {

sdk/update.go

@@ -266,8 +266,11 @@ func (s *sdk) buildSnapshot(ctx android.ModuleContext, sdkVariants []*sdk) andro
 	}
 	s.builderForTests = builder
 
-	// Create the prebuilt modules for each of the member modules.
+	// Group the variants for each member module together and then group the members of each member
+	// type together.
 	members := s.groupMemberVariantsByMemberThenType(ctx, memberVariantDeps)
+
+	// Create the prebuilt modules for each of the member modules.
 	for _, member := range members {
 		memberType := member.memberType
 
@@ -613,7 +616,7 @@ type unversionedToVersionedTransformation struct {
 func (t unversionedToVersionedTransformation) transformModule(module *bpModule) *bpModule {
 	// Use a versioned name for the module but remember the original name for the
 	// snapshot.
-	name := module.getValue("name").(string)
+	name := module.Name()
 	module.setProperty("name", t.builder.versionedSdkMemberName(name, true))
 	module.insertAfter("name", "sdk_member_name", name)
 	// Remove the prefer property if present as versioned modules never need marking with prefer.
@@ -637,7 +640,7 @@ type unversionedTransformation struct {
 
 func (t unversionedTransformation) transformModule(module *bpModule) *bpModule {
 	// If the module is an internal member then use a unique name for it.
-	name := module.getValue("name").(string)
+	name := module.Name()
 	module.setProperty("name", t.builder.unversionedSdkMemberName(name, true))
 	return module
 }
@@ -689,12 +692,26 @@ func generateFilteredBpContents(contents *generatedContents, bpFile *bpFile, mod
 func outputPropertySet(contents *generatedContents, set *bpPropertySet) {
 	contents.Indent()
 
+	addComment := func(name string) {
+		if text, ok := set.comments[name]; ok {
+			for _, line := range strings.Split(text, "\n") {
+				contents.Printfln("// %s", line)
+			}
+		}
+	}
+
 	// Output the properties first, followed by the nested sets. This ensures a
 	// consistent output irrespective of whether property sets are created before
 	// or after the properties. This simplifies the creation of the module.
 	for _, name := range set.order {
 		value := set.getValue(name)
 
+		// Do not write property sets in the properties phase.
+		if _, ok := value.(*bpPropertySet); ok {
+			continue
+		}
+
+		addComment(name)
 		switch v := value.(type) {
 		case []string:
 			length := len(v)
@@ -715,9 +732,6 @@ func outputPropertySet(contents *generatedContents, set *bpPropertySet) {
 		case bool:
 			contents.Printfln("%s: %t,", name, v)
 
-		case *bpPropertySet:
-			// Do not write property sets in the properties phase.
-
 		default:
 			contents.Printfln("%s: %q,", name, value)
 		}
@@ -729,6 +743,7 @@ func outputPropertySet(contents *generatedContents, set *bpPropertySet) {
 		// Only write property sets in the sets phase.
 		switch v := value.(type) {
 		case *bpPropertySet:
+			addComment(name)
 			contents.Printfln("%s: {", name)
 			outputPropertySet(contents, v)
 			contents.Printfln("},")
@@ -747,7 +762,9 @@ func (s *sdk) GetAndroidBpContentsForTests() string {
 func (s *sdk) GetUnversionedAndroidBpContentsForTests() string {
 	contents := &generatedContents{}
 	generateFilteredBpContents(contents, s.builderForTests.bpFile, func(module *bpModule) bool {
-		return !strings.Contains(module.properties["name"].(string), "@")
+		name := module.Name()
+		// Include modules that are either unversioned or have no name.
+		return !strings.Contains(name, "@")
 	})
 	return contents.content.String()
 }
@@ -755,7 +772,9 @@ func (s *sdk) GetUnversionedAndroidBpContentsForTests() string {
 func (s *sdk) GetVersionedAndroidBpContentsForTests() string {
 	contents := &generatedContents{}
 	generateFilteredBpContents(contents, s.builderForTests.bpFile, func(module *bpModule) bool {
-		return strings.Contains(module.properties["name"].(string), "@")
+		name := module.Name()
+		// Include modules that are either versioned or have no name.
+		return name == "" || strings.Contains(name, "@")
 	})
 	return contents.content.String()
 }