From baaabf99a4453dde8f1ba04a43e1538345a4f490 Mon Sep 17 00:00:00 2001 From: sergeyb Date: Wed, 26 Aug 2026 18:12:40 +0000 Subject: [PATCH] fix(targethasher): frame rule hashes for BUG-006 Summary: Intent: - Fix BUG-006 by eliminating structural collisions in rule hashing. - Preserve deterministic, order-independent hashing for unordered rule data. Changes: - Add canonical encoding with field, type, presence, and length framing without an encoding version marker. - Frame collection element types, counts, values, and nested messages before hashing. - Add regression coverage for prior collision shapes and collection-order determinism. --- Generated by the pr-create skill in devexp-agent-marketplace --- core/targethasher/graph_test.go | 6 +- core/targethasher/sourcehasher.go | 429 +++++++++++++++---------- core/targethasher/sourcehasher_test.go | 248 ++++++++++++++ 3 files changed, 510 insertions(+), 173 deletions(-) diff --git a/core/targethasher/graph_test.go b/core/targethasher/graph_test.go index 5adc9c63..abb58f44 100644 --- a/core/targethasher/graph_test.go +++ b/core/targethasher/graph_test.go @@ -316,7 +316,7 @@ func Test_RemoveAttrs(t *testing.T) { assert.NoError(t, err) assert.False(t, target.External) assert.Equal(t, regularTarget.GetRule().GetAttribute(), target.Attributes) - assert.Equal(t, []byte{0x69, 0x8b, 0x5, 0x75, 0x55, 0x80, 0x66, 0x5d, 0x7e, 0xbc, 0x75, 0x4, 0x8e, 0x62, 0x48, 0xb0, 0x82, 0x9c, 0x87, 0x82}, target.HashWithoutDeps) + assert.Equal(t, []byte{0x24, 0xa2, 0x3a, 0x8, 0x9e, 0x19, 0x34, 0x6e, 0xf3, 0x62, 0x9a, 0x8f, 0xca, 0x5e, 0x54, 0xc, 0xf2, 0xdd, 0x9d, 0x8d}, target.HashWithoutDeps) externalTarget := &buildpb.Target{ Type: buildpb.Target_RULE.Enum(), @@ -341,7 +341,7 @@ func Test_RemoveAttrs(t *testing.T) { external, err := toTarget(externalTarget) assert.NoError(t, err) assert.True(t, external.External) - assert.Equal(t, []byte{0xbb, 0xee, 0x72, 0xbb, 0xda, 0x44, 0xa0, 0xb5, 0x27, 0x9f, 0x9c, 0xde, 0xda, 0xb3, 0xc9, 0x46, 0xbe, 0x7e, 0x14, 0x92}, external.HashWithoutDeps) + assert.Equal(t, []byte{0xfd, 0xc5, 0xc7, 0x60, 0x80, 0x27, 0xc9, 0xee, 0x59, 0x2d, 0x8e, 0xb, 0x67, 0x3f, 0xae, 0xab, 0xc4, 0x8b, 0x4f, 0xe3}, external.HashWithoutDeps) // add sha256 attribute, hash should change externalTarget.Rule.Attribute = append(externalTarget.Rule.Attribute, &buildpb.Attribute{ @@ -351,7 +351,7 @@ func Test_RemoveAttrs(t *testing.T) { external, err = toTarget(externalTarget) assert.NoError(t, err) assert.True(t, external.External) - assert.Equal(t, []byte{0x7c, 0xde, 0x91, 0xc2, 0x94, 0x1a, 0x22, 0xf3, 0xb2, 0x18, 0x7c, 0x21, 0xbf, 0x32, 0x17, 0xc0, 0xa3, 0xf0, 0xc, 0x77}, external.HashWithoutDeps) + assert.Equal(t, []byte{0x6c, 0xeb, 0xec, 0x11, 0x74, 0x82, 0xae, 0x48, 0x6c, 0xff, 0x4f, 0x3c, 0xb1, 0xd2, 0xcf, 0x79, 0xf0, 0xe0, 0xee, 0xfc}, external.HashWithoutDeps) } func validateResultIsStable(t *testing.T, baseResult, result Result) { diff --git a/core/targethasher/sourcehasher.go b/core/targethasher/sourcehasher.go index c575dc0f..470267d1 100644 --- a/core/targethasher/sourcehasher.go +++ b/core/targethasher/sourcehasher.go @@ -15,8 +15,10 @@ package targethasher import ( + "bytes" "context" "crypto/sha1" + "encoding/binary" "errors" "fmt" "hash" @@ -26,7 +28,6 @@ import ( "path/filepath" "slices" "sort" - "strconv" "strings" buildpb "github.com/bazelbuild/buildtools/build_proto" @@ -192,196 +193,284 @@ func externalTargetForRule(t string) string { return externalWorkspaceRulePrefix + strings.TrimLeft(strings.Split(t, "//")[0], "@") } -// HashRuleCommon hashes the common elements of a buildpb.Rule. +type canonicalValueType byte + +const ( + canonicalMessage canonicalValueType = iota + 1 + canonicalString + canonicalBool + canonicalInt32 + canonicalEnum + canonicalList +) + +type canonicalEncoder struct { + buffer bytes.Buffer +} + +// HashRuleCommon hashes the common elements of a buildpb.Rule using explicit +// field, type, presence, and length framing. func HashRuleCommon(r *buildpb.Rule, h hash.Hash) { - // Name *string - io.WriteString(h, r.GetName()) - // RuleClass *string - io.WriteString(h, r.GetRuleClass()) - // Location *string - // don't hash location, as it machine local paths - // Attribute []*Attribute - // Before hashing, sort to guarantee consistency - attributes := slices.Clone(r.GetAttribute()) - sort.Slice(attributes, func(i, j int) bool { - return attributes[i].GetName() < attributes[j].GetName() + var encoder canonicalEncoder + var rule []byte + if r != nil { + rule = encodeRule(r) + } + encoder.writeField("rule", canonicalMessage, r != nil, rule) + _, _ = h.Write(encoder.buffer.Bytes()) +} + +func encodeRule(r *buildpb.Rule) []byte { + return encodeMessage("Rule", func(encoder *canonicalEncoder) { + encoder.writeOptionalString("name", r.Name) + encoder.writeOptionalString("rule_class", r.RuleClass) + + // Location is machine-local and intentionally excluded. + ruleAttributes := r.GetAttribute() + attributes := make([]*buildpb.Attribute, 0, len(ruleAttributes)) + for _, attribute := range ruleAttributes { + if attribute == nil { + continue + } + // generator_location identifies a macro call site and does not affect + // the rule's behavior. + if attribute.GetName() == "generator_location" { + continue + } + attributes = append(attributes, attribute) + } + // Bazel rule classes enforce unique attribute names, so sorting by name + // provides a deterministic order for rule instances. + sort.Slice(attributes, func(i, j int) bool { + return attributes[i].GetName() < attributes[j].GetName() + }) + encodedAttributes := make([][]byte, 0, len(attributes)) + for _, attribute := range attributes { + encodedAttributes = append(encodedAttributes, encodeAttribute(attribute)) + } + encoder.writeList("attribute", canonicalMessage, encodedAttributes) + encoder.writeSortedStrings("rule_input", r.GetRuleInput()) + encoder.writeSortedStrings("rule_output", r.GetRuleOutput()) + encoder.writeSortedStrings("default_setting", r.GetDefaultSetting()) + + // Aspects do not appear in the query representation and remain excluded. + encoder.writeOptionalString("skylark_environment_hash_code", r.SkylarkEnvironmentHashCode) }) - for _, attr := range attributes { - hashAttributes(h, attr) +} + +func encodeAttribute(attribute *buildpb.Attribute) []byte { + if attribute == nil { + return nil } - // RuleInput []string - // Before hashing, sort to guarantee consistency - ruleInputs := slices.Clone(r.GetRuleInput()) - sort.Strings(ruleInputs) - for _, ri := range ruleInputs { - io.WriteString(h, ri) + + return encodeMessage("Attribute", func(encoder *canonicalEncoder) { + encoder.writeOptionalString("name", attribute.Name) + + // Parseable locations are machine-local and intentionally excluded. + encoder.writeOptionalBool("explicitly_specified", attribute.ExplicitlySpecified) + encoder.writeOptionalBool("nodep", attribute.Nodep) + writeOptionalEnum(encoder, "type", attribute.Type) + encoder.writeOptionalInt32("int_value", attribute.IntValue) + encoder.writeOptionalString("string_value", attribute.StringValue) + encoder.writeOptionalBool("boolean_value", attribute.BooleanValue) + writeOptionalEnum(encoder, "tristate_value", attribute.TristateValue) + encoder.writeSortedStrings("string_list_value", attribute.GetStringListValue()) + writeSortedMessages(encoder, "string_dict_value", attribute.GetStringDictValue(), encodeStringDictEntry) + writeSortedMessages(encoder, "fileset_list_value", attribute.GetFilesetListValue(), encodeFilesetEntry) + writeSortedMessages(encoder, "label_list_dict_value", attribute.GetLabelListDictValue(), encodeLabelListDictEntry) + writeSortedMessages(encoder, "string_list_dict_value", attribute.GetStringListDictValue(), encodeStringListDictEntry) + encoder.writeSortedInt32s("int_list_value", attribute.GetIntListValue()) + writeSortedMessages(encoder, "label_dict_unary_value", attribute.GetLabelDictUnaryValue(), encodeLabelDictUnaryEntry) + writeSortedMessages(encoder, "label_keyed_string_dict_value", attribute.GetLabelKeyedStringDictValue(), encodeLabelKeyedStringDictEntry) + + // License, deprecated string-dict-unary values, and selector lists retain + // their existing exclusion from rule hashes. + }) +} + +func encodeStringDictEntry(entry *buildpb.StringDictEntry) []byte { + if entry == nil { + return nil } - // RuleOutput []string - // Before hashing, sort to guarantee consistency - ruleOutputs := slices.Clone(r.GetRuleOutput()) - sort.Strings(ruleOutputs) - for _, ro := range ruleOutputs { - io.WriteString(h, ro) + return encodeMessage("StringDictEntry", func(encoder *canonicalEncoder) { + encoder.writeOptionalString("key", entry.Key) + encoder.writeOptionalString("value", entry.Value) + }) +} + +func encodeFilesetEntry(entry *buildpb.FilesetEntry) []byte { + if entry == nil { + return nil + } + return encodeMessage("FilesetEntry", func(encoder *canonicalEncoder) { + encoder.writeOptionalString("source", entry.Source) + encoder.writeOptionalString("destination_directory", entry.DestinationDirectory) + encoder.writeOptionalBool("files_present", entry.FilesPresent) + encoder.writeSortedStrings("file", entry.GetFile()) + encoder.writeSortedStrings("exclude", entry.GetExclude()) + writeOptionalEnum(encoder, "symlink_behavior", entry.SymlinkBehavior) + encoder.writeOptionalString("strip_prefix", entry.StripPrefix) + }) +} + +func encodeLabelListDictEntry(entry *buildpb.LabelListDictEntry) []byte { + if entry == nil { + return nil } - // DefaultSetting []string - // Before hashing, sort to guarantee consistency - defaultSettings := slices.Clone(r.GetDefaultSetting()) - sort.Strings(defaultSettings) - for _, d := range defaultSettings { - io.WriteString(h, d) + return encodeMessage("LabelListDictEntry", func(encoder *canonicalEncoder) { + encoder.writeOptionalString("key", entry.Key) + encoder.writeSortedStrings("value", entry.GetValue()) + }) +} + +func encodeStringListDictEntry(entry *buildpb.StringListDictEntry) []byte { + if entry == nil { + return nil } - // SkylarkAttributeAspects []*AttributeAspect - // don't need to hash this, aspects don't appear in our query. - // SkylarkEnvironmentHashCode *string - io.WriteString(h, r.GetSkylarkEnvironmentHashCode()) + return encodeMessage("StringListDictEntry", func(encoder *canonicalEncoder) { + encoder.writeOptionalString("key", entry.Key) + encoder.writeSortedStrings("value", entry.GetValue()) + }) } -func hashAttributes(h hash.Hash, a *buildpb.Attribute) { - // generator_location is present if the rule is generated from a macro, but - // should not be hashed as the generating macro's location in a build file - // is irrelevant for our purposes - if a.GetName() == "generator_location" { - return +func encodeLabelDictUnaryEntry(entry *buildpb.LabelDictUnaryEntry) []byte { + if entry == nil { + return nil } - // Name *string - io.WriteString(h, a.GetName()) - // DEPRECATEDParseableLocation *Location - // note: Location contains machine specific elements, don't hash - // ExplicitlySpecified *bool - if a.GetExplicitlySpecified() { - h.Write([]byte{1}) + return encodeMessage("LabelDictUnaryEntry", func(encoder *canonicalEncoder) { + encoder.writeOptionalString("key", entry.Key) + encoder.writeOptionalString("value", entry.Value) + }) +} + +func encodeLabelKeyedStringDictEntry(entry *buildpb.LabelKeyedStringDictEntry) []byte { + if entry == nil { + return nil } - // Nodep *bool - if a.GetNodep() { - h.Write([]byte{1}) + return encodeMessage("LabelKeyedStringDictEntry", func(encoder *canonicalEncoder) { + encoder.writeOptionalString("key", entry.Key) + encoder.writeOptionalString("value", entry.Value) + }) +} + +func encodeMessage(typeName string, writeFields func(*canonicalEncoder)) []byte { + var encoder canonicalEncoder + encoder.writeField("message_type", canonicalString, true, []byte(typeName)) + writeFields(&encoder) + return encoder.buffer.Bytes() +} + +func (e *canonicalEncoder) writeField(name string, valueType canonicalValueType, present bool, payload []byte) { + e.writeBytes([]byte(name)) + _ = e.buffer.WriteByte(byte(valueType)) + if !present { + _ = e.buffer.WriteByte(0) + return } - // Type *Attribute_Discriminator - io.WriteString(h, a.GetType().String()) - // IntValue *int32 - if a.IntValue != nil { - io.WriteString(h, strconv.Itoa(int(a.GetIntValue()))) + _ = e.buffer.WriteByte(1) + e.writeBytes(payload) +} + +func (e *canonicalEncoder) writeOptionalString(name string, value *string) { + if value == nil { + e.writeField(name, canonicalString, false, nil) + return } - // StringValue *string - io.WriteString(h, a.GetStringValue()) - // BooleanValue *bool - if a.GetBooleanValue() { - h.Write([]byte{1}) + e.writeField(name, canonicalString, true, []byte(*value)) +} + +func (e *canonicalEncoder) writeOptionalBool(name string, value *bool) { + if value == nil { + e.writeField(name, canonicalBool, false, nil) + return } - // TristateValue *Attribute_Tristate - if a.TristateValue != nil { - io.WriteString(h, a.GetTristateValue().String()) + payload := byte(0) + if *value { + payload = 1 } - // StringListValue []string - // Before hashing, sort to guarantee consistency - stringListValue := slices.Clone(a.GetStringListValue()) - sort.Strings(stringListValue) - for _, s := range stringListValue { - io.WriteString(h, s) + e.writeField(name, canonicalBool, true, []byte{payload}) +} + +func (e *canonicalEncoder) writeOptionalInt32(name string, value *int32) { + if value == nil { + e.writeField(name, canonicalInt32, false, nil) + return } - // License *License - // StringDictValue []*StringDictEntry - // Before hashing, sort to guarantee consistency - stringDictValue := slices.Clone(a.GetStringDictValue()) - sort.Slice(stringDictValue, func(i, j int) bool { - return stringDictValue[i].GetKey() < stringDictValue[j].GetKey() - }) - for _, d := range stringDictValue { - io.WriteString(h, d.GetKey()) - io.WriteString(h, d.GetValue()) + e.writeField(name, canonicalInt32, true, encodeInt32(*value)) +} + +func writeOptionalEnum[T ~int32](e *canonicalEncoder, name string, value *T) { + if value == nil { + e.writeField(name, canonicalEnum, false, nil) + return } - // FilesetListValue []*FilesetEntry - // Before hashing, sort to guarantee consistency - filesetListValue := slices.Clone(a.GetFilesetListValue()) - sort.Slice(filesetListValue, func(i, j int) bool { - return filesetListValue[i].GetSource() < filesetListValue[j].GetSource() - }) - for _, f := range filesetListValue { - // Source *string - io.WriteString(h, f.GetSource()) - // DestinationDirectory *string - io.WriteString(h, f.GetDestinationDirectory()) - // FilesPresent *bool - if f.GetFilesPresent() { - h.Write([]byte{1}) - } - // File []string - // Before hashing, sort to guarantee consistency - files := slices.Clone(f.GetFile()) - sort.Strings(files) - for _, file := range files { - io.WriteString(h, file) - } - // Exclude []string - // Before hashing, sort to guarantee consistency - excludedFiles := slices.Clone(f.GetExclude()) - sort.Strings(excludedFiles) - for _, file := range excludedFiles { - io.WriteString(h, file) - } - // SymlinkBehavior *FilesetEntry_SymlinkBehavior - io.WriteString(h, f.GetSymlinkBehavior().String()) - // StripPrefix *string - io.WriteString(h, f.GetStripPrefix()) + e.writeField(name, canonicalEnum, true, encodeInt32(int32(*value))) +} + +func (e *canonicalEncoder) writeSortedStrings(name string, values []string) { + sorted := slices.Clone(values) + sort.Strings(sorted) + elements := make([][]byte, 0, len(sorted)) + for _, value := range sorted { + elements = append(elements, []byte(value)) } - // LabelListDictValue []*LabelListDictEntry - // Before hashing, sort to guarantee consistency - labelListDictValue := slices.Clone(a.GetLabelListDictValue()) - sort.Slice(labelListDictValue, func(i, j int) bool { - return labelListDictValue[i].GetKey() < labelListDictValue[j].GetKey() + e.writeList(name, canonicalString, elements) +} + +func (e *canonicalEncoder) writeSortedInt32s(name string, values []int32) { + sorted := slices.Clone(values) + sort.Slice(sorted, func(i, j int) bool { + return sorted[i] < sorted[j] }) - for _, ll := range labelListDictValue { - io.WriteString(h, ll.GetKey()) - // Before hashing, sort to guarantee consistency - llv := slices.Clone(ll.GetValue()) - sort.Strings(llv) - for _, v := range llv { - io.WriteString(h, v) - } + elements := make([][]byte, 0, len(sorted)) + for _, value := range sorted { + elements = append(elements, encodeInt32(value)) } - // StringListDictValue []*StringListDictEntry - // Before hashing, sort to guarantee consistency - stringListDictValue := slices.Clone(a.GetStringListDictValue()) - sort.Slice(stringListDictValue, func(i, j int) bool { - return stringListDictValue[i].GetKey() < stringListDictValue[j].GetKey() - }) - for _, sl := range stringListDictValue { - io.WriteString(h, sl.GetKey()) - // Before hashing, sort to guarantee consistency - slv := slices.Clone(sl.GetValue()) - sort.Strings(slv) - for _, v := range slv { - io.WriteString(h, v) + e.writeList(name, canonicalInt32, elements) +} + +func writeSortedMessages[T any](e *canonicalEncoder, name string, values []*T, encode func(*T) []byte) { + elements := make([][]byte, 0, len(values)) + for _, value := range values { + if value == nil { + continue } + elements = append(elements, encode(value)) } - // IntListValue []int32 - // Before hashing, sort to guarantee consistency - intListValue := slices.Clone(a.GetIntListValue()) - sort.Slice(intListValue, func(i, j int) bool { - return intListValue[i] < intListValue[j] - }) - for _, i := range intListValue { - io.WriteString(h, strconv.Itoa(int(i))) - } - // LabelDictUnaryValue []*LabelDictUnaryEntry - // Before hashing, sort to guarantee consistency - labelDictUnaryValue := slices.Clone(a.GetLabelDictUnaryValue()) - sort.Slice(labelDictUnaryValue, func(i, j int) bool { - return labelDictUnaryValue[i].GetKey() < labelDictUnaryValue[j].GetKey() - }) - for _, d := range labelDictUnaryValue { - io.WriteString(h, d.GetKey()) - io.WriteString(h, d.GetValue()) + sortElements(elements) + e.writeList(name, canonicalMessage, elements) +} + +func (e *canonicalEncoder) writeList(name string, elementType canonicalValueType, elements [][]byte) { + var payload canonicalEncoder + _ = payload.buffer.WriteByte(byte(elementType)) + payload.writeUvarint(uint64(len(elements))) + for _, element := range elements { + _ = payload.buffer.WriteByte(1) + payload.writeBytes(element) } - // LabelKeyedStringDictValue []*LabelKeyedStringDictEntry - // Before hashing, sort to guarantee consistency - labelKeyedStringDictValue := slices.Clone(a.GetLabelKeyedStringDictValue()) - sort.Slice(labelKeyedStringDictValue, func(i, j int) bool { - return labelKeyedStringDictValue[i].GetKey() < labelKeyedStringDictValue[j].GetKey() + e.writeField(name, canonicalList, true, payload.buffer.Bytes()) +} + +func (e *canonicalEncoder) writeBytes(value []byte) { + e.writeUvarint(uint64(len(value))) + _, _ = e.buffer.Write(value) +} + +func (e *canonicalEncoder) writeUvarint(value uint64) { + var encoded [binary.MaxVarintLen64]byte + length := binary.PutUvarint(encoded[:], value) + _, _ = e.buffer.Write(encoded[:length]) +} + +func encodeInt32(value int32) []byte { + var encoded [4]byte + binary.BigEndian.PutUint32(encoded[:], uint32(value)) + return encoded[:] +} + +func sortElements(elements [][]byte) { + sort.Slice(elements, func(i, j int) bool { + return bytes.Compare(elements[i], elements[j]) < 0 }) - for _, d := range labelKeyedStringDictValue { - io.WriteString(h, d.GetKey()) - io.WriteString(h, d.GetValue()) - } - // SelectorList *Attribute_SelectorList - // pass on this for now } diff --git a/core/targethasher/sourcehasher_test.go b/core/targethasher/sourcehasher_test.go index 21e9db70..d3115467 100644 --- a/core/targethasher/sourcehasher_test.go +++ b/core/targethasher/sourcehasher_test.go @@ -187,6 +187,254 @@ func TestDiskHashHelper_MissingFileProducesDeterministicHash(t *testing.T) { func strPtr(s string) *string { return &s } +func valuePtr[T any](value T) *T { return &value } + +func hashRuleForTest(rule *buildpb.Rule) []byte { + h := newHash() + HashRuleCommon(rule, h) + return h.Sum(nil) +} + +func TestHashRuleCommon_DistinguishesStructuralCollisions(t *testing.T) { + tests := []struct { + name string + left *buildpb.Rule + right *buildpb.Rule + }{ + { + name: "rule field boundaries", + left: &buildpb.Rule{ + Name: strPtr("a"), + RuleClass: strPtr("bc"), + }, + right: &buildpb.Rule{ + Name: strPtr("ab"), + RuleClass: strPtr("c"), + }, + }, + { + name: "collection element boundaries", + left: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("values"), + Type: buildpb.Attribute_STRING_LIST.Enum(), + StringListValue: []string{"a", "bc"}, + }}, + }, + right: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("values"), + Type: buildpb.Attribute_STRING_LIST.Enum(), + StringListValue: []string{"ab", "c"}, + }}, + }, + }, + { + name: "collection field boundaries", + left: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + RuleInput: []string{"a"}, + RuleOutput: []string{"bc"}, + }, + right: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + RuleInput: []string{"ab"}, + RuleOutput: []string{"c"}, + }, + }, + { + name: "scalar field and type boundaries", + left: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("value"), + IntValue: valuePtr[int32](1), + }}, + }, + right: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("value"), + StringValue: strPtr("1"), + }}, + }, + }, + { + name: "optional scalar presence", + left: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("value"), + }}, + }, + right: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("value"), + ExplicitlySpecified: valuePtr(false), + }}, + }, + }, + { + name: "nested message field boundaries", + left: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("values"), + Type: buildpb.Attribute_STRING_DICT.Enum(), + StringDictValue: []*buildpb.StringDictEntry{{ + Key: strPtr("a"), + Value: strPtr("bc"), + }}, + }}, + }, + right: &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("values"), + Type: buildpb.Attribute_STRING_DICT.Enum(), + StringDictValue: []*buildpb.StringDictEntry{{ + Key: strPtr("ab"), + Value: strPtr("c"), + }}, + }}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.NotEqual(t, hashRuleForTest(tt.left), hashRuleForTest(tt.right)) + }) + } +} + +func TestHashRuleCommon_IsDeterministicAcrossCollectionOrder(t *testing.T) { + left := &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + RuleInput: []string{"//pkg:b", "//pkg:a"}, + RuleOutput: []string{"out_b", "out_a"}, + DefaultSetting: []string{"z", "a"}, + Attribute: []*buildpb.Attribute{ + { + Name: strPtr("tags"), + Type: buildpb.Attribute_STRING_LIST.Enum(), + StringListValue: []string{"beta", "alpha"}, + }, + { + Name: strPtr("mapping"), + Type: buildpb.Attribute_STRING_DICT.Enum(), + StringDictValue: []*buildpb.StringDictEntry{ + {Key: strPtr("z"), Value: strPtr("last")}, + {Key: strPtr("a"), Value: strPtr("first")}, + }, + }, + { + Name: strPtr("filesets"), + Type: buildpb.Attribute_FILESET_ENTRY_LIST.Enum(), + FilesetListValue: []*buildpb.FilesetEntry{ + { + Source: strPtr("//pkg:z"), + DestinationDirectory: strPtr("dest-z"), + File: []string{"b", "a"}, + Exclude: []string{"d", "c"}, + }, + { + Source: strPtr("//pkg:a"), + DestinationDirectory: strPtr("dest-a"), + }, + }, + }, + }, + } + right := &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + RuleInput: []string{"//pkg:a", "//pkg:b"}, + RuleOutput: []string{"out_a", "out_b"}, + DefaultSetting: []string{"a", "z"}, + Attribute: []*buildpb.Attribute{ + { + Name: strPtr("filesets"), + Type: buildpb.Attribute_FILESET_ENTRY_LIST.Enum(), + FilesetListValue: []*buildpb.FilesetEntry{ + { + Source: strPtr("//pkg:a"), + DestinationDirectory: strPtr("dest-a"), + }, + { + Source: strPtr("//pkg:z"), + DestinationDirectory: strPtr("dest-z"), + File: []string{"a", "b"}, + Exclude: []string{"c", "d"}, + }, + }, + }, + { + Name: strPtr("mapping"), + Type: buildpb.Attribute_STRING_DICT.Enum(), + StringDictValue: []*buildpb.StringDictEntry{ + {Key: strPtr("a"), Value: strPtr("first")}, + {Key: strPtr("z"), Value: strPtr("last")}, + }, + }, + { + Name: strPtr("tags"), + Type: buildpb.Attribute_STRING_LIST.Enum(), + StringListValue: []string{"alpha", "beta"}, + }, + }, + } + + leftHash := hashRuleForTest(left) + assert.Equal(t, leftHash, hashRuleForTest(left)) + assert.Equal(t, leftHash, hashRuleForTest(right)) +} + +func TestHashRuleCommon_IgnoresNilMessages(t *testing.T) { + withNilMessages := &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{ + nil, + { + Name: strPtr("mapping"), + Type: buildpb.Attribute_STRING_DICT.Enum(), + StringDictValue: []*buildpb.StringDictEntry{ + nil, + {Key: strPtr("key"), Value: strPtr("value")}, + }, + }, + }, + } + withoutNilMessages := &buildpb.Rule{ + Name: strPtr("//pkg:target"), + RuleClass: strPtr("test_rule"), + Attribute: []*buildpb.Attribute{{ + Name: strPtr("mapping"), + Type: buildpb.Attribute_STRING_DICT.Enum(), + StringDictValue: []*buildpb.StringDictEntry{ + {Key: strPtr("key"), Value: strPtr("value")}, + }, + }}, + } + + assert.Equal(t, hashRuleForTest(withoutNilMessages), hashRuleForTest(withNilMessages)) +} + func TestDiskHashHelper_RespectsContextCancellation(t *testing.T) { tmp := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(tmp, "file.txt"), []byte("x"), 0o644))