diff --git a/core/common/BUILD.bazel b/core/common/BUILD.bazel index d18e3e92..4d28f1ca 100644 --- a/core/common/BUILD.bazel +++ b/core/common/BUILD.bazel @@ -2,17 +2,28 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "common", - srcs = ["utils.go"], + srcs = [ + "nameidmapper.go", + "utils.go", + ], importpath = "github.com/uber/tango/core/common", visibility = ["//visibility:public"], - deps = ["//tangopb"], + deps = [ + "//core/targethasher", + "//tangopb", + "@com_github_bazelbuild_buildtools//build_proto", + ], ) go_test( name = "common_test", - srcs = ["utils_test.go"], + srcs = [ + "nameidmapper_test.go", + "utils_test.go", + ], embed = [":common"], deps = [ "//tangopb", + "@com_github_stretchr_testify//assert", ], ) diff --git a/core/common/nameidmapper.go b/core/common/nameidmapper.go new file mode 100644 index 00000000..3ba0bb5c --- /dev/null +++ b/core/common/nameidmapper.go @@ -0,0 +1,41 @@ +package common + +// NameIDMapper assigns stable int32 IDs to string names on demand. +// IDs are assigned sequentially starting from 0. +type NameIDMapper struct { + nameToID map[string]int32 + nextID int32 +} + +// NewNameIdMapper creates a new NameIdMapper. +func NewNameIDMapper() *NameIDMapper { + return &NameIDMapper{ + nameToID: make(map[string]int32), + nextID: 0, + } +} + +// ID returns the existing ID for the provided name or assigns a new one. +func (a *NameIDMapper) ID(name string) int32 { + if id, ok := a.nameToID[name]; ok { + return id + } + id := a.nextID + a.nextID++ + a.nameToID[name] = id + return id +} + +// Mapping returns the underlying name->id map. +func (a *NameIDMapper) Mapping() map[string]int32 { + return a.nameToID +} + +// Invert returns an id->name map built from the current name->id map. +func (a *NameIDMapper) Invert() map[int32]string { + out := make(map[int32]string, len(a.nameToID)) + for name, id := range a.nameToID { + out[id] = name + } + return out +} diff --git a/core/common/nameidmapper_test.go b/core/common/nameidmapper_test.go new file mode 100644 index 00000000..a97cfa33 --- /dev/null +++ b/core/common/nameidmapper_test.go @@ -0,0 +1,46 @@ +package common + +import ( + "testing" + "github.com/stretchr/testify/assert" +) + +func TestNameIDMapper_AssignsSequentialAndStableIDs(t *testing.T) { + mapper := NewNameIDMapper() + + idA := mapper.ID("a") + assert.Equal(t, int32(0), idA) + idB := mapper.ID("b") + assert.Equal(t, int32(1), idB) + // Re-requesting 'a' should return the same id + idA2 := mapper.ID("a") + assert.Equal(t, idA, idA2) + // A new, third name should get the next sequential id + idC := mapper.ID("c") + assert.Equal(t, int32(2), idC) +} + +func TestNameIDMapper_MappingAndInvert(t *testing.T) { + mapper := NewNameIDMapper() + names := []string{"x", "y", "z"} + for _, n := range names { + mapper.ID(n) + } + + mapping := mapper.Mapping() + assert.Equal(t, 3, len(mapping)) + assert.Equal(t, int32(0), mapping["x"]) + assert.Equal(t, int32(1), mapping["y"]) + assert.Equal(t, int32(2), mapping["z"]) + + inv := mapper.Invert() + assert.Equal(t, 3, len(inv)) + assert.Equal(t, "x", inv[0]) + assert.Equal(t, "y", inv[1]) + assert.Equal(t, "z", inv[2]) + + // Mutating the inverted map should not affect the original mapping + inv[3] = "extra" + _, ok := mapping["extra"] + assert.False(t, ok) +} diff --git a/core/common/utils.go b/core/common/utils.go index 5948465c..8f4127a5 100644 --- a/core/common/utils.go +++ b/core/common/utils.go @@ -1,10 +1,13 @@ package common import ( - "github.com/uber/tango/tangopb" "encoding/base64" "path/filepath" "strings" + + buildpb "github.com/bazelbuild/buildtools/build_proto" + "github.com/uber/tango/core/targethasher" + "github.com/uber/tango/tangopb" ) // ToShortRemote returns the short remote name given a git ssh remote string. @@ -33,3 +36,111 @@ func getReqsBase64(requestURLs []string) string { } return strings.Join(encodedURLs, "-") } + + + +// ResultToGetTargetGraphResponse converts a Result to a GetTargetGraphResponse +func ResultToGetTargetGraphResponse(result targethasher.Result) ([]*tangopb.GetTargetGraphResponse, error) { + // Map target names to ids. This list is topologically sorted, so the ids are stable. + targetNamesMapping := make(map[string]int32, len(result.TargetNames)) + for i, name := range result.TargetNames { + targetNamesMapping[name] = int32(i) + } + + ruleTypeMapper := NewNameIDMapper() + getRuleTypeID := func(key string) int32 { return ruleTypeMapper.ID(key) } + + tagMapper := NewNameIDMapper() + getTagID := func(key string) int32 { return tagMapper.ID(key) } + + attrNameMapper := NewNameIDMapper() + getAttrNameID := func(key string) int32 { return attrNameMapper.ID(key) } + + attrStrValMapper := NewNameIDMapper() + getAttrStrValID := func(key string) int32 { return attrStrValMapper.ID(key) } + + // Build the optimized targets slice + optimizedTargets := make([]*tangopb.OptimizedTarget, 0, len(result.Targets)) + + for _, t := range result.Targets { + nameID := targetNamesMapping[t.Name] + + depIDs := make([]int32, 0, len(t.Deps)) + for _, depName := range t.Deps { + if _, ok := targetNamesMapping[depName]; !ok { + continue + } + depIDs = append(depIDs, targetNamesMapping[depName]) + } + + ot := &tangopb.OptimizedTarget{ + Id: nameID, + Hash: string(t.Hash), + DirectDependencies: depIDs, + } + + // RuleType + if t.RuleType != "" { + id := getRuleTypeID(t.RuleType) + ot.RuleType = id + } + + // Tags + if len(t.Tags) > 0 { + tagIDs := make([]int32, 0, len(t.Tags)) + for _, tag := range t.Tags { + tagIDs = append(tagIDs, getTagID(tag)) + } + ot.Tags = tagIDs + } + ot.Root = t.Root + ot.External = t.External + if len(t.Attributes) > 0 { + attrs := make(map[int32]int32, len(t.Attributes)) + for _, attr := range t.Attributes { + // Only include STRING attributes with non-nil name and value to avoid nil dereferences. + if attr.GetType() == buildpb.Attribute_STRING && attr.Name != nil && attr.StringValue != nil { + nameID := getAttrNameID(*attr.Name) + valID := getAttrStrValID(*attr.StringValue) + attrs[nameID] = valID + } + } + ot.Attributes = attrs + } + + optimizedTargets = append(optimizedTargets, ot) + } + + // Invert mappings: string -> id => id -> string + targetIDToName := make(map[int32]string, len(targetNamesMapping)) + for s, id := range targetNamesMapping { + targetIDToName[id] = s + } + + ruleTypeIDToName := ruleTypeMapper.Invert() + tagIDToName := tagMapper.Invert() + attrNameIDToName := attrNameMapper.Invert() + attrStrValIDToVal := attrStrValMapper.Invert() + + // Assemble final OptimizedTargets + return []*tangopb.GetTargetGraphResponse{ + { + Item: &tangopb.GetTargetGraphResponse_Targets{ + Targets: &tangopb.OptimizedTargets{ + Targets: optimizedTargets, + }, + }, + }, + { + Item: &tangopb.GetTargetGraphResponse_Metadata{ + Metadata: &tangopb.Metadata{ + TargetIdMapping: targetIDToName, + RuleTypeMapping: ruleTypeIDToName, + TagMapping: tagIDToName, + AttributeNameMapping: attrNameIDToName, + AttributeStringValueMapping: attrStrValIDToVal, + }, + }, + }, + }, nil +} diff --git a/core/config/BUILD.bazel b/core/config/BUILD.bazel index 22d7ec34..0c5cf4bd 100644 --- a/core/config/BUILD.bazel +++ b/core/config/BUILD.bazel @@ -5,5 +5,5 @@ go_library( srcs = ["config.go"], importpath = "github.com/uber/tango/core/config", visibility = ["//visibility:public"], - deps = ["@com_github_goccy_go_yaml//:go_default_library"], + deps = ["@com_github_goccy_go_yaml//:go-yaml"], ) diff --git a/core/targethasher/graph.go b/core/targethasher/graph.go index ae26f6ca..5bb0edf7 100644 --- a/core/targethasher/graph.go +++ b/core/targethasher/graph.go @@ -6,7 +6,6 @@ import ( "sort" "strings" - "github.com/uber/tango/tangopb" buildpb "github.com/bazelbuild/buildtools/build_proto" ) @@ -506,162 +505,3 @@ func hasAnyPrefix(t *buildpb.Target, rootRulePrefixes []string) bool { } return false } - -// ResultToGetTargetGraphResponse converts a Result to a GetTargetGraphResponse -func ResultToGetTargetGraphResponse(result Result) ([]*tangopb.GetTargetGraphResponse, error) { - // Map target names to ids. This list is topologically sorted, so the ids are stable. - targetNamesMapping := make(map[string]int32, len(result.TargetNames)) - for i, name := range result.TargetNames { - targetNamesMapping[name] = int32(i) - } - - ruleTypeMapping := make(map[string]int32) - var ruleTypeCurr int32 - getRuleTypeID := func(key string) int32 { - if id, ok := ruleTypeMapping[key]; ok { - return id - } - id := ruleTypeCurr - ruleTypeCurr++ - ruleTypeMapping[key] = id - return id - } - - tagMapping := make(map[string]int32) - var tagCurr int32 - getTagID := func(key string) int32 { - if id, ok := tagMapping[key]; ok { - return id - } - id := tagCurr - tagCurr++ - tagMapping[key] = id - return id - } - - attrNameMapping := make(map[string]int32) - var attrNameCurr int32 - getAttrNameID := func(key string) int32 { - if id, ok := attrNameMapping[key]; ok { - return id - } - id := attrNameCurr - attrNameCurr++ - attrNameMapping[key] = id - return id - } - - attrStrValMapping := make(map[string]int32) - var attrStrValCurr int32 - getAttrStrValID := func(key string) int32 { - if id, ok := attrStrValMapping[key]; ok { - return id - } - id := attrStrValCurr - attrStrValCurr++ - attrStrValMapping[key] = id - return id - } - - // Build the optimized targets slice - optimizedTargets := make([]*tangopb.OptimizedTarget, 0, len(result.Targets)) - - for _, t := range result.Targets { - // Intern name - nameID := targetNamesMapping[t.Name] - - // Intern dependencies that are also in targetNames - depIDs := make([]int32, 0, len(t.Deps)) - for _, depName := range t.Deps { - if _, ok := targetNamesMapping[depName]; !ok { - continue - } - depIDs = append(depIDs, targetNamesMapping[depName]) - } - - ot := &tangopb.OptimizedTarget{ - Id: nameID, - Hash: string(t.Hash), - DirectDependencies: depIDs, - } - - // RuleType - if t.RuleType != "" { - id := getRuleTypeID(t.RuleType) - ot.RuleType = id - } - - // Tags - if len(t.Tags) > 0 { - tagIDs := make([]int32, 0, len(t.Tags)) - for _, tag := range t.Tags { - tagIDs = append(tagIDs, getTagID(tag)) - } - ot.Tags = tagIDs - } - ot.Root = t.Root - ot.External = t.External - if len(t.Attributes) > 0 { - attrs := make(map[int32]int32, len(t.Attributes)) - for _, attr := range t.Attributes { - // Only include STRING attributes with non-nil name and value to avoid nil dereferences. - if attr.GetType() == buildpb.Attribute_STRING && attr.Name != nil && attr.StringValue != nil { - nameID := getAttrNameID(*attr.Name) - valID := getAttrStrValID(*attr.StringValue) - attrs[nameID] = valID - } - } - ot.Attributes = attrs - } - - optimizedTargets = append(optimizedTargets, ot) - } - - // Invert mappings: string -> id => id -> string - targetIDToName := make(map[int32]string, len(targetNamesMapping)) - for s, id := range targetNamesMapping { - targetIDToName[id] = s - } - - ruleTypeIDToName := make(map[int32]string, len(ruleTypeMapping)) - for s, id := range ruleTypeMapping { - ruleTypeIDToName[id] = s - } - - tagIDToName := make(map[int32]string, len(tagMapping)) - for s, id := range tagMapping { - tagIDToName[id] = s - } - - attrNameIDToName := make(map[int32]string, len(attrNameMapping)) - for s, id := range attrNameMapping { - attrNameIDToName[id] = s - } - - attrStrValIDToVal := make(map[int32]string, len(attrStrValMapping)) - for s, id := range attrStrValMapping { - attrStrValIDToVal[id] = s - } - - // Assemble final OptimizedTargets - return []*tangopb.GetTargetGraphResponse{ - { - Item: &tangopb.GetTargetGraphResponse_Targets{ - Targets: &tangopb.OptimizedTargets{ - Targets: optimizedTargets, - }, - }, - }, - { - Item: &tangopb.GetTargetGraphResponse_Metadata{ - Metadata: &tangopb.Metadata{ - TargetIdMapping: targetIDToName, - RuleTypeMapping: ruleTypeIDToName, - TagMapping: tagIDToName, - AttributeNameMapping: attrNameIDToName, - AttributeStringValueMapping: attrStrValIDToVal, - }, - }, - }, - }, nil -} diff --git a/orchestrator/BUILD.bazel b/orchestrator/BUILD.bazel index dbe696ab..039be691 100644 --- a/orchestrator/BUILD.bazel +++ b/orchestrator/BUILD.bazel @@ -16,7 +16,6 @@ go_library( "//core/git", "//core/repomanager", "//core/storage", - "//core/targethasher", "//core/workspace", "//tangopb", "@org_uber_go_zap//:zap", diff --git a/orchestrator/native_orchestrator.go b/orchestrator/native_orchestrator.go index 98755f53..394f92ab 100644 --- a/orchestrator/native_orchestrator.go +++ b/orchestrator/native_orchestrator.go @@ -12,7 +12,6 @@ import ( "github.com/uber/tango/core/git" "github.com/uber/tango/core/repomanager" "github.com/uber/tango/core/storage" - "github.com/uber/tango/core/targethasher" "github.com/uber/tango/core/workspace" "go.uber.org/zap" "time" @@ -136,7 +135,7 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, param GetTarget b.logger.Error("getGraph: Error computing target graph", zap.Any("request build description", param.Req.BuildDescription), zap.Error(err)) return nil, err } - graphs, err := targethasher.ResultToGetTargetGraphResponse(result) + graphs, err := common.ResultToGetTargetGraphResponse(result) if err != nil { b.logger.Error("getGraph: Error converting target graph to GetTargetGraphResponse", zap.Any("request build description", param.Req.BuildDescription), zap.Error(err)) return nil, err