Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rule: unset attribute if src is nil and no keep annotation in dst when merge #1993

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 42 additions & 7 deletions rule/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,14 @@ func MergeRules(src, dst *Rule, mergeable map[string]bool, filename string) {
//
// The following kinds of expressions are recognized.
//
// * nil
// * strings (can only be merged with strings)
// * lists of strings
// * a call to select with a dict argument. The dict keys must be strings,
// - nil
// - strings (can only be merged with strings)
// - lists of strings
// - a call to select with a dict argument. The dict keys must be strings,
// and the values must be lists of strings.
// * a list of strings combined with a select call using +. The list must
// - a list of strings combined with a select call using +. The list must
// be the left operand.
// * an attr value that implements the Merger interface.
// - an attr value that implements the Merger interface.
//
// An error is returned if the expressions can't be merged, for example
// because they are not in one of the above formats.
Expand All @@ -102,7 +102,7 @@ func mergeAttrValues(srcAttr, dstAttr *attrValue) (bzl.Expr, error) {
return nil, nil
}
dst := dstAttr.expr.RHS
if srcAttr == nil && (dst == nil || isScalar(dst)) {
if srcAttr == nil && (dst == nil || !shouldKeepRecursively(dst)) {
return nil, nil
}
if srcAttr != nil && isScalar(srcAttr.expr.RHS) {
Expand Down Expand Up @@ -138,6 +138,41 @@ func mergeAttrValues(srcAttr, dstAttr *attrValue) (bzl.Expr, error) {
return makePlatformStringsExpr(mergedExprs), nil
}

func shouldKeepRecursively(e bzl.Expr) bool {
if ShouldKeep(e) {
return true
}
switch e := e.(type) {
case *bzl.AssignExpr:
if shouldKeepRecursively(e.RHS) {
return true
}
case *bzl.DictExpr:
for _, kv := range e.List {
if shouldKeepRecursively(kv.Value) {
return true
}
}
case *bzl.ListExpr:
for _, el := range e.List {
if shouldKeepRecursively(el) {
return true
}
}
case *bzl.CallExpr:
for _, arg := range e.List {
if shouldKeepRecursively(arg) {
return true
}
}
case *bzl.BinaryExpr:
if shouldKeepRecursively(e.X) || shouldKeepRecursively(e.Y) {
return true
}
}
return false
}

func mergePlatformStringsExprs(src, dst platformStringsExprs) (platformStringsExprs, error) {
var ps platformStringsExprs
var err error
Expand Down