Merge "Retry: Make ConfiguredJarList immutable"

android/config.go

@@ -1361,14 +1361,31 @@ func (l *ConfiguredJarList) IndexOfJar(jar string) int {
 	return IndexList(jar, l.jars)
 }
 
+func copyAndAppend(list []string, item string) []string {
+	// Create the result list to be 1 longer than the input.
+	result := make([]string, len(list)+1)
+
+	// Copy the whole input list into the result.
+	count := copy(result, list)
+
+	// Insert the extra item at the end.
+	result[count] = item
+
+	return result
+}
+
 // Append an (apex, jar) pair to the list.
-func (l *ConfiguredJarList) Append(apex string, jar string) {
+func (l *ConfiguredJarList) Append(apex string, jar string) ConfiguredJarList {
-	l.apexes = append(l.apexes, apex)
+	// Create a copy of the backing arrays before appending to avoid sharing backing
-	l.jars = append(l.jars, jar)
+	// arrays that are mutated across instances.
+	apexes := copyAndAppend(l.apexes, apex)
+	jars := copyAndAppend(l.jars, jar)
+
+	return ConfiguredJarList{apexes, jars}
 }
 
 // Filter out sublist.
-func (l *ConfiguredJarList) RemoveList(list ConfiguredJarList) {
+func (l *ConfiguredJarList) RemoveList(list ConfiguredJarList) ConfiguredJarList {
 	apexes := make([]string, 0, l.Len())
 	jars := make([]string, 0, l.Len())
 
@@ -1380,13 +1397,7 @@ func (l *ConfiguredJarList) RemoveList(list ConfiguredJarList) {
 		}
 	}
 
-	l.apexes = apexes
+	return ConfiguredJarList{apexes, jars}
-	l.jars = jars
-}
-
-// A copy of itself.
-func (l *ConfiguredJarList) CopyOf() ConfiguredJarList {
-	return ConfiguredJarList{CopyOf(l.apexes), CopyOf(l.jars)}
 }
 
 // A copy of the list of strings containing jar components.
@@ -1469,6 +1480,14 @@ func (l *ConfiguredJarList) DevicePaths(cfg Config, ostype OsType) []string {
 	return paths
 }
 
+func (l *ConfiguredJarList) String() string {
+	var pairs []string
+	for i := 0; i < l.Len(); i++ {
+		pairs = append(pairs, l.apexes[i]+":"+l.jars[i])
+	}
+	return strings.Join(pairs, ",")
+}
+
 func splitListOfPairsIntoPairOfLists(list []string) ([]string, []string, error) {
 	// Now we need to populate this list by splitting each item in the slice of
 	// pairs and appending them to the appropriate list of apexes or jars.

android/config_test.go

@@ -91,3 +91,49 @@ func TestMissingVendorConfig(t *testing.T) {
 		t.Errorf("Expected false")
 	}
 }
+
+func assertStringEquals(t *testing.T, expected, actual string) {
+	if actual != expected {
+		t.Errorf("expected %q found %q", expected, actual)
+	}
+}
+
+func TestConfiguredJarList(t *testing.T) {
+	list1 := CreateTestConfiguredJarList([]string{"apex1:jarA"})
+
+	t.Run("create", func(t *testing.T) {
+		assertStringEquals(t, "apex1:jarA", list1.String())
+	})
+
+	list2 := list1.Append("apex2", "jarB")
+	t.Run("append", func(t *testing.T) {
+		assertStringEquals(t, "apex1:jarA,apex2:jarB", list2.String())
+	})
+
+	t.Run("append does not modify", func(t *testing.T) {
+		assertStringEquals(t, "apex1:jarA", list1.String())
+	})
+
+	// Make sure that two lists created by appending to the same list do not share storage.
+	list3 := list1.Append("apex3", "jarC")
+	t.Run("append does not share", func(t *testing.T) {
+		assertStringEquals(t, "apex1:jarA,apex2:jarB", list2.String())
+		assertStringEquals(t, "apex1:jarA,apex3:jarC", list3.String())
+	})
+
+	list4 := list3.RemoveList(list1)
+	t.Run("remove", func(t *testing.T) {
+		assertStringEquals(t, "apex3:jarC", list4.String())
+	})
+
+	t.Run("remove does not modify", func(t *testing.T) {
+		assertStringEquals(t, "apex1:jarA,apex3:jarC", list3.String())
+	})
+
+	// Make sure that two lists created by removing from the same list do not share storage.
+	list5 := list3.RemoveList(CreateTestConfiguredJarList([]string{"apex3:jarC"}))
+	t.Run("remove", func(t *testing.T) {
+		assertStringEquals(t, "apex3:jarC", list4.String())
+		assertStringEquals(t, "apex1:jarA", list5.String())
+	})
+}

java/dexpreopt_config.go

@@ -81,13 +81,12 @@ func genBootImageConfigs(ctx android.PathContext) map[string]*bootImageConfig {
 		targets := dexpreoptTargets(ctx)
 		deviceDir := android.PathForOutput(ctx, ctx.Config().DeviceName())
 
-		artModules := global.ArtApexJars.CopyOf()
+		artModules := global.ArtApexJars
 		// With EMMA_INSTRUMENT_FRAMEWORK=true the Core libraries depend on jacoco.
 		if ctx.Config().IsEnvTrue("EMMA_INSTRUMENT_FRAMEWORK") {
-			artModules.Append("com.android.art", "jacocoagent")
+			artModules = artModules.Append("com.android.art", "jacocoagent")
 		}
-		frameworkModules := global.BootJars.CopyOf()
+		frameworkModules := global.BootJars.RemoveList(artModules)
-		frameworkModules.RemoveList(artModules)
 
 		artSubdir := "apex/art_boot_images/javalib"
 		frameworkSubdir := "system/framework"