From bd299cc49b59709da113266d606fc39f6eca5eb3 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Fri, 18 Oct 2024 17:41:22 -0700 Subject: [PATCH 1/4] fix: overwrite attribute values when not mergeable or resolveable --- merger/merger.go | 21 ++++++++++++++++----- rule/merge.go | 24 ++++++++++++++++++++---- rule/merge_test.go | 44 ++++++++++++++++++++++++++++++++++++++------ 3 files changed, 74 insertions(+), 15 deletions(-) diff --git a/merger/merger.go b/merger/merger.go index 29f39b331..35becc7f7 100644 --- a/merger/merger.go +++ b/merger/merger.go @@ -107,21 +107,32 @@ const UnstableInsertIndexKey = "_gazelle_insert_index" // If a rule is marked with a "# keep" comment, the whole rule will not // be modified. func MergeFile(oldFile *rule.File, emptyRules, genRules []*rule.Rule, phase Phase, kinds map[string]rule.KindInfo, aliasedKinds map[string]string) { - getMergeAttrs := func(r *rule.Rule) map[string]bool { + isMergeable := func(r *rule.Rule, attr string) bool { + // Attributes merged in this phase. if phase == PreResolve { - return kinds[r.Kind()].MergeableAttrs + return kinds[r.Kind()].MergeableAttrs[attr] } else { - return kinds[r.Kind()].ResolveAttrs + return kinds[r.Kind()].ResolveAttrs[attr] } } + isManaged := func(r *rule.Rule, attr string) bool { + // Name and visibility are uniquely managed by gazelle. + if attr == "name" || attr == "visibility" { + return true + } + k := kinds[r.Kind()] + // All known attributes of this rule kind. + return k.NonEmptyAttrs[attr] || k.SubstituteAttrs[attr] || k.ResolveAttrs[attr] || k.MergeableAttrs[attr] + } + // Merge empty rules into the file and delete any rules which become empty. for _, emptyRule := range emptyRules { if oldRule, _ := match(oldFile.Rules, emptyRule, kinds[emptyRule.Kind()], false, aliasedKinds); oldRule != nil { if oldRule.ShouldKeep() { continue } - rule.MergeRules(emptyRule, oldRule, getMergeAttrs(emptyRule), oldFile.Path) + rule.MergeRules(emptyRule, oldRule, isMergeable, isManaged, oldFile.Path) if oldRule.IsEmpty(kinds[oldRule.Kind()]) { oldRule.Delete() } @@ -169,7 +180,7 @@ func MergeFile(oldFile *rule.File, emptyRules, genRules []*rule.Rule, phase Phas genRule.Insert(oldFile) } } else { - rule.MergeRules(genRule, matchRules[i], getMergeAttrs(genRule), oldFile.Path) + rule.MergeRules(genRule, matchRules[i], isMergeable, isManaged, oldFile.Path) } } } diff --git a/rule/merge.go b/rule/merge.go index ae2acdda4..6f5aebb33 100644 --- a/rule/merge.go +++ b/rule/merge.go @@ -24,6 +24,8 @@ import ( bzl "github.com/bazelbuild/buildtools/build" ) +type IsMergeable = func(r *Rule, attr string) bool + // MergeRules copies information from src into dst, usually discarding // information in dst when they have the same attributes. // @@ -41,14 +43,14 @@ import ( // marked with a "# keep" comment, values in the attribute not marked with // a "# keep" comment will be dropped. If the attribute is empty afterward, // it will be deleted. -func MergeRules(src, dst *Rule, mergeable map[string]bool, filename string) { +func MergeRules(src, dst *Rule, isMergeable, isManaged IsMergeable, filename string) { if dst.ShouldKeep() { return } // Process attributes that are in dst but not in src. for key, dstAttr := range dst.attrs { - if _, ok := src.attrs[key]; ok || !mergeable[key] || ShouldKeep(dstAttr.expr) { + if _, ok := src.attrs[key]; ok || !isMergeable(src, key) || ShouldKeep(dstAttr.expr) { continue } if mergedValue, err := mergeAttrValues(nil, &dstAttr); err != nil { @@ -63,9 +65,20 @@ func MergeRules(src, dst *Rule, mergeable map[string]bool, filename string) { // Merge attributes from src into dst. for key, srcAttr := range src.attrs { - if dstAttr, ok := dst.attrs[key]; !ok { + // Always set properties that are not yet set. + dstAttr, dstAttrExists := dst.attrs[key] + if !dstAttrExists { dst.SetAttr(key, srcAttr.expr.RHS) - } else if mergeable[key] && !ShouldKeep(dstAttr.expr) { + continue + } + + // Do not override attributes marked with "# keep". + if ShouldKeep(dstAttr.expr) { + continue + } + + if isMergeable(src, key) { + // Merge mergeable attributes. if mergedValue, err := mergeAttrValues(&srcAttr, &dstAttr); err != nil { start, end := dstAttr.expr.RHS.Span() log.Printf("%s:%d.%d-%d.%d: could not merge expression", filename, start.Line, start.LineRune, end.Line, end.LineRune) @@ -74,6 +87,9 @@ func MergeRules(src, dst *Rule, mergeable map[string]bool, filename string) { } else { dst.SetAttr(key, mergedValue) } + } else if !isManaged(src, key) { + // Overwrite unknown attributes. + dst.SetAttr(key, srcAttr.expr.RHS) } } diff --git a/rule/merge_test.go b/rule/merge_test.go index 6718d90b6..35d863295 100644 --- a/rule/merge_test.go +++ b/rule/merge_test.go @@ -16,12 +16,20 @@ limitations under the License. package rule_test import ( + "reflect" "testing" "github.com/bazelbuild/bazel-gazelle/rule" bzl "github.com/bazelbuild/buildtools/build" ) +func depsMergeable(r *rule.Rule, attr string) bool { + return attr == "deps" +} +func notMergeable(r *rule.Rule, attr string) bool { + return false +} + func TestMergeRules(t *testing.T) { t.Run("private attributes are merged", func(t *testing.T) { src := rule.NewRule("go_library", "go_default_library") @@ -29,7 +37,7 @@ func TestMergeRules(t *testing.T) { privateAttrVal := "private_value" src.SetPrivateAttr(privateAttrKey, privateAttrVal) dst := rule.NewRule("go_library", "go_default_library") - rule.MergeRules(src, dst, map[string]bool{}, "") + rule.MergeRules(src, dst, notMergeable, notMergeable, "") if dst.PrivateAttr(privateAttrKey).(string) != privateAttrVal { t.Fatalf("private attributes are merged: got %v; want %s", dst.PrivateAttr(privateAttrKey), privateAttrVal) @@ -44,7 +52,7 @@ func TestMergeRules_WithSortedStringAttr(t *testing.T) { sortedStringAttrVal := rule.SortedStrings{"@qux", "//foo:bar", "//foo:baz"} src.SetAttr(sortedStringAttrKey, sortedStringAttrVal) dst := rule.NewRule("go_library", "go_default_library") - rule.MergeRules(src, dst, map[string]bool{}, "") + rule.MergeRules(src, dst, depsMergeable, notMergeable, "") valExpr, ok := dst.Attr(sortedStringAttrKey).(*bzl.ListExpr) if !ok { @@ -68,7 +76,7 @@ func TestMergeRules_WithSortedStringAttr(t *testing.T) { src.SetAttr(sortedStringAttrKey, sortedStringAttrVal) dst := rule.NewRule("go_library", "go_default_library") dst.SetAttr(sortedStringAttrKey, rule.SortedStrings{"@qux", "//foo:bar", "//bacon:eggs"}) - rule.MergeRules(src, dst, map[string]bool{"deps": true}, "") + rule.MergeRules(src, dst, depsMergeable, notMergeable, "") valExpr, ok := dst.Attr(sortedStringAttrKey).(*bzl.ListExpr) if !ok { @@ -90,7 +98,7 @@ func TestMergeRules_WithSortedStringAttr(t *testing.T) { dst := rule.NewRule("go_library", "go_default_library") sortedStringAttrVal := rule.SortedStrings{"@qux", "//foo:bar", "//foo:baz"} dst.SetAttr(sortedStringAttrKey, sortedStringAttrVal) - rule.MergeRules(src, dst, map[string]bool{"deps": true}, "") + rule.MergeRules(src, dst, depsMergeable, notMergeable, "") if dst.Attr(sortedStringAttrKey) != nil { t.Fatalf("delete existing sorted strings: got %v; want nil", @@ -106,7 +114,7 @@ func TestMergeRules_WithUnsortedStringAttr(t *testing.T) { sortedStringAttrVal := rule.UnsortedStrings{"@qux", "//foo:bar", "//foo:baz"} src.SetAttr(sortedStringAttrKey, sortedStringAttrVal) dst := rule.NewRule("go_library", "go_default_library") - rule.MergeRules(src, dst, map[string]bool{}, "") + rule.MergeRules(src, dst, depsMergeable, notMergeable, "") valExpr, ok := dst.Attr(sortedStringAttrKey).(*bzl.ListExpr) if !ok { @@ -130,7 +138,7 @@ func TestMergeRules_WithUnsortedStringAttr(t *testing.T) { src.SetAttr(sortedStringAttrKey, sortedStringAttrVal) dst := rule.NewRule("go_library", "go_default_library") dst.SetAttr(sortedStringAttrKey, rule.UnsortedStrings{"@qux", "//foo:bar", "//bacon:eggs"}) - rule.MergeRules(src, dst, map[string]bool{"deps": true}, "") + rule.MergeRules(src, dst, depsMergeable, notMergeable, "") valExpr, ok := dst.Attr(sortedStringAttrKey).(*bzl.ListExpr) if !ok { @@ -147,3 +155,27 @@ func TestMergeRules_WithUnsortedStringAttr(t *testing.T) { } }) } + +func TestMergeRules_WithUnmanagedAttr(t *testing.T) { + t.Run("unknown string attributes overwrite attributes in non-empty rule", func(t *testing.T) { + src := rule.NewRule("go_library", "go_default_library") + unknownStringAttrKey := "unknown_attr" + overwrittenAttrVal := rule.UnsortedStrings{"@qux", "//foo:bar", "//foo:baz"} + src.SetAttr(unknownStringAttrKey, "foobar") + dst := rule.NewRule("go_library", "go_default_library") + dst.SetAttr(unknownStringAttrKey, overwrittenAttrVal) + rule.MergeRules(src, dst, notMergeable, notMergeable, "") + + valExpr, ok := dst.Attr(unknownStringAttrKey).(*bzl.StringExpr) + if !ok { + t.Fatalf("unknown attributes are overwritten: got %v; want *bzl.StringExpr", + reflect.TypeOf(dst.Attr(unknownStringAttrKey))) + } + + expected := "foobar" + if valExpr.Value != expected { + t.Fatalf("unknown attributes are overwritten: got %v; want %v", + valExpr.Value, expected) + } + }) +} From 3210bebfbdb08e945739717d1d95baf0c855ccae Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Wed, 23 Oct 2024 13:15:03 -0700 Subject: [PATCH 2/4] fixup! fix: overwrite attribute values when not mergeable or resolveable --- merger/merger.go | 40 ++++++++++++++++------------ rule/merge.go | 15 ++++++----- rule/merge_test.go | 65 +++++++++++++++++++++++++++++++++------------- 3 files changed, 78 insertions(+), 42 deletions(-) diff --git a/merger/merger.go b/merger/merger.go index 35becc7f7..61a6724e2 100644 --- a/merger/merger.go +++ b/merger/merger.go @@ -107,23 +107,23 @@ const UnstableInsertIndexKey = "_gazelle_insert_index" // If a rule is marked with a "# keep" comment, the whole rule will not // be modified. func MergeFile(oldFile *rule.File, emptyRules, genRules []*rule.Rule, phase Phase, kinds map[string]rule.KindInfo, aliasedKinds map[string]string) { - isMergeable := func(r *rule.Rule, attr string) bool { - // Attributes merged in this phase. - if phase == PreResolve { - return kinds[r.Kind()].MergeableAttrs[attr] - } else { - return kinds[r.Kind()].ResolveAttrs[attr] - } - } + getMergeAttrs := func(r *rule.Rule) map[string]bool { + kind := kinds[r.Kind()] - isManaged := func(r *rule.Rule, attr string) bool { - // Name and visibility are uniquely managed by gazelle. - if attr == "name" || attr == "visibility" { - return true + mergeableAttrs := map[string]bool{ + // Name and visibility are uniquely managed by gazelle. + "name": false, + "visibility": false, } - k := kinds[r.Kind()] - // All known attributes of this rule kind. - return k.NonEmptyAttrs[attr] || k.SubstituteAttrs[attr] || k.ResolveAttrs[attr] || k.MergeableAttrs[attr] + + // All attributes managed by gazelle, marking only the current merge attributes + // with a truthy value. + combineMergeAttrs(mergeableAttrs, kind.MergeableAttrs, phase == PreResolve) + combineMergeAttrs(mergeableAttrs, kind.ResolveAttrs, phase == PostResolve) + combineMergeAttrs(mergeableAttrs, kind.NonEmptyAttrs, false) + combineMergeAttrs(mergeableAttrs, kind.SubstituteAttrs, false) + + return mergeableAttrs } // Merge empty rules into the file and delete any rules which become empty. @@ -132,7 +132,7 @@ func MergeFile(oldFile *rule.File, emptyRules, genRules []*rule.Rule, phase Phas if oldRule.ShouldKeep() { continue } - rule.MergeRules(emptyRule, oldRule, isMergeable, isManaged, oldFile.Path) + rule.MergeRules(emptyRule, oldRule, getMergeAttrs(emptyRule), oldFile.Path) if oldRule.IsEmpty(kinds[oldRule.Kind()]) { oldRule.Delete() } @@ -180,11 +180,17 @@ func MergeFile(oldFile *rule.File, emptyRules, genRules []*rule.Rule, phase Phas genRule.Insert(oldFile) } } else { - rule.MergeRules(genRule, matchRules[i], isMergeable, isManaged, oldFile.Path) + rule.MergeRules(genRule, matchRules[i], getMergeAttrs(genRule), oldFile.Path) } } } +func combineMergeAttrs(dst map[string]bool, src map[string]bool, value bool) { + for k, _ := range src { + dst[k] = value || dst[k] + } +} + // substituteRule replaces local labels (those beginning with ":", referring to // targets in the same package) according to a substitution map. This is used // to update generated rules before merging when the corresponding existing diff --git a/rule/merge.go b/rule/merge.go index 6f5aebb33..cfc2524d6 100644 --- a/rule/merge.go +++ b/rule/merge.go @@ -24,8 +24,6 @@ import ( bzl "github.com/bazelbuild/buildtools/build" ) -type IsMergeable = func(r *Rule, attr string) bool - // MergeRules copies information from src into dst, usually discarding // information in dst when they have the same attributes. // @@ -43,14 +41,17 @@ type IsMergeable = func(r *Rule, attr string) bool // marked with a "# keep" comment, values in the attribute not marked with // a "# keep" comment will be dropped. If the attribute is empty afterward, // it will be deleted. -func MergeRules(src, dst *Rule, isMergeable, isManaged IsMergeable, filename string) { +// +// If src has an attribute not present in 'mergeable' and not marked with a +// "# keep" comment, values in the dist attribute will be overwritten. +func MergeRules(src, dst *Rule, mergeable map[string]bool, filename string) { if dst.ShouldKeep() { return } // Process attributes that are in dst but not in src. for key, dstAttr := range dst.attrs { - if _, ok := src.attrs[key]; ok || !isMergeable(src, key) || ShouldKeep(dstAttr.expr) { + if _, ok := src.attrs[key]; ok || !mergeable[key] || ShouldKeep(dstAttr.expr) { continue } if mergedValue, err := mergeAttrValues(nil, &dstAttr); err != nil { @@ -72,12 +73,12 @@ func MergeRules(src, dst *Rule, isMergeable, isManaged IsMergeable, filename str continue } - // Do not override attributes marked with "# keep". + // Do not overwrite attributes marked with "# keep". if ShouldKeep(dstAttr.expr) { continue } - if isMergeable(src, key) { + if attrMergeable, attrKnown := mergeable[key]; attrMergeable { // Merge mergeable attributes. if mergedValue, err := mergeAttrValues(&srcAttr, &dstAttr); err != nil { start, end := dstAttr.expr.RHS.Span() @@ -87,7 +88,7 @@ func MergeRules(src, dst *Rule, isMergeable, isManaged IsMergeable, filename str } else { dst.SetAttr(key, mergedValue) } - } else if !isManaged(src, key) { + } else if !attrKnown { // Overwrite unknown attributes. dst.SetAttr(key, srcAttr.expr.RHS) } diff --git a/rule/merge_test.go b/rule/merge_test.go index 35d863295..99d7b29ac 100644 --- a/rule/merge_test.go +++ b/rule/merge_test.go @@ -23,13 +23,6 @@ import ( bzl "github.com/bazelbuild/buildtools/build" ) -func depsMergeable(r *rule.Rule, attr string) bool { - return attr == "deps" -} -func notMergeable(r *rule.Rule, attr string) bool { - return false -} - func TestMergeRules(t *testing.T) { t.Run("private attributes are merged", func(t *testing.T) { src := rule.NewRule("go_library", "go_default_library") @@ -37,7 +30,7 @@ func TestMergeRules(t *testing.T) { privateAttrVal := "private_value" src.SetPrivateAttr(privateAttrKey, privateAttrVal) dst := rule.NewRule("go_library", "go_default_library") - rule.MergeRules(src, dst, notMergeable, notMergeable, "") + rule.MergeRules(src, dst, map[string]bool{}, "") if dst.PrivateAttr(privateAttrKey).(string) != privateAttrVal { t.Fatalf("private attributes are merged: got %v; want %s", dst.PrivateAttr(privateAttrKey), privateAttrVal) @@ -52,7 +45,7 @@ func TestMergeRules_WithSortedStringAttr(t *testing.T) { sortedStringAttrVal := rule.SortedStrings{"@qux", "//foo:bar", "//foo:baz"} src.SetAttr(sortedStringAttrKey, sortedStringAttrVal) dst := rule.NewRule("go_library", "go_default_library") - rule.MergeRules(src, dst, depsMergeable, notMergeable, "") + rule.MergeRules(src, dst, map[string]bool{"deps": true}, "") valExpr, ok := dst.Attr(sortedStringAttrKey).(*bzl.ListExpr) if !ok { @@ -76,7 +69,7 @@ func TestMergeRules_WithSortedStringAttr(t *testing.T) { src.SetAttr(sortedStringAttrKey, sortedStringAttrVal) dst := rule.NewRule("go_library", "go_default_library") dst.SetAttr(sortedStringAttrKey, rule.SortedStrings{"@qux", "//foo:bar", "//bacon:eggs"}) - rule.MergeRules(src, dst, depsMergeable, notMergeable, "") + rule.MergeRules(src, dst, map[string]bool{"deps": true}, "") valExpr, ok := dst.Attr(sortedStringAttrKey).(*bzl.ListExpr) if !ok { @@ -98,7 +91,7 @@ func TestMergeRules_WithSortedStringAttr(t *testing.T) { dst := rule.NewRule("go_library", "go_default_library") sortedStringAttrVal := rule.SortedStrings{"@qux", "//foo:bar", "//foo:baz"} dst.SetAttr(sortedStringAttrKey, sortedStringAttrVal) - rule.MergeRules(src, dst, depsMergeable, notMergeable, "") + rule.MergeRules(src, dst, map[string]bool{"deps": true}, "") if dst.Attr(sortedStringAttrKey) != nil { t.Fatalf("delete existing sorted strings: got %v; want nil", @@ -114,7 +107,7 @@ func TestMergeRules_WithUnsortedStringAttr(t *testing.T) { sortedStringAttrVal := rule.UnsortedStrings{"@qux", "//foo:bar", "//foo:baz"} src.SetAttr(sortedStringAttrKey, sortedStringAttrVal) dst := rule.NewRule("go_library", "go_default_library") - rule.MergeRules(src, dst, depsMergeable, notMergeable, "") + rule.MergeRules(src, dst, map[string]bool{"deps": true}, "") valExpr, ok := dst.Attr(sortedStringAttrKey).(*bzl.ListExpr) if !ok { @@ -138,7 +131,7 @@ func TestMergeRules_WithUnsortedStringAttr(t *testing.T) { src.SetAttr(sortedStringAttrKey, sortedStringAttrVal) dst := rule.NewRule("go_library", "go_default_library") dst.SetAttr(sortedStringAttrKey, rule.UnsortedStrings{"@qux", "//foo:bar", "//bacon:eggs"}) - rule.MergeRules(src, dst, depsMergeable, notMergeable, "") + rule.MergeRules(src, dst, map[string]bool{"deps": true}, "") valExpr, ok := dst.Attr(sortedStringAttrKey).(*bzl.ListExpr) if !ok { @@ -164,18 +157,54 @@ func TestMergeRules_WithUnmanagedAttr(t *testing.T) { src.SetAttr(unknownStringAttrKey, "foobar") dst := rule.NewRule("go_library", "go_default_library") dst.SetAttr(unknownStringAttrKey, overwrittenAttrVal) - rule.MergeRules(src, dst, notMergeable, notMergeable, "") + rule.MergeRules(src, dst, map[string]bool{}, "") valExpr, ok := dst.Attr(unknownStringAttrKey).(*bzl.StringExpr) if !ok { - t.Fatalf("unknown attributes are overwritten: got %v; want *bzl.StringExpr", - reflect.TypeOf(dst.Attr(unknownStringAttrKey))) + t.Fatalf("unknown attributes (%q) are overwritten: got %v; want *bzl.StringExpr", + unknownStringAttrKey, reflect.TypeOf(dst.Attr(unknownStringAttrKey))) } expected := "foobar" if valExpr.Value != expected { - t.Fatalf("unknown attributes are overwritten: got %v; want %v", - valExpr.Value, expected) + t.Fatalf("unknown attributes (%q) are overwritten: got %v; want %v", + unknownStringAttrKey, valExpr.Value, expected) + } + }) + + t.Run("known string attributes do NOT overwrite attributes in non-empty rule", func(t *testing.T) { + src := rule.NewRule("go_library", "go_default_library") + unknownStringAttrKey := "unknown_attr" + knownStringAttrKey := "knownAttrName" + overwrittenAttrVal := "original" + src.SetAttr(unknownStringAttrKey, "foobar") + src.SetAttr(knownStringAttrKey, "foobar") + dst := rule.NewRule("go_library", "go_default_library") + dst.SetAttr(unknownStringAttrKey, overwrittenAttrVal) + dst.SetAttr(knownStringAttrKey, overwrittenAttrVal) + rule.MergeRules(src, dst, map[string]bool{knownStringAttrKey: false}, "") + + // The unknown + overwritten attribute + unknownValExpr, ok := dst.Attr(unknownStringAttrKey).(*bzl.StringExpr) + if !ok { + t.Fatalf("unknown attributes (%q) are overwritten: got %v; want *bzl.StringExpr", + unknownStringAttrKey, reflect.TypeOf(dst.Attr(unknownStringAttrKey))) + } + expected := "foobar" + if unknownValExpr.Value != expected { + t.Fatalf("unknown attributes (%q) are overwritten: got %v; want %v", + unknownStringAttrKey, unknownValExpr.Value, expected) + } + + // The known but non-mergeable attributes + knownValExpr, ok := dst.Attr(knownStringAttrKey).(*bzl.StringExpr) + if !ok { + t.Fatalf("known attributes (%q) are NOT overwritten: got %v; want *bzl.StringExpr", + knownStringAttrKey, reflect.TypeOf(dst.Attr(knownStringAttrKey))) + } + if knownValExpr.Value != overwrittenAttrVal { + t.Fatalf("known attributes (%q) are NOT overwritten: got %v; want %v", + knownStringAttrKey, knownValExpr.Value, overwrittenAttrVal) } }) } From 844e000843743016f5f0508b50b6050c12d16d9d Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Fri, 25 Oct 2024 15:48:02 -0700 Subject: [PATCH 3/4] fixup! fix: overwrite attribute values when not mergeable or resolveable --- rule/merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rule/merge.go b/rule/merge.go index cfc2524d6..249f04260 100644 --- a/rule/merge.go +++ b/rule/merge.go @@ -43,7 +43,7 @@ import ( // it will be deleted. // // If src has an attribute not present in 'mergeable' and not marked with a -// "# keep" comment, values in the dist attribute will be overwritten. +// "# keep" comment, values in the dst attribute will be overwritten. func MergeRules(src, dst *Rule, mergeable map[string]bool, filename string) { if dst.ShouldKeep() { return From 87ba5e775dae8f30a07501a6d2d59ccb5b68a50b Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Fri, 25 Oct 2024 16:30:44 -0700 Subject: [PATCH 4/4] fixup! fix: overwrite attribute values when not mergeable or resolveable --- rule/merge.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rule/merge.go b/rule/merge.go index 249f04260..170fcf614 100644 --- a/rule/merge.go +++ b/rule/merge.go @@ -42,8 +42,9 @@ import ( // a "# keep" comment will be dropped. If the attribute is empty afterward, // it will be deleted. // -// If src has an attribute not present in 'mergeable' and not marked with a -// "# keep" comment, values in the dst attribute will be overwritten. +// If src has an attribute not present in 'mergeable' and the attribute +// does not exist or is not marked with a "# keep" comment, values in the +// dst attribute will be overwritten. func MergeRules(src, dst *Rule, mergeable map[string]bool, filename string) { if dst.ShouldKeep() { return