Merge "Support middleman actions in mixed builds"

android/bazel.go

@@ -222,7 +222,7 @@ var (
 	// Per-module denylist to opt modules out of mixed builds. Such modules will
 	// still be generated via bp2build.
 	mixedBuildsDisabledList = []string{
-		"libc_bionic_ndk",                  // cparsons@, http://b/183213331, Handle generated headers in mixed builds.
+		"libc_bionic_ndk",                  // cparsons@ cc_library_static, depends on //bionic/libc:libsystemproperties
 		"libc_common",                      // cparsons@ cc_library_static, depends on //bionic/libc:libc_nopthread
 		"libc_common_static",               // cparsons@ cc_library_static, depends on //bionic/libc:libc_common
 		"libc_common_shared",               // cparsons@ cc_library_static, depends on //bionic/libc:libc_common

bazel/aquery.go

@@ -81,23 +81,30 @@ type BuildStatement struct {
 	Mnemonic    string
 }
 
-// AqueryBuildStatements returns an array of BuildStatements which should be registered (and output
+// A helper type for aquery processing which facilitates retrieval of path IDs from their
-// to a ninja file) to correspond one-to-one with the given action graph json proto (from a bazel
+// less readable Bazel structures (depset and path fragment).
-// aquery invocation).
+type aqueryArtifactHandler struct {
-func AqueryBuildStatements(aqueryJsonProto []byte) ([]BuildStatement, error) {
+	// Maps middleman artifact Id to input artifact depset ID.
-	buildStatements := []BuildStatement{}
+	// Middleman artifacts are treated as "substitute" artifacts for mixed builds. For example,
-
+	// if we find a middleman action which has outputs [foo, bar], and output [baz_middleman], then,
-	var aqueryResult actionGraphContainer
+	// for each other action which has input [baz_middleman], we add [foo, bar] to the inputs for
-	err := json.Unmarshal(aqueryJsonProto, &aqueryResult)
+	// that action instead.
-
+	middlemanIdToDepsetIds map[int][]int
-	if err != nil {
+	// Maps depset Id to depset struct.
-		return nil, err
+	depsetIdToDepset map[int]depSetOfFiles
-	}
+	// depsetIdToArtifactIdsCache is a memoization of depset flattening, because flattening
+	// may be an expensive operation.
+	depsetIdToArtifactIdsCache map[int][]int
+	// Maps artifact Id to fully expanded path.
+	artifactIdToPath map[int]string
+}
 
+func newAqueryHandler(aqueryResult actionGraphContainer) (*aqueryArtifactHandler, error) {
 	pathFragments := map[int]pathFragment{}
 	for _, pathFragment := range aqueryResult.PathFragments {
 		pathFragments[pathFragment.Id] = pathFragment
 	}
+
 	artifactIdToPath := map[int]string{}
 	for _, artifact := range aqueryResult.Artifacts {
 		artifactPath, err := expandPathFragment(artifact.PathFragmentId, pathFragments)
@@ -112,22 +119,87 @@ func AqueryBuildStatements(aqueryJsonProto []byte) ([]BuildStatement, error) {
 		depsetIdToDepset[depset.Id] = depset
 	}
 
-	// depsetIdToArtifactIdsCache is a memoization of depset flattening, because flattening
-	// may be an expensive operation.
-	depsetIdToArtifactIdsCache := map[int][]int{}
-
 	// Do a pass through all actions to identify which artifacts are middleman artifacts.
-	// These will be omitted from the inputs of other actions.
+	middlemanIdToDepsetIds := map[int][]int{}
-	// TODO(b/180945500): Handle middleman actions; without proper handling, depending on generated
-	// headers may cause build failures.
-	middlemanArtifactIds := map[int]bool{}
 	for _, actionEntry := range aqueryResult.Actions {
 		if actionEntry.Mnemonic == "Middleman" {
 			for _, outputId := range actionEntry.OutputIds {
-				middlemanArtifactIds[outputId] = true
+				middlemanIdToDepsetIds[outputId] = actionEntry.InputDepSetIds
 			}
 		}
 	}
+	return &aqueryArtifactHandler{
+		middlemanIdToDepsetIds:     middlemanIdToDepsetIds,
+		depsetIdToDepset:           depsetIdToDepset,
+		depsetIdToArtifactIdsCache: map[int][]int{},
+		artifactIdToPath:           artifactIdToPath,
+	}, nil
+}
+
+func (a *aqueryArtifactHandler) getInputPaths(depsetIds []int) ([]string, error) {
+	inputPaths := []string{}
+
+	for _, inputDepSetId := range depsetIds {
+		inputArtifacts, err := a.artifactIdsFromDepsetId(inputDepSetId)
+		if err != nil {
+			return nil, err
+		}
+		for _, inputId := range inputArtifacts {
+			if middlemanInputDepsetIds, isMiddlemanArtifact := a.middlemanIdToDepsetIds[inputId]; isMiddlemanArtifact {
+				// Add all inputs from middleman actions which created middleman artifacts which are
+				// in the inputs for this action.
+				swappedInputPaths, err := a.getInputPaths(middlemanInputDepsetIds)
+				if err != nil {
+					return nil, err
+				}
+				inputPaths = append(inputPaths, swappedInputPaths...)
+			} else {
+				inputPath, exists := a.artifactIdToPath[inputId]
+				if !exists {
+					return nil, fmt.Errorf("undefined input artifactId %d", inputId)
+				}
+				inputPaths = append(inputPaths, inputPath)
+			}
+		}
+	}
+	return inputPaths, nil
+}
+
+func (a *aqueryArtifactHandler) artifactIdsFromDepsetId(depsetId int) ([]int, error) {
+	if result, exists := a.depsetIdToArtifactIdsCache[depsetId]; exists {
+		return result, nil
+	}
+	if depset, exists := a.depsetIdToDepset[depsetId]; exists {
+		result := depset.DirectArtifactIds
+		for _, childId := range depset.TransitiveDepSetIds {
+			childArtifactIds, err := a.artifactIdsFromDepsetId(childId)
+			if err != nil {
+				return nil, err
+			}
+			result = append(result, childArtifactIds...)
+		}
+		a.depsetIdToArtifactIdsCache[depsetId] = result
+		return result, nil
+	} else {
+		return nil, fmt.Errorf("undefined input depsetId %d", depsetId)
+	}
+}
+
+// AqueryBuildStatements returns an array of BuildStatements which should be registered (and output
+// to a ninja file) to correspond one-to-one with the given action graph json proto (from a bazel
+// aquery invocation).
+func AqueryBuildStatements(aqueryJsonProto []byte) ([]BuildStatement, error) {
+	buildStatements := []BuildStatement{}
+
+	var aqueryResult actionGraphContainer
+	err := json.Unmarshal(aqueryJsonProto, &aqueryResult)
+	if err != nil {
+		return nil, err
+	}
+	aqueryHandler, err := newAqueryHandler(aqueryResult)
+	if err != nil {
+		return nil, err
+	}
 
 	for _, actionEntry := range aqueryResult.Actions {
 		if shouldSkipAction(actionEntry) {
@@ -136,7 +208,7 @@ func AqueryBuildStatements(aqueryJsonProto []byte) ([]BuildStatement, error) {
 		outputPaths := []string{}
 		var depfile *string
 		for _, outputId := range actionEntry.OutputIds {
-			outputPath, exists := artifactIdToPath[outputId]
+			outputPath, exists := aqueryHandler.artifactIdToPath[outputId]
 			if !exists {
 				return nil, fmt.Errorf("undefined outputId %d", outputId)
 			}
@@ -151,25 +223,11 @@ func AqueryBuildStatements(aqueryJsonProto []byte) ([]BuildStatement, error) {
 				outputPaths = append(outputPaths, outputPath)
 			}
 		}
-		inputPaths := []string{}
+		inputPaths, err := aqueryHandler.getInputPaths(actionEntry.InputDepSetIds)
-		for _, inputDepSetId := range actionEntry.InputDepSetIds {
+		if err != nil {
-			inputArtifacts, err :=
+			return nil, err
-				artifactIdsFromDepsetId(depsetIdToDepset, depsetIdToArtifactIdsCache, inputDepSetId)
-			if err != nil {
-				return nil, err
-			}
-			for _, inputId := range inputArtifacts {
-				if _, isMiddlemanArtifact := middlemanArtifactIds[inputId]; isMiddlemanArtifact {
-					// Omit middleman artifacts.
-					continue
-				}
-				inputPath, exists := artifactIdToPath[inputId]
-				if !exists {
-					return nil, fmt.Errorf("undefined input artifactId %d", inputId)
-				}
-				inputPaths = append(inputPaths, inputPath)
-			}
 		}
+
 		buildStatement := BuildStatement{
 			Command:     strings.Join(proptools.ShellEscapeList(actionEntry.Arguments), " "),
 			Depfile:     depfile,
@@ -192,8 +250,9 @@ func shouldSkipAction(a action) bool {
 	if a.Mnemonic == "Symlink" || a.Mnemonic == "SourceSymlinkManifest" || a.Mnemonic == "SymlinkTree" {
 		return true
 	}
-	// TODO(b/180945500): Handle middleman actions; without proper handling, depending on generated
+	// Middleman actions are not handled like other actions; they are handled separately as a
-	// headers may cause build failures.
+	// preparatory step so that their inputs may be relayed to actions depending on middleman
+	// artifacts.
 	if a.Mnemonic == "Middleman" {
 		return true
 	}
@@ -209,28 +268,6 @@ func shouldSkipAction(a action) bool {
 	return false
 }
 
-func artifactIdsFromDepsetId(depsetIdToDepset map[int]depSetOfFiles,
-	depsetIdToArtifactIdsCache map[int][]int, depsetId int) ([]int, error) {
-	if result, exists := depsetIdToArtifactIdsCache[depsetId]; exists {
-		return result, nil
-	}
-	if depset, exists := depsetIdToDepset[depsetId]; exists {
-		result := depset.DirectArtifactIds
-		for _, childId := range depset.TransitiveDepSetIds {
-			childArtifactIds, err :=
-				artifactIdsFromDepsetId(depsetIdToDepset, depsetIdToArtifactIdsCache, childId)
-			if err != nil {
-				return nil, err
-			}
-			result = append(result, childArtifactIds...)
-		}
-		depsetIdToArtifactIdsCache[depsetId] = result
-		return result, nil
-	} else {
-		return nil, fmt.Errorf("undefined input depsetId %d", depsetId)
-	}
-}
-
 func expandPathFragment(id int, pathFragmentsMap map[int]pathFragment) (string, error) {
 	labels := []string{}
 	currId := id

bazel/aquery_test.go

@@ -718,6 +718,93 @@ func TestTransitiveInputDepsets(t *testing.T) {
 	assertBuildStatements(t, expectedBuildStatements, actualbuildStatements)
 }
 
+func TestMiddlemenAction(t *testing.T) {
+	const inputString = `
+{
+  "artifacts": [{
+    "id": 1,
+    "pathFragmentId": 1
+  }, {
+    "id": 2,
+    "pathFragmentId": 2
+  }, {
+    "id": 3,
+    "pathFragmentId": 3
+  }, {
+    "id": 4,
+    "pathFragmentId": 4
+  }, {
+    "id": 5,
+    "pathFragmentId": 5
+  }, {
+    "id": 6,
+    "pathFragmentId": 6
+  }],
+  "pathFragments": [{
+    "id": 1,
+    "label": "middleinput_one"
+  }, {
+    "id": 2,
+    "label": "middleinput_two"
+  }, {
+    "id": 3,
+    "label": "middleman_artifact"
+  }, {
+    "id": 4,
+    "label": "maininput_one"
+  }, {
+    "id": 5,
+    "label": "maininput_two"
+  }, {
+    "id": 6,
+    "label": "output"
+  }],
+  "depSetOfFiles": [{
+    "id": 1,
+    "directArtifactIds": [1, 2]
+  }, {
+    "id": 2,
+    "directArtifactIds": [3, 4, 5]
+  }],
+  "actions": [{
+    "targetId": 1,
+    "actionKey": "x",
+    "mnemonic": "Middleman",
+    "arguments": ["touch", "foo"],
+    "inputDepSetIds": [1],
+    "outputIds": [3],
+    "primaryOutputId": 3
+  }, {
+    "targetId": 2,
+    "actionKey": "y",
+    "mnemonic": "Main action",
+    "arguments": ["touch", "foo"],
+    "inputDepSetIds": [2],
+    "outputIds": [6],
+    "primaryOutputId": 6
+  }]
+}`
+
+	actual, err := AqueryBuildStatements([]byte(inputString))
+	if err != nil {
+		t.Errorf("Unexpected error %q", err)
+	}
+	if expected := 1; len(actual) != expected {
+		t.Fatalf("Expected %d build statements, got %d", expected, len(actual))
+	}
+
+	bs := actual[0]
+	expectedInputs := []string{"middleinput_one", "middleinput_two", "maininput_one", "maininput_two"}
+	if !reflect.DeepEqual(bs.InputPaths, expectedInputs) {
+		t.Errorf("Expected main action inputs %q, but got %q", expectedInputs, bs.InputPaths)
+	}
+
+	expectedOutputs := []string{"output"}
+	if !reflect.DeepEqual(bs.OutputPaths, expectedOutputs) {
+		t.Errorf("Expected main action outputs %q, but got %q", expectedOutputs, bs.OutputPaths)
+	}
+}
+
 func assertError(t *testing.T, err error, expected string) {
 	if err == nil {
 		t.Errorf("expected error '%s', but got no error", expected)