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
The following commit(s) were added to refs/heads/master by this push:
new 430b0bb Discard duplicates in merged dep conditional
430b0bb is described below
commit 430b0bb91da119e67a8111f772ef9b287426ce58
Author: Christopher Collins <[email protected]>
AuthorDate: Mon Nov 26 13:27:12 2018 -0800
Discard duplicates in merged dep conditional
Newt merges conditional dependencies into a single dependency. For
example, newt transforms the following:
pkg.deps.SETTING_A:
- my_package
pkg.deps.SETTING_B:
- my_package
into:
pkg.deps.'(SETTING_A || SETTING B):
- my_package
(internally)
This allows a `pkg.yml` file to declare separate conditions under which
a package gets pulled in, rather than requiring the user to manually
collapse them all into a single expression.
The bug is that sometimes newt would add duplicate expressions to the
merged expression, e.g.,
pkg.deps.'((SETTING_A || SETTING_B) || SETTING_B)'
The resulting dependency graph is unchanged, but the text in the log and
in the dep / revdep commands was ugly and confusing.
The problem was that newt was not always normalizing each expression
before comparing it to the previously-seen expressions. If an
expression contained extra whitespace, it would not compare equal to the
equivalent whitespace-free expression. This commit fixes this issue by
always normalizing conditionals before storing them.
---
newt/resolve/resolve.go | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/newt/resolve/resolve.go b/newt/resolve/resolve.go
index 46e6294..c9ccb42 100644
--- a/newt/resolve/resolve.go
+++ b/newt/resolve/resolve.go
@@ -234,6 +234,12 @@ func (r *Resolver) resolveDep(dep *pkg.Dependency,
func (rpkg *ResolvePackage) AddDep(
depPkg *ResolvePackage, api string, expr string) bool {
+ norm, err := parse.NormalizeExpr(expr)
+ if err != nil {
+ panic("invalid expression, should have been caught earlier: " +
+ err.Error())
+ }
+
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
@@ -243,18 +249,11 @@ func (rpkg *ResolvePackage) AddDep(
// 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 _, ok := dep.ExprMap[norm]; !ok {
+ dep.ExprMap[norm] = struct{}{}
+ merged := dep.ExprString()
- if oldExpr != merged {
log.Debugf("Package %s has conflicting dependencies on
%s: "+
"old=`%s` new=`%s`; merging them into a single
conditional: "+
"`%s`",
@@ -274,7 +273,7 @@ func (rpkg *ResolvePackage) AddDep(
Rpkg: depPkg,
Api: api,
ExprMap: map[string]struct{}{
- expr: struct{}{},
+ norm: struct{}{},
},
}
}
