You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2018/05/04 18:52:09 UTC

[GitHub] ccollins476ad closed pull request #163: Fix conditional dependency lists

ccollins476ad closed pull request #163: Fix conditional dependency lists
URL: https://github.com/apache/mynewt-newt/pull/163
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/newt/resolve/resolve.go b/newt/resolve/resolve.go
index 6e2cd187..9d329519 100644
--- a/newt/resolve/resolve.go
+++ b/newt/resolve/resolve.go
@@ -24,8 +24,6 @@ import (
 	"sort"
 	"strings"
 
-	log "github.com/Sirupsen/logrus"
-
 	"mynewt.apache.org/newt/newt/flash"
 	"mynewt.apache.org/newt/newt/pkg"
 	"mynewt.apache.org/newt/newt/project"
@@ -80,9 +78,11 @@ type ResolvePackage struct {
 	// Keeps track of API requirements and whether they are satisfied.
 	reqApiMap map[string]resolveReqApi
 
-	depsResolved  bool
-	apisSatisfied bool
-	refCount      int
+	depsResolved bool
+
+	// Tracks this package's number of dependents.  If this number reaches 0,
+	// this package can be deleted from the resolver.
+	refCount int
 }
 
 type ResolveSet struct {
@@ -180,7 +180,6 @@ func (rpkg *ResolvePackage) AddDep(
 	if dep := rpkg.Deps[apiPkg]; dep != nil {
 		if dep.Api != "" && api == "" {
 			dep.Api = api
-			return true
 		} else {
 			return false
 		}
@@ -190,8 +189,19 @@ func (rpkg *ResolvePackage) AddDep(
 			Api:  api,
 			Expr: expr,
 		}
-		return true
 	}
+
+	// If this dependency came from an API requirement, record that the API
+	// requirement is now satisfied.
+	if api != "" {
+		apiReq := rpkg.reqApiMap[api]
+		apiReq.expr = expr
+		apiReq.satisfied = true
+		rpkg.reqApiMap[api] = apiReq
+	}
+
+	apiPkg.refCount++
+	return true
 }
 
 func (r *Resolver) rpkgSlice() []*ResolvePackage {
@@ -229,108 +239,83 @@ func (r *Resolver) addPkg(lpkg *pkg.LocalPackage) (*ResolvePackage, bool) {
 
 	rpkg := NewResolvePkg(lpkg)
 	r.pkgMap[lpkg] = rpkg
-	rpkg.refCount = 1
 	return rpkg, true
 }
 
-// @return bool                 true if this is a new API.
-func (r *Resolver) addApi(
-	apiString string, rpkg *ResolvePackage, expr string) bool {
-
-	api := r.apis[apiString]
-	if api.rpkg == nil {
-		r.apis[apiString] = resolveApi{
-			rpkg: rpkg,
-			expr: expr,
-		}
-		return true
-	} else {
-		if api.rpkg != rpkg {
-			if r.apiConflicts[apiString] == nil {
-				r.apiConflicts[apiString] = map[*ResolvePackage]struct{}{}
-			}
-			r.apiConflicts[apiString][api.rpkg] = struct{}{}
-			r.apiConflicts[apiString][rpkg] = struct{}{}
-		}
-		return false
-	}
-}
-
-func joinExprs(expr1 string, expr2 string) string {
-	if expr1 == "" {
-		return expr2
-	}
-	if expr2 == "" {
-		return expr1
+func (r *Resolver) sortedRpkgs() []*ResolvePackage {
+	rpkgs := make([]*ResolvePackage, 0, len(r.pkgMap))
+	for _, rpkg := range r.pkgMap {
+		rpkgs = append(rpkgs, rpkg)
 	}
 
-	return expr1 + "," + expr2
+	SortResolvePkgs(rpkgs)
+	return rpkgs
 }
 
-// Searches for a package which can satisfy bpkg's API requirement.  If such a
-// package is found, bpkg's API requirement is marked as satisfied, and the
-// package is added to bpkg's dependency list.
-//
-// @return bool                 true if the API is now satisfied.
-func (r *Resolver) satisfyApi(
-	rpkg *ResolvePackage, reqApi string, expr string) bool {
-
-	api := r.apis[reqApi]
-	if api.rpkg == nil {
-		// Insert null entry to indicate an unsatisfied API.
-		r.apis[reqApi] = resolveApi{}
-		return false
+// Selects the final API suppliers among all packages implementing APIs.  The
+// result gets written to the resolver's `apis` map.  If more than one package
+// implements the same API, an API conflict error is recorded.
+func (r *Resolver) selectApiSuppliers() {
+	apiMap := map[string][]resolveApi{}
+
+	for _, rpkg := range r.sortedRpkgs() {
+		settings := r.cfg.SettingsForLpkg(rpkg.Lpkg)
+		apiStrings := rpkg.Lpkg.PkgY.GetSlice("pkg.apis", settings)
+		for _, entry := range apiStrings {
+			apiStr, ok := entry.Value.(string)
+			if ok && apiStr != "" {
+				apiMap[apiStr] = append(apiMap[apiStr], resolveApi{
+					rpkg: rpkg,
+					expr: entry.Expr,
+				})
+			}
+		}
 	}
 
-	rpkg.reqApiMap[reqApi] = resolveReqApi{
-		satisfied: true,
-		expr:      expr,
+	apiNames := make([]string, 0, len(apiMap))
+	for name, _ := range apiMap {
+		apiNames = append(apiNames, name)
 	}
+	sort.Strings(apiNames)
 
-	// This package now has a new unresolved dependency.
-	rpkg.depsResolved = false
-
-	log.Debugf("API requirement satisfied; pkg=%s API=(%s, %s) expr=(%s)",
-		rpkg.Lpkg.Name(), reqApi, api.rpkg.Lpkg.FullName(), expr)
-
-	return true
+	for _, name := range apiNames {
+		apis := apiMap[name]
+		for _, api := range apis {
+			old := r.apis[name]
+			if old.rpkg != nil {
+				if r.apiConflicts[name] == nil {
+					r.apiConflicts[name] = map[*ResolvePackage]struct{}{}
+				}
+				r.apiConflicts[name][api.rpkg] = struct{}{}
+				r.apiConflicts[name][old.rpkg] = struct{}{}
+			} else {
+				r.apis[name] = api
+			}
+		}
+	}
 }
 
-// @return bool                 true if a new dependency was detected as a
-//                                  result of satisfying an API for this
-//                                  package.
-func (r *Resolver) satisfyApis(rpkg *ResolvePackage) bool {
-	// Assume all this package's APIs are satisfied and that no new
-	// dependencies will be detected.
-	rpkg.apisSatisfied = true
-	newDeps := false
-
+// Populates the specified package's set of API requirements.
+func (r *Resolver) calcApiReqsFor(rpkg *ResolvePackage) {
 	settings := r.cfg.SettingsForLpkg(rpkg.Lpkg)
 
-	// Determine if any of the package's API requirements can now be satisfied.
-	// If so, another full iteration is required.
 	reqApiEntries := rpkg.Lpkg.PkgY.GetSlice("pkg.req_apis", settings)
 	for _, entry := range reqApiEntries {
 		apiStr, ok := entry.Value.(string)
 		if ok && apiStr != "" {
-			if !rpkg.reqApiMap[apiStr].satisfied {
-				apiSatisfied := r.satisfyApi(rpkg, apiStr, entry.Expr)
-				if apiSatisfied {
-					// An API was satisfied; the package now has a new
-					// dependency that needs to be resolved.
-					newDeps = true
-				} else {
-					rpkg.reqApiMap[apiStr] = resolveReqApi{
-						satisfied: false,
-						expr:      "",
-					}
-					rpkg.apisSatisfied = false
-				}
+			rpkg.reqApiMap[apiStr] = resolveReqApi{
+				satisfied: false,
+				expr:      "",
 			}
 		}
 	}
+}
 
-	return newDeps
+// Populates all packages' API requirements sets.
+func (r *Resolver) calcApiReqs() {
+	for _, rpkg := range r.pkgMap {
+		r.calcApiReqsFor(rpkg)
+	}
 }
 
 // @return bool                 True if this this function changed the resolver
@@ -368,22 +353,23 @@ func (r *Resolver) loadDepsForPkg(rpkg *ResolvePackage) (bool, error) {
 		}
 	}
 
+	// This iteration may have deleted some dependency relationships (e.g., if
+	// a new syscfg setting was discovered which causes this package's
+	// dependency list to be overwritten).  Detect and delete these
+	// relationships.
 	for rdep, _ := range rpkg.Deps {
 		if _, ok := seen[rdep]; !ok {
 			delete(rpkg.Deps, rdep)
 			rdep.refCount--
 			changed = true
-		}
-	}
 
-	// Determine if this package supports any APIs that we haven't seen
-	// yet.  If so, another full iteration is required.
-	apiEntries := rpkg.Lpkg.PkgY.GetSlice("pkg.apis", settings)
-	for _, entry := range apiEntries {
-		apiStr, ok := entry.Value.(string)
-		if ok && apiStr != "" {
-			if r.addApi(apiStr, rpkg, entry.Expr) {
-				changed = true
+			// If we just deleted the last reference to a package, remove the
+			// package entirely from the resolver and syscfg.
+			if rdep.refCount == 0 {
+				delete(r.pkgMap, rdep.Lpkg)
+
+				// Delete the package from syscfg.
+				r.cfg.DeletePkg(rdep.Lpkg)
 			}
 		}
 	}
@@ -413,13 +399,6 @@ func (r *Resolver) resolvePkg(rpkg *ResolvePackage) (bool, error) {
 		rpkg.depsResolved = !newDeps
 	}
 
-	if !rpkg.apisSatisfied {
-		newApiDep := r.satisfyApis(rpkg)
-		if newApiDep {
-			newDeps = true
-		}
-	}
-
 	return newDeps, nil
 }
 
@@ -477,11 +456,24 @@ func (r *Resolver) resolveDepsOnce() (bool, error) {
 	return newDeps, nil
 }
 
+// Given a fully calculated syscfg and API map, resolves package dependencies
+// by populating the resolver's package map.  This function should only be
+// called if the resolver's syscfg (`cfg`) member is assigned.  This only
+// happens for split images when the individual loader and app components are
+// resolved separately, after the master syscfg and API map have been
+// calculated.
 func (r *Resolver) resolveDeps() ([]*ResolvePackage, error) {
 	if _, err := r.resolveDepsOnce(); err != nil {
 		return nil, err
 	}
 
+	// Now that the final set of packages is known, determine which ones
+	// satisfy each required API.
+	r.selectApiSuppliers()
+
+	// Determine which packages have API requirements.
+	r.calcApiReqs()
+
 	// Satisfy API requirements.
 	if err := r.resolveApiDeps(); err != nil {
 		return nil, err
@@ -491,6 +483,10 @@ func (r *Resolver) resolveDeps() ([]*ResolvePackage, error) {
 	return rpkgs, nil
 }
 
+// Performs a set of resolution actions:
+// 1. Calculates the system configuration (syscfg).
+// 2. Determines which packages satisfy which API requirements.
+// 3. Resolves package dependencies by populating the resolver's package map.
 func (r *Resolver) resolveDepsAndCfg() error {
 	if _, err := r.resolveDepsOnce(); err != nil {
 		return err
@@ -508,7 +504,6 @@ func (r *Resolver) resolveDepsAndCfg() error {
 			// reprocessed.
 			for _, rpkg := range r.pkgMap {
 				rpkg.depsResolved = false
-				rpkg.apisSatisfied = false
 			}
 		}
 
@@ -522,34 +517,52 @@ func (r *Resolver) resolveDepsAndCfg() error {
 		}
 	}
 
+	// Now that the final set of packages is known, determine which ones
+	// satisfy each required API.
+	r.selectApiSuppliers()
+
+	// Determine which packages have API requirements.
+	r.calcApiReqs()
+
 	// Satisfy API requirements.
 	if err := r.resolveApiDeps(); err != nil {
 		return err
 	}
 
-	// Remove orphaned packages.
-	for lpkg, rpkg := range r.pkgMap {
-		if rpkg.refCount == 0 {
-			delete(r.pkgMap, lpkg)
-		}
-	}
-
 	// Log the final syscfg.
 	r.cfg.Log()
 
 	return nil
 }
 
+func joinExprs(expr1 string, expr2 string) string {
+	if expr1 == "" {
+		return expr2
+	}
+	if expr2 == "" {
+		return expr1
+	}
+
+	return expr1 + "," + expr2
+}
+
 // Transforms each package's required APIs to hard dependencies.  That is, this
 // function determines which package supplies each required API, and adds the
 // corresponding dependecy to each package which requires the API.
 func (r *Resolver) resolveApiDeps() error {
 	for _, rpkg := range r.pkgMap {
 		for apiString, reqApi := range rpkg.reqApiMap {
-			api := r.apis[apiString]
-			if api.rpkg != nil {
+			// Determine which package satisfies this API requirement.
+			api, ok := r.apis[apiString]
+
+			// If there is a package that supports the requested API, add a
+			// hard dependency to the package.  Otherwise, record an
+			// unsatisfied API requirement with an empty API struct.
+			if ok && api.rpkg != nil {
 				rpkg.AddDep(api.rpkg, apiString,
 					joinExprs(api.expr, reqApi.expr))
+			} else if !ok {
+				r.apis[apiString] = resolveApi{}
 			}
 		}
 	}
@@ -608,9 +621,6 @@ func ResolveFull(
 
 	res := newResolution()
 	res.Cfg = r.cfg
-	if err := r.resolveApiDeps(); err != nil {
-		return nil, err
-	}
 
 	// Determine which package satisfies each API and which APIs are
 	// unsatisfied.
diff --git a/newt/syscfg/syscfg.go b/newt/syscfg/syscfg.go
index 3de31bb2..8f5fb193 100644
--- a/newt/syscfg/syscfg.go
+++ b/newt/syscfg/syscfg.go
@@ -116,6 +116,10 @@ type Cfg struct {
 	PriorityViolations []CfgPriority
 
 	FlashConflicts []CfgFlashConflict
+
+	// Multiple packages defining the same setting.
+	// [setting-name][defining-package][{}]
+	Redefines map[string]map[*pkg.LocalPackage]struct{}
 }
 
 func NewCfg() Cfg {
@@ -126,6 +130,7 @@ func NewCfg() Cfg {
 		Violations:         map[string][]CfgRestriction{},
 		PriorityViolations: []CfgPriority{},
 		FlashConflicts:     []CfgFlashConflict{},
+		Redefines:          map[string]map[*pkg.LocalPackage]struct{}{},
 	}
 }
 
@@ -145,7 +150,7 @@ func (cfg *Cfg) SettingsForLpkg(lpkg *pkg.LocalPackage) map[string]string {
 		_, ok := settings[k]
 		if ok {
 			log.Warnf("Attempt to override syscfg setting %s with "+
-				"injected feature from package %s", k, lpkg.Name())
+				"injected feature from package %s", k, lpkg.FullName())
 		} else {
 			settings[k] = v
 		}
@@ -158,7 +163,7 @@ func (point CfgPoint) Name() string {
 	if point.Source == nil {
 		return "newt"
 	} else {
-		return point.Source.Name()
+		return point.Source.FullName()
 	}
 }
 
@@ -177,6 +182,12 @@ func (entry *CfgEntry) appendValue(lpkg *pkg.LocalPackage, value interface{}) {
 	entry.Value = strval
 }
 
+// Replaces the source (defining) package in a syscfg entry.
+func (entry *CfgEntry) replaceSource(lpkg *pkg.LocalPackage) {
+	entry.PackageDef = lpkg
+	entry.History[0].Source = lpkg
+}
+
 func historyToString(history []CfgPoint) string {
 	if len(history) == 0 {
 		return "(undefined)"
@@ -245,13 +256,38 @@ func (entry *CfgEntry) ambiguityText() string {
 			str += ", "
 		}
 
-		str += p.Source.Name()
+		str += p.Source.FullName()
 	}
 	str += "]"
 
 	return str
 }
 
+// Detects all priority violations in an entry and returns them in a slice.
+func (entry *CfgEntry) priorityViolations() []CfgPriority {
+	var violations []CfgPriority
+
+	for i := 1; i < len(entry.History); i++ {
+		p := entry.History[i]
+
+		// Overrides must come from a higher priority package, with one
+		// exception: a package can override its own setting.
+		curType := normalizePkgType(p.Source.Type())
+		defType := normalizePkgType(entry.PackageDef.Type())
+
+		if p.Source != entry.PackageDef && curType <= defType {
+			priority := CfgPriority{
+				PackageDef:  entry.PackageDef,
+				PackageSrc:  p.Source,
+				SettingName: entry.Name,
+			}
+			violations = append(violations, priority)
+		}
+	}
+
+	return violations
+}
+
 func FeatureToCflag(featureName string) string {
 	return fmt.Sprintf("-D%s=1", settingName(featureName))
 }
@@ -305,6 +341,22 @@ func readSetting(name string, lpkg *pkg.LocalPackage,
 	return entry, nil
 }
 
+// Records a redefinition error (two packages defining the same setting).
+func (cfg *Cfg) addRedefine(settingName string,
+	pkg1 *pkg.LocalPackage, pkg2 *pkg.LocalPackage) {
+
+	if cfg.Redefines[settingName] == nil {
+		cfg.Redefines[settingName] = map[*pkg.LocalPackage]struct{}{}
+	}
+
+	log.Debugf("Detected multiple definitions of the same setting: "+
+		"setting=%s pkg1=%s pkg2=%s",
+		settingName, pkg1.FullName(), pkg2.FullName())
+
+	cfg.Redefines[settingName][pkg1] = struct{}{}
+	cfg.Redefines[settingName][pkg2] = struct{}{}
+}
+
 func (cfg *Cfg) readDefsOnce(lpkg *pkg.LocalPackage,
 	settings map[string]string) error {
 	yc := lpkg.SyscfgY
@@ -326,29 +378,49 @@ func (cfg *Cfg) readDefsOnce(lpkg *pkg.LocalPackage,
 			entry, err := readSetting(k, lpkg, vals)
 			if err != nil {
 				return util.FmtNewtError("Config for package %s: %s",
-					lpkg.Name(), err.Error())
+					lpkg.FullName(), err.Error())
 			}
 
+			replace := true
 			if oldEntry, exists := cfg.Settings[k]; exists {
-				// Setting already defined.  Warn the user and keep the
-				// original definition.
+				// Setting already defined.  This is allowed for injected
+				// settings (settings automatically populated by newt).  For
+				// all other settings, record a redefinition error.
 				point := mostRecentPoint(oldEntry)
-				if !point.IsInjected() {
-					util.StatusMessage(util.VERBOSITY_QUIET,
-						"* Warning: setting %s redefined (pkg1=%s pkg2=%s)\n",
-						k, point.Source.Name(), lpkg.Name())
+				if point.IsInjected() {
+					// Indicate that the setting came from this package, not
+					// from newt.
+					entry.replaceSource(lpkg)
+
+					// Keep the injected value (override the default).
+					entry.Value = oldEntry.Value
+				} else {
+					replace = false
+					cfg.addRedefine(k, oldEntry.PackageDef, lpkg)
 				}
+			}
 
-				entry.History = append(entry.History, oldEntry.History...)
-				entry.Value = oldEntry.Value
+			if replace {
+				// Not an illegal redefine; populate the master settings list
+				// with the new entry.
+				cfg.Settings[k] = entry
 			}
-			cfg.Settings[k] = entry
 		}
 	}
 
 	return nil
 }
 
+// Records an orphan override error (override of undefined setting).
+func (cfg *Cfg) addOrphan(settingName string, value string,
+	lpkg *pkg.LocalPackage) {
+
+	cfg.Orphans[settingName] = append(cfg.Orphans[settingName], CfgPoint{
+		Value:  value,
+		Source: lpkg,
+	})
+}
+
 func (cfg *Cfg) readValsOnce(lpkg *pkg.LocalPackage,
 	settings map[string]string) error {
 	yc := lpkg.SyscfgY
@@ -367,28 +439,10 @@ func (cfg *Cfg) readValsOnce(lpkg *pkg.LocalPackage,
 	for k, v := range values {
 		entry, ok := cfg.Settings[k]
 		if ok {
-			sourcetype := normalizePkgType(lpkg.Type())
-			deftype := normalizePkgType(entry.PackageDef.Type())
-
-			// Overrides must come from a higher priority package, with one
-			// exception: a package can override its own setting.
-			if lpkg != entry.PackageDef && sourcetype <= deftype {
-				priority := CfgPriority{
-					PackageDef:  entry.PackageDef,
-					PackageSrc:  lpkg,
-					SettingName: k,
-				}
-				cfg.PriorityViolations = append(cfg.PriorityViolations, priority)
-			} else {
-				entry.appendValue(lpkg, v)
-				cfg.Settings[k] = entry
-			}
+			entry.appendValue(lpkg, v)
+			cfg.Settings[k] = entry
 		} else {
-			orphan := CfgPoint{
-				Value:  stringValue(v),
-				Source: lpkg,
-			}
-			cfg.Orphans[k] = append(cfg.Orphans[k], orphan)
+			cfg.addOrphan(k, stringValue(v), lpkg)
 		}
 	}
 
@@ -413,6 +467,64 @@ func (cfg *Cfg) Log() {
 	}
 }
 
+// Removes all traces of a package from syscfg.
+func (cfg *Cfg) DeletePkg(lpkg *pkg.LocalPackage) {
+	log.Debugf("Deleting package from syscfg: %s", lpkg.FullName())
+
+	// First, check if the specified package contributes to a redefinition
+	// error.  If so, the deletion of this package might resolve the error.
+	for name, m := range cfg.Redefines {
+		delete(m, lpkg)
+		if len(m) == 1 {
+			// Only one package defines this setting now; the redefinition
+			// error is resolved.
+			delete(cfg.Redefines, name)
+
+			//Loop through the map `m` just to get access to its only element.
+			var source *pkg.LocalPackage
+			for lp, _ := range m {
+				source = lp
+			}
+
+			// Replace the setting's source with the one remaining package that
+			// defines the setting.
+			entry := cfg.Settings[name]
+			entry.replaceSource(source)
+
+			// Update the master settings map.
+			cfg.Settings[name] = entry
+
+			log.Debugf("Resolved syscfg redefine; setting=%s pkg=%s",
+				name, source.FullName())
+		}
+	}
+
+	// Next, delete the specified package from the master settings map.
+	for name, entry := range cfg.Settings {
+		if entry.PackageDef == lpkg {
+			// The package-to-delete is this setting's source.  All overrides
+			// become orphans.
+			for i := 1; i < len(entry.History); i++ {
+				p := entry.History[i]
+				cfg.addOrphan(name, stringValue(p.Value), p.Source)
+			}
+			delete(cfg.Settings, name)
+		} else {
+			// Remove any overrides created by the deleted package.
+			for i := 1; i < len(entry.History); /* i inc. in loop body */ {
+				if entry.History[i].Source == lpkg {
+					entry.History = append(
+						entry.History[:i], entry.History[i+1:]...)
+
+					cfg.Settings[name] = entry
+				} else {
+					i++
+				}
+			}
+		}
+	}
+}
+
 func (cfg *Cfg) settingsOfType(typ CfgSettingType) []CfgEntry {
 	entries := []CfgEntry{}
 
@@ -441,6 +553,15 @@ func (cfg *Cfg) detectViolations() {
 	}
 }
 
+// Detects all priority violations in the syscfg and records them internally.
+func (cfg *Cfg) detectPriorityViolations() {
+	for _, entry := range cfg.Settings {
+		cfg.PriorityViolations = append(
+			cfg.PriorityViolations, entry.priorityViolations()...)
+	}
+}
+
+// Detects all flash conflict errors in the syscfg and records them internally.
 func (cfg *Cfg) detectFlashConflicts(flashMap flash.FlashMap) {
 	entries := cfg.settingsOfType(CFG_SETTING_TYPE_FLASH_OWNER)
 
@@ -530,6 +651,34 @@ func (cfg *Cfg) ErrorText() string {
 
 	historyMap := map[string][]CfgPoint{}
 
+	// Redefinition errors.
+	if len(cfg.Redefines) > 0 {
+		settingNames := make([]string, len(cfg.Redefines))
+		i := 0
+		for k, _ := range cfg.Redefines {
+			settingNames[i] = k
+			i++
+		}
+		sort.Strings(settingNames)
+
+		str += "Settings defined by multiple packages:"
+		for _, n := range settingNames {
+			pkgNames := make([]string, len(cfg.Redefines[n]))
+			i := 0
+			for p, _ := range cfg.Redefines[n] {
+				pkgNames[i] = p.FullName()
+				i++
+			}
+			sort.Strings(pkgNames)
+
+			str += fmt.Sprintf("\n    %s:", n)
+			for _, pn := range pkgNames {
+				str += " " + pn
+			}
+		}
+	}
+
+	// Violation errors.
 	if len(cfg.Violations) > 0 {
 		str += "Syscfg restriction violations detected:\n"
 		for settingName, rslice := range cfg.Violations {
@@ -545,6 +694,7 @@ func (cfg *Cfg) ErrorText() string {
 		}
 	}
 
+	// Ambiguity errors.
 	if len(cfg.Ambiguities) > 0 {
 		str += "Syscfg ambiguities detected:\n"
 
@@ -561,6 +711,7 @@ func (cfg *Cfg) ErrorText() string {
 		}
 	}
 
+	// Priority violation errors.
 	if len(cfg.PriorityViolations) > 0 {
 		str += "Priority violations detected (Packages can only override " +
 			"settings defined by packages of lower priority):\n"
@@ -568,8 +719,10 @@ func (cfg *Cfg) ErrorText() string {
 			entry := cfg.Settings[priority.SettingName]
 			historyMap[priority.SettingName] = entry.History
 
-			str += fmt.Sprintf("    Package: %s overriding setting: %s defined by %s\n",
-				priority.PackageSrc.Name(), priority.SettingName, priority.PackageDef.Name())
+			str += fmt.Sprintf(
+				"    Package: %s overriding setting: %s defined by %s\n",
+				priority.PackageSrc.FullName(), priority.SettingName,
+				priority.PackageDef.FullName())
 		}
 	}
 
@@ -585,12 +738,10 @@ func (cfg *Cfg) ErrorText() string {
 		}
 	}
 
-	if str == "" {
-		return ""
+	if len(historyMap) > 0 {
+		str += "\n" + historyText(historyMap)
 	}
 
-	str += "\n" + historyText(historyMap)
-
 	return str
 }
 
@@ -615,12 +766,10 @@ func (cfg *Cfg) WarningText() string {
 		}
 	}
 
-	if str == "" {
-		return ""
+	if len(historyMap) > 0 {
+		str += "\n" + historyText(historyMap)
 	}
 
-	str += "\n" + historyText(historyMap)
-
 	return str
 }
 
@@ -756,6 +905,7 @@ func Read(lpkgs []*pkg.LocalPackage, apis []string,
 
 	cfg.detectAmbiguities()
 	cfg.detectViolations()
+	cfg.detectPriorityViolations()
 	cfg.detectFlashConflicts(flashMap)
 
 	return cfg, nil


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services