Merge "cosmetics: readability"

This commit is contained in:
Treehugger Robot
2022-05-31 23:27:45 +00:00
committed by Gerrit Code Review
2 changed files with 48 additions and 44 deletions

View File

@@ -27,17 +27,21 @@ import (
"github.com/google/blueprint/proptools" "github.com/google/blueprint/proptools"
) )
type artifactId int
type depsetId int
type pathFragmentId int
// artifact contains relevant portions of Bazel's aquery proto, Artifact. // artifact contains relevant portions of Bazel's aquery proto, Artifact.
// Represents a single artifact, whether it's a source file or a derived output file. // Represents a single artifact, whether it's a source file or a derived output file.
type artifact struct { type artifact struct {
Id int Id artifactId
PathFragmentId int PathFragmentId pathFragmentId
} }
type pathFragment struct { type pathFragment struct {
Id int Id pathFragmentId
Label string Label string
ParentId int ParentId pathFragmentId
} }
// KeyValuePair represents Bazel's aquery proto, KeyValuePair. // KeyValuePair represents Bazel's aquery proto, KeyValuePair.
@@ -63,9 +67,9 @@ type AqueryDepset struct {
// Represents a data structure containing one or more files. Depsets in Bazel are an efficient // Represents a data structure containing one or more files. Depsets in Bazel are an efficient
// data structure for storing large numbers of file paths. // data structure for storing large numbers of file paths.
type depSetOfFiles struct { type depSetOfFiles struct {
Id int Id depsetId
DirectArtifactIds []int DirectArtifactIds []artifactId
TransitiveDepSetIds []int TransitiveDepSetIds []depsetId
} }
// action contains relevant portions of Bazel's aquery proto, Action. // action contains relevant portions of Bazel's aquery proto, Action.
@@ -73,9 +77,9 @@ type depSetOfFiles struct {
type action struct { type action struct {
Arguments []string Arguments []string
EnvironmentVariables []KeyValuePair EnvironmentVariables []KeyValuePair
InputDepSetIds []int InputDepSetIds []depsetId
Mnemonic string Mnemonic string
OutputIds []int OutputIds []artifactId
TemplateContent string TemplateContent string
Substitutions []KeyValuePair Substitutions []KeyValuePair
} }
@@ -113,20 +117,20 @@ type aqueryArtifactHandler struct {
// Maps depset id to AqueryDepset, a representation of depset which is // Maps depset id to AqueryDepset, a representation of depset which is
// post-processed for middleman artifact handling, unhandled artifact // post-processed for middleman artifact handling, unhandled artifact
// dropping, content hashing, etc. // dropping, content hashing, etc.
depsetIdToAqueryDepset map[int]AqueryDepset depsetIdToAqueryDepset map[depsetId]AqueryDepset
// Maps content hash to AqueryDepset. // Maps content hash to AqueryDepset.
depsetHashToAqueryDepset map[string]AqueryDepset depsetHashToAqueryDepset map[string]AqueryDepset
// depsetIdToArtifactIdsCache is a memoization of depset flattening, because flattening // depsetIdToArtifactIdsCache is a memoization of depset flattening, because flattening
// may be an expensive operation. // may be an expensive operation.
depsetHashToArtifactPathsCache map[string][]string depsetHashToArtifactPathsCache map[string][]string
// Maps artifact ContentHash to fully expanded path. // Maps artifact ids to fully expanded paths.
artifactIdToPath map[int]string artifactIdToPath map[artifactId]string
} }
// The tokens should be substituted with the value specified here, instead of the // The tokens should be substituted with the value specified here, instead of the
// one returned in 'substitutions' of TemplateExpand action. // one returned in 'substitutions' of TemplateExpand action.
var TemplateActionOverriddenTokens = map[string]string{ var templateActionOverriddenTokens = map[string]string{
// Uses "python3" for %python_binary% instead of the value returned by aquery // Uses "python3" for %python_binary% instead of the value returned by aquery
// which is "py3wrapper.sh". See removePy3wrapperScript. // which is "py3wrapper.sh". See removePy3wrapperScript.
"%python_binary%": "python3", "%python_binary%": "python3",
@@ -138,13 +142,20 @@ var manifestFilePattern = regexp.MustCompile(".*/.+\\.runfiles/MANIFEST$")
// The file name of py3wrapper.sh, which is used by py_binary targets. // The file name of py3wrapper.sh, which is used by py_binary targets.
const py3wrapperFileName = "/py3wrapper.sh" const py3wrapperFileName = "/py3wrapper.sh"
func newAqueryHandler(aqueryResult actionGraphContainer) (*aqueryArtifactHandler, error) { func indexBy[K comparable, V any](values []V, keyFn func(v V) K) map[K]V {
pathFragments := map[int]pathFragment{} m := map[K]V{}
for _, pathFragment := range aqueryResult.PathFragments { for _, v := range values {
pathFragments[pathFragment.Id] = pathFragment m[keyFn(v)] = v
}
return m
} }
artifactIdToPath := map[int]string{} func newAqueryHandler(aqueryResult actionGraphContainer) (*aqueryArtifactHandler, error) {
pathFragments := indexBy(aqueryResult.PathFragments, func(pf pathFragment) pathFragmentId {
return pf.Id
})
artifactIdToPath := map[artifactId]string{}
for _, artifact := range aqueryResult.Artifacts { for _, artifact := range aqueryResult.Artifacts {
artifactPath, err := expandPathFragment(artifact.PathFragmentId, pathFragments) artifactPath, err := expandPathFragment(artifact.PathFragmentId, pathFragments)
if err != nil { if err != nil {
@@ -158,7 +169,7 @@ func newAqueryHandler(aqueryResult actionGraphContainer) (*aqueryArtifactHandler
// if we find a middleman action which has outputs [foo, bar], and output [baz_middleman], then, // if we find a middleman action which has outputs [foo, bar], and output [baz_middleman], then,
// for each other action which has input [baz_middleman], we add [foo, bar] to the inputs for // for each other action which has input [baz_middleman], we add [foo, bar] to the inputs for
// that action instead. // that action instead.
middlemanIdToDepsetIds := map[int][]int{} middlemanIdToDepsetIds := map[artifactId][]depsetId{}
for _, actionEntry := range aqueryResult.Actions { for _, actionEntry := range aqueryResult.Actions {
if actionEntry.Mnemonic == "Middleman" { if actionEntry.Mnemonic == "Middleman" {
for _, outputId := range actionEntry.OutputIds { for _, outputId := range actionEntry.OutputIds {
@@ -167,19 +178,12 @@ func newAqueryHandler(aqueryResult actionGraphContainer) (*aqueryArtifactHandler
} }
} }
// Store all depset IDs to validate all depset links are resolvable. depsetIdToDepset := indexBy(aqueryResult.DepSetOfFiles, func(d depSetOfFiles) depsetId {
depsetIds := map[int]bool{} return d.Id
for _, depset := range aqueryResult.DepSetOfFiles { })
depsetIds[depset.Id] = true
}
depsetIdToDepset := map[int]depSetOfFiles{}
for _, depset := range aqueryResult.DepSetOfFiles {
depsetIdToDepset[depset.Id] = depset
}
aqueryHandler := aqueryArtifactHandler{ aqueryHandler := aqueryArtifactHandler{
depsetIdToAqueryDepset: map[int]AqueryDepset{}, depsetIdToAqueryDepset: map[depsetId]AqueryDepset{},
depsetHashToAqueryDepset: map[string]AqueryDepset{}, depsetHashToAqueryDepset: map[string]AqueryDepset{},
depsetHashToArtifactPathsCache: map[string][]string{}, depsetHashToArtifactPathsCache: map[string][]string{},
artifactIdToPath: artifactIdToPath, artifactIdToPath: artifactIdToPath,
@@ -198,12 +202,12 @@ func newAqueryHandler(aqueryResult actionGraphContainer) (*aqueryArtifactHandler
// Ensures that the handler's depsetIdToAqueryDepset map contains an entry for the given // Ensures that the handler's depsetIdToAqueryDepset map contains an entry for the given
// depset. // depset.
func (a *aqueryArtifactHandler) populateDepsetMaps(depset depSetOfFiles, middlemanIdToDepsetIds map[int][]int, depsetIdToDepset map[int]depSetOfFiles) (AqueryDepset, error) { func (a *aqueryArtifactHandler) populateDepsetMaps(depset depSetOfFiles, middlemanIdToDepsetIds map[artifactId][]depsetId, depsetIdToDepset map[depsetId]depSetOfFiles) (AqueryDepset, error) {
if aqueryDepset, containsDepset := a.depsetIdToAqueryDepset[depset.Id]; containsDepset { if aqueryDepset, containsDepset := a.depsetIdToAqueryDepset[depset.Id]; containsDepset {
return aqueryDepset, nil return aqueryDepset, nil
} }
transitiveDepsetIds := depset.TransitiveDepSetIds transitiveDepsetIds := depset.TransitiveDepSetIds
directArtifactPaths := []string{} var directArtifactPaths []string
for _, artifactId := range depset.DirectArtifactIds { for _, artifactId := range depset.DirectArtifactIds {
path, pathExists := a.artifactIdToPath[artifactId] path, pathExists := a.artifactIdToPath[artifactId]
if !pathExists { if !pathExists {
@@ -232,7 +236,7 @@ func (a *aqueryArtifactHandler) populateDepsetMaps(depset depSetOfFiles, middlem
} }
} }
childDepsetHashes := []string{} var childDepsetHashes []string
for _, childDepsetId := range transitiveDepsetIds { for _, childDepsetId := range transitiveDepsetIds {
childDepset, exists := depsetIdToDepset[childDepsetId] childDepset, exists := depsetIdToDepset[childDepsetId]
if !exists { if !exists {
@@ -258,8 +262,8 @@ func (a *aqueryArtifactHandler) populateDepsetMaps(depset depSetOfFiles, middlem
// input paths contained in these depsets. // input paths contained in these depsets.
// This is a potentially expensive operation, and should not be invoked except // This is a potentially expensive operation, and should not be invoked except
// for actions which need specialized input handling. // for actions which need specialized input handling.
func (a *aqueryArtifactHandler) getInputPaths(depsetIds []int) ([]string, error) { func (a *aqueryArtifactHandler) getInputPaths(depsetIds []depsetId) ([]string, error) {
inputPaths := []string{} var inputPaths []string
for _, inputDepSetId := range depsetIds { for _, inputDepSetId := range depsetIds {
depset := a.depsetIdToAqueryDepset[inputDepSetId] depset := a.depsetIdToAqueryDepset[inputDepSetId]
@@ -338,7 +342,7 @@ func AqueryBuildStatements(aqueryJsonProto []byte) ([]BuildStatement, []AqueryDe
} }
depsetsByHash := map[string]AqueryDepset{} depsetsByHash := map[string]AqueryDepset{}
depsets := []AqueryDepset{} var depsets []AqueryDepset
for _, aqueryDepset := range aqueryHandler.depsetIdToAqueryDepset { for _, aqueryDepset := range aqueryHandler.depsetIdToAqueryDepset {
if prevEntry, hasKey := depsetsByHash[aqueryDepset.ContentHash]; hasKey { if prevEntry, hasKey := depsetsByHash[aqueryDepset.ContentHash]; hasKey {
// Two depsets collide on hash. Ensure that their contents are identical. // Two depsets collide on hash. Ensure that their contents are identical.
@@ -391,8 +395,8 @@ func depsetContentHash(directPaths []string, transitiveDepsetHashes []string) st
return fullHash return fullHash
} }
func (a *aqueryArtifactHandler) depsetContentHashes(inputDepsetIds []int) ([]string, error) { func (a *aqueryArtifactHandler) depsetContentHashes(inputDepsetIds []depsetId) ([]string, error) {
hashes := []string{} var hashes []string
for _, depsetId := range inputDepsetIds { for _, depsetId := range inputDepsetIds {
if aqueryDepset, exists := a.depsetIdToAqueryDepset[depsetId]; !exists { if aqueryDepset, exists := a.depsetIdToAqueryDepset[depsetId]; !exists {
return nil, fmt.Errorf("undefined input depsetId %d", depsetId) return nil, fmt.Errorf("undefined input depsetId %d", depsetId)
@@ -452,7 +456,7 @@ func (a *aqueryArtifactHandler) pythonZipperActionBuildStatement(actionEntry act
// See go/python-binary-host-mixed-build for more details. // See go/python-binary-host-mixed-build for more details.
pythonZipFilePath := outputPaths[0] pythonZipFilePath := outputPaths[0]
pyBinaryFound := false pyBinaryFound := false
for i, _ := range prevBuildStatements { for i := range prevBuildStatements {
if len(prevBuildStatements[i].OutputPaths) == 1 && prevBuildStatements[i].OutputPaths[0]+".zip" == pythonZipFilePath { if len(prevBuildStatements[i].OutputPaths) == 1 && prevBuildStatements[i].OutputPaths[0]+".zip" == pythonZipFilePath {
prevBuildStatements[i].InputPaths = append(prevBuildStatements[i].InputPaths, pythonZipFilePath) prevBuildStatements[i].InputPaths = append(prevBuildStatements[i].InputPaths, pythonZipFilePath)
pyBinaryFound = true pyBinaryFound = true
@@ -565,7 +569,7 @@ func expandTemplateContent(actionEntry action) string {
replacerString := []string{} replacerString := []string{}
for _, pair := range actionEntry.Substitutions { for _, pair := range actionEntry.Substitutions {
value := pair.Value value := pair.Value
if val, ok := TemplateActionOverriddenTokens[pair.Key]; ok { if val, ok := templateActionOverriddenTokens[pair.Key]; ok {
value = val value = val
} }
replacerString = append(replacerString, pair.Key, value) replacerString = append(replacerString, pair.Key, value)
@@ -662,8 +666,8 @@ func shouldSkipAction(a action) bool {
return false return false
} }
func expandPathFragment(id int, pathFragmentsMap map[int]pathFragment) (string, error) { func expandPathFragment(id pathFragmentId, pathFragmentsMap map[pathFragmentId]pathFragment) (string, error) {
labels := []string{} var labels []string
currId := id currId := id
// Only positive IDs are valid for path fragments. An ID of zero indicates a terminal node. // Only positive IDs are valid for path fragments. An ID of zero indicates a terminal node.
for currId > 0 { for currId > 0 {

2
go.mod
View File

@@ -16,4 +16,4 @@ replace github.com/google/go-cmp v0.5.5 => ../../external/go-cmp
// Indirect dep from go-cmp // Indirect dep from go-cmp
exclude golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 exclude golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543
go 1.15 go 1.18