[mynewt-newt] 03/03: Merge conflicting dependency conditionals

[cc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ccollins pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git

commit fc6fb5c26ab53d1a4b6d40be89f4cd8091d2d39c
Author: Christopher Collins <cc...@apache.org>
AuthorDate: Wed Nov 7 17:33:55 2018 -0800

    Merge conflicting dependency conditionals
    
    This commit addresses the following example:
    
        pkg.deps.FOO:
            - my_pkg
        pkg.deps.BAR:
            - my_pkg
    
    This package has two conflicting conditional dependencies.
    
    Before this commit, newt would only process one of the dependencies, and
    it did so in an inconsistent order.  So, the build would end up with one
    or the other of the dependencies, but never both.
    
    Now, newt merges such conflicting conditional dependencies into a single
    dependency.  The resulting dependency is conditional on the union of all
    the sub expreessions.  Newt would (internally) convert the above example
    to:
    
        pkg.deps.'(FOO) || (BAR)'
            - my_pkg
---
 newt/builder/depgraph.go |  5 +--
 newt/parse/parse.go      | 34 ++++++++++++++++---
 newt/resolve/resolve.go  | 85 ++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 111 insertions(+), 13 deletions(-)

diff --git a/newt/builder/depgraph.go b/newt/builder/depgraph.go
index ba12d3c..165efef 100644
--- a/newt/builder/depgraph.go
+++ b/newt/builder/depgraph.go
@@ -118,8 +118,9 @@ func depString(dep *resolve.ResolveDep) string {
 		reasons = append(reasons, fmt.Sprintf("api:%s", dep.Api))
 	}
 
-	if dep.Expr != "" {
-		reasons = append(reasons, fmt.Sprintf("syscfg:%s", dep.Expr))
+	exprStr := dep.ExprString()
+	if exprStr != "" {
+		reasons = append(reasons, fmt.Sprintf("syscfg:%s", exprStr))
 	}
 
 	if len(reasons) > 0 {
diff --git a/newt/parse/parse.go b/newt/parse/parse.go
index f6aeba0..89e1371 100644
--- a/newt/parse/parse.go
+++ b/newt/parse/parse.go
@@ -554,6 +554,21 @@ func Eval(expr *Node, settings map[string]string) (bool, error) {
 	}
 }
 
+func lexAndParse(expr string) (*Node, error) {
+	tokens, err := Lex(expr)
+	if err != nil {
+		return nil, err
+	}
+
+	n, err := Parse(tokens)
+	if err != nil {
+		return nil, util.FmtNewtError("error parsing [%s]: %s",
+			expr, err.Error())
+	}
+
+	return n, nil
+}
+
 // Parses and evaluates string containing a syscfg expression.
 //
 // @param expr                  The expression to parse.
@@ -561,18 +576,27 @@ func Eval(expr *Node, settings map[string]string) (bool, error) {
 //
 // @return bool                 Whether the expression evaluates to true.
 func ParseAndEval(expr string, settings map[string]string) (bool, error) {
-	tokens, err := Lex(expr)
+	n, err := lexAndParse(expr)
 	if err != nil {
 		return false, err
 	}
 
-	n, err := Parse(tokens)
+	v, err := Eval(n, settings)
+	return v, err
+}
+
+// Parses an expression and converts it to its normalized text form.
+func NormalizeExpr(expr string) (string, error) {
+	n, err := lexAndParse(expr)
 	if err != nil {
-		return false, fmt.Errorf("error parsing [%s]: %s\n", expr, err.Error())
+		return "", err
 	}
 
-	v, err := Eval(n, settings)
-	return v, err
+	if n == nil {
+		return "", nil
+	}
+
+	return n.String(), nil
 }
 
 // Evaluates the truthfulness of a text expression.
diff --git a/newt/resolve/resolve.go b/newt/resolve/resolve.go
index ef13e6a..46e6294 100644
--- a/newt/resolve/resolve.go
+++ b/newt/resolve/resolve.go
@@ -28,6 +28,7 @@ import (
 
 	"mynewt.apache.org/newt/newt/flash"
 	"mynewt.apache.org/newt/newt/logcfg"
+	"mynewt.apache.org/newt/newt/parse"
 	"mynewt.apache.org/newt/newt/pkg"
 	"mynewt.apache.org/newt/newt/project"
 	"mynewt.apache.org/newt/newt/syscfg"
@@ -77,8 +78,8 @@ type ResolveDep struct {
 	// Name of API that generated the dependency; "" if a hard dependency.
 	Api string
 
-	// Text of syscfg expression that generated this dependency; "" for none.
-	Expr string
+	// Set of syscfg expressions that generated this dependency.
+	ExprMap map[string]struct{}
 }
 
 type ResolvePackage struct {
@@ -175,7 +176,48 @@ func NewResolvePkg(lpkg *pkg.LocalPackage) *ResolvePackage {
 	}
 }
 
-func (r *Resolver) resolveDep(dep *pkg.Dependency, depender string) (*pkg.LocalPackage, error) {
+// Creates an expression string from all the conditionals associated with the
+// dependency.  The resulting expression is the union of all sub expressions.
+// For example:
+//     pkg.deps.FOO:
+//         - my_pkg
+//     pkg.deps.BAR:
+//         - my_pkg
+//
+// The expression string for the `my_pkg` dependency is:
+//     (FOO) || (BAR)
+func (rdep *ResolveDep) ExprString() string {
+	// If there is an unconditional dependency, the conditional dependencies
+	// can be ignored.
+	if _, ok := rdep.ExprMap[""]; ok {
+		return ""
+	}
+
+	exprs := make([]string, 0, len(rdep.ExprMap))
+	for expr, _ := range rdep.ExprMap {
+		exprs = append(exprs, expr)
+	}
+
+	// The union of one object is itself.
+	if len(exprs) == 1 {
+		return exprs[0]
+	}
+
+	// Sort all the subexpressions and OR them together.
+	sort.Strings(exprs)
+	s := ""
+	for i, expr := range exprs {
+		if i != 0 {
+			s += fmt.Sprintf(" || ")
+		}
+		s += fmt.Sprintf("(%s)", expr)
+	}
+	return s
+}
+
+func (r *Resolver) resolveDep(dep *pkg.Dependency,
+	depender string) (*pkg.LocalPackage, error) {
+
 	proj := project.GetProject()
 
 	if proj.ResolveDependency(dep) == nil {
@@ -193,16 +235,47 @@ func (rpkg *ResolvePackage) AddDep(
 	depPkg *ResolvePackage, api string, expr string) bool {
 
 	if dep := rpkg.Deps[depPkg]; dep != nil {
+		// This package already depends on dep.  If the conditional expression
+		// is new, or if the API string is different, then the existing
+		// dependency needs to be updated with the new information.  Otherwise,
+		// ignore the duplicate.
+
+		// Determine if this is a new conditional expression.
+		oldExpr := dep.ExprString()
+
+		norm, err := parse.NormalizeExpr(expr)
+		if err != nil {
+			panic("invalid expression, should have been caught earlier: " +
+				err.Error())
+		}
+
+		dep.ExprMap[norm] = struct{}{}
+		merged := dep.ExprString()
+
+		changed := false
+
+		if oldExpr != merged {
+			log.Debugf("Package %s has conflicting dependencies on %s: "+
+				"old=`%s` new=`%s`; merging them into a single conditional: "+
+				"`%s`",
+				rpkg.Lpkg.FullName(), dep.Rpkg.Lpkg.FullName(),
+				oldExpr, expr, merged)
+			changed = true
+		}
+
 		if dep.Api != "" && api == "" {
 			dep.Api = api
-		} else {
-			return false
+			changed = true
 		}
+
+		return changed
 	} else {
 		rpkg.Deps[depPkg] = &ResolveDep{
 			Rpkg: depPkg,
 			Api:  api,
-			Expr: expr,
+			ExprMap: map[string]struct{}{
+				expr: struct{}{},
+			},
 		}
 	}