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/08 18:13:17 UTC

[GitHub] ccollins476ad closed pull request #158: Allow package-level syscfg restrictions

ccollins476ad closed pull request #158: Allow package-level syscfg restrictions
URL: https://github.com/apache/mynewt-newt/pull/158
 
 
   

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/builder/buildpackage.go b/newt/builder/buildpackage.go
index ef3b0031..75330e67 100644
--- a/newt/builder/buildpackage.go
+++ b/newt/builder/buildpackage.go
@@ -128,7 +128,7 @@ func (bpkg *BuildPackage) CompilerInfo(
 	}
 
 	ci := toolchain.NewCompilerInfo()
-	settings := b.cfg.SettingsForLpkg(bpkg.rpkg.Lpkg)
+	settings := b.cfg.AllSettingsForLpkg(bpkg.rpkg.Lpkg)
 
 	// Read each set of flags and expand repo designators ("@<repo-nme>") into
 	// paths.
diff --git a/newt/resolve/resolve.go b/newt/resolve/resolve.go
index 9d329519..a7c45ccc 100644
--- a/newt/resolve/resolve.go
+++ b/newt/resolve/resolve.go
@@ -259,7 +259,7 @@ func (r *Resolver) selectApiSuppliers() {
 	apiMap := map[string][]resolveApi{}
 
 	for _, rpkg := range r.sortedRpkgs() {
-		settings := r.cfg.SettingsForLpkg(rpkg.Lpkg)
+		settings := r.cfg.AllSettingsForLpkg(rpkg.Lpkg)
 		apiStrings := rpkg.Lpkg.PkgY.GetSlice("pkg.apis", settings)
 		for _, entry := range apiStrings {
 			apiStr, ok := entry.Value.(string)
@@ -297,7 +297,7 @@ func (r *Resolver) selectApiSuppliers() {
 
 // Populates the specified package's set of API requirements.
 func (r *Resolver) calcApiReqsFor(rpkg *ResolvePackage) {
-	settings := r.cfg.SettingsForLpkg(rpkg.Lpkg)
+	settings := r.cfg.AllSettingsForLpkg(rpkg.Lpkg)
 
 	reqApiEntries := rpkg.Lpkg.PkgY.GetSlice("pkg.req_apis", settings)
 	for _, entry := range reqApiEntries {
@@ -323,7 +323,7 @@ func (r *Resolver) calcApiReqs() {
 //                                  in this case.
 //         error                non-nil on failure.
 func (r *Resolver) loadDepsForPkg(rpkg *ResolvePackage) (bool, error) {
-	settings := r.cfg.SettingsForLpkg(rpkg.Lpkg)
+	settings := r.cfg.AllSettingsForLpkg(rpkg.Lpkg)
 
 	changed := false
 
diff --git a/newt/syscfg/restrict.go b/newt/syscfg/restrict.go
index d78e5221..c998fce5 100644
--- a/newt/syscfg/restrict.go
+++ b/newt/syscfg/restrict.go
@@ -150,8 +150,8 @@ func normalizeExpr(expr string, baseSetting string) string {
 	return expr
 }
 
-func (cfg *Cfg) violationText(entry CfgEntry, r CfgRestriction) string {
-	prefix := fmt.Sprintf("%s(%s) ", entry.Name, entry.Value)
+func (cfg *Cfg) settingViolationText(entry CfgEntry, r CfgRestriction) string {
+	prefix := fmt.Sprintf("Setting %s(%s) ", entry.Name, entry.Value)
 	if r.Code == CFG_RESTRICTION_CODE_NOTNULL {
 		return prefix + "must not be null"
 	} else {
@@ -159,8 +159,16 @@ func (cfg *Cfg) violationText(entry CfgEntry, r CfgRestriction) string {
 	}
 }
 
+func (cfg *Cfg) packageViolationText(pkgName string, r CfgRestriction) string {
+	return fmt.Sprintf("Package %s requires: %s", pkgName, r.Expr)
+}
+
 func (r *CfgRestriction) relevantSettingNames() []string {
-	names := []string{r.BaseSetting}
+	var names []string
+
+	if r.BaseSetting != "" {
+		names = append(names, r.BaseSetting)
+	}
 
 	if r.Code == CFG_RESTRICTION_CODE_EXPR {
 		tokens, _ := parse.Lex(normalizeExpr(r.Expr, r.BaseSetting))
@@ -184,7 +192,13 @@ func (cfg *Cfg) restrictionMet(
 		return baseEntry.Value != ""
 
 	case CFG_RESTRICTION_CODE_EXPR:
-		expr := normalizeExpr(r.Expr, r.BaseSetting)
+		var expr string
+		if r.BaseSetting != "" {
+			expr = normalizeExpr(r.Expr, r.BaseSetting)
+		} else {
+			expr = r.Expr
+		}
+
 		val, err := parse.ParseAndEval(expr, settings)
 		if err != nil {
 			util.StatusMessage(util.VERBOSITY_QUIET,
diff --git a/newt/syscfg/syscfg.go b/newt/syscfg/syscfg.go
index 8f5fb193..81e38155 100644
--- a/newt/syscfg/syscfg.go
+++ b/newt/syscfg/syscfg.go
@@ -101,6 +101,9 @@ type CfgFlashConflict struct {
 type Cfg struct {
 	Settings map[string]CfgEntry
 
+	// Restrictions at the package level (i.e. "syscfg.restrictions").
+	PackageRestrictions map[string][]CfgRestriction
+
 	//// Errors
 	// Overrides of undefined settings.
 	Orphans map[string][]CfgPoint
@@ -109,12 +112,16 @@ type Cfg struct {
 	// values; not overridden by higher priority package.
 	Ambiguities map[string][]CfgPoint
 
-	// Setting restrictions not met.
-	Violations map[string][]CfgRestriction
+	// Setting-level restrictions not met.
+	SettingViolations map[string][]CfgRestriction
+
+	// Package-level restrictions not met.
+	PackageViolations map[string][]CfgRestriction
 
 	// Attempted override by bottom-priority packages (libraries).
 	PriorityViolations []CfgPriority
 
+	// Two or more flash areas overlap.
 	FlashConflicts []CfgFlashConflict
 
 	// Multiple packages defining the same setting.
@@ -124,13 +131,15 @@ type Cfg struct {
 
 func NewCfg() Cfg {
 	return Cfg{
-		Settings:           map[string]CfgEntry{},
-		Orphans:            map[string][]CfgPoint{},
-		Ambiguities:        map[string][]CfgPoint{},
-		Violations:         map[string][]CfgRestriction{},
-		PriorityViolations: []CfgPriority{},
-		FlashConflicts:     []CfgFlashConflict{},
-		Redefines:          map[string]map[*pkg.LocalPackage]struct{}{},
+		Settings:            map[string]CfgEntry{},
+		PackageRestrictions: map[string][]CfgRestriction{},
+		Orphans:             map[string][]CfgPoint{},
+		Ambiguities:         map[string][]CfgPoint{},
+		SettingViolations:   map[string][]CfgRestriction{},
+		PackageViolations:   map[string][]CfgRestriction{},
+		PriorityViolations:  []CfgPriority{},
+		FlashConflicts:      []CfgFlashConflict{},
+		Redefines:           map[string]map[*pkg.LocalPackage]struct{}{},
 	}
 }
 
@@ -143,8 +152,25 @@ func (cfg *Cfg) SettingValues() map[string]string {
 	return values
 }
 
-func (cfg *Cfg) SettingsForLpkg(lpkg *pkg.LocalPackage) map[string]string {
-	settings := cfg.SettingValues()
+// If the specified package has any injected settings, returns a new map
+// consisting of the union of the injected settings and the provided base
+// settings.
+//
+// Else (no injected settings), returns the base settings unmodified.
+func (cfg *Cfg) settingsForLpkg(lpkg *pkg.LocalPackage,
+	baseSettings map[string]string) map[string]string {
+
+	var settings map[string]string
+
+	injected := lpkg.InjectedSettings()
+	if len(injected) == 0 {
+		return baseSettings
+	}
+
+	settings = make(map[string]string, len(baseSettings))
+	for k, v := range baseSettings {
+		settings[k] = v
+	}
 
 	for k, v := range lpkg.InjectedSettings() {
 		_, ok := settings[k]
@@ -159,6 +185,10 @@ func (cfg *Cfg) SettingsForLpkg(lpkg *pkg.LocalPackage) map[string]string {
 	return settings
 }
 
+func (cfg *Cfg) AllSettingsForLpkg(lpkg *pkg.LocalPackage) map[string]string {
+	return cfg.settingsForLpkg(lpkg, cfg.SettingValues())
+}
+
 func (point CfgPoint) Name() string {
 	if point.Source == nil {
 		return "newt"
@@ -361,15 +391,7 @@ func (cfg *Cfg) readDefsOnce(lpkg *pkg.LocalPackage,
 	settings map[string]string) error {
 	yc := lpkg.SyscfgY
 
-	lsettings := cfg.SettingsForLpkg(lpkg)
-	for k, v := range settings {
-		lsettings[k] = v
-	}
-	for k, _ := range lsettings {
-		if _, ok := settings[k]; ok == false {
-			delete(lsettings, k)
-		}
-	}
+	lsettings := cfg.settingsForLpkg(lpkg, settings)
 
 	defs := yc.GetValStringMap("syscfg.defs", lsettings)
 	if defs != nil {
@@ -421,20 +443,31 @@ func (cfg *Cfg) addOrphan(settingName string, value string,
 	})
 }
 
-func (cfg *Cfg) readValsOnce(lpkg *pkg.LocalPackage,
+func (cfg *Cfg) readRestrictions(lpkg *pkg.LocalPackage,
 	settings map[string]string) error {
+
 	yc := lpkg.SyscfgY
+	lsettings := cfg.settingsForLpkg(lpkg, settings)
 
-	lsettings := cfg.SettingsForLpkg(lpkg)
-	for k, v := range settings {
-		lsettings[k] = v
-	}
-	for k, _ := range lsettings {
-		if _, ok := settings[k]; ok == false {
-			delete(lsettings, k)
+	restrictionStrings := yc.GetValStringSlice("syscfg.restrictions", lsettings)
+	for _, rstring := range restrictionStrings {
+		r, err := readRestriction("", rstring)
+		if err != nil {
+			return util.PreNewtError(err, "error parsing restriction: %s", rstring)
 		}
+		cfg.PackageRestrictions[lpkg.Name()] =
+			append(cfg.PackageRestrictions[lpkg.Name()], r)
 	}
 
+	return nil
+}
+
+func (cfg *Cfg) readValsOnce(lpkg *pkg.LocalPackage,
+	settings map[string]string) error {
+	yc := lpkg.SyscfgY
+
+	lsettings := cfg.settingsForLpkg(lpkg, settings)
+
 	values := yc.GetValStringMap("syscfg.vals", lsettings)
 	for k, v := range values {
 		entry, ok := cfg.Settings[k]
@@ -540,15 +573,34 @@ func (cfg *Cfg) settingsOfType(typ CfgSettingType) []CfgEntry {
 func (cfg *Cfg) detectViolations() {
 	settings := cfg.SettingValues()
 	for _, entry := range cfg.Settings {
-		var ev []CfgRestriction
+		var sv []CfgRestriction
 		for _, r := range entry.Restrictions {
 			if !cfg.restrictionMet(r, settings) {
-				ev = append(ev, r)
+				sv = append(sv, r)
+			}
+		}
+
+		if sv != nil {
+			cfg.SettingViolations[entry.Name] = sv
+		}
+	}
+
+	pkgNames := make([]string, 0, len(cfg.PackageRestrictions))
+	for n, _ := range cfg.PackageRestrictions {
+		pkgNames = append(pkgNames, n)
+	}
+	sort.Strings(pkgNames)
+
+	for _, n := range pkgNames {
+		var pv []CfgRestriction
+		for _, r := range cfg.PackageRestrictions[n] {
+			if !cfg.restrictionMet(r, settings) {
+				pv = append(pv, r)
 			}
 		}
 
-		if ev != nil {
-			cfg.Violations[entry.Name] = ev
+		if pv != nil {
+			cfg.PackageViolations[n] = pv
 		}
 	}
 }
@@ -679,9 +731,9 @@ func (cfg *Cfg) ErrorText() string {
 	}
 
 	// Violation errors.
-	if len(cfg.Violations) > 0 {
+	if len(cfg.SettingViolations) > 0 || len(cfg.PackageViolations) > 0 {
 		str += "Syscfg restriction violations detected:\n"
-		for settingName, rslice := range cfg.Violations {
+		for settingName, rslice := range cfg.SettingViolations {
 			baseEntry := cfg.Settings[settingName]
 			historyMap[settingName] = baseEntry.History
 			for _, r := range rslice {
@@ -689,7 +741,16 @@ func (cfg *Cfg) ErrorText() string {
 					reqEntry := cfg.Settings[name]
 					historyMap[name] = reqEntry.History
 				}
-				str += "    " + cfg.violationText(baseEntry, r) + "\n"
+				str += "    " + cfg.settingViolationText(baseEntry, r) + "\n"
+			}
+		}
+		for pkgName, rslice := range cfg.PackageViolations {
+			for _, r := range rslice {
+				for _, name := range r.relevantSettingNames() {
+					reqEntry := cfg.Settings[name]
+					historyMap[name] = reqEntry.History
+				}
+				str += "    " + cfg.packageViolationText(pkgName, r) + "\n"
 			}
 		}
 	}
@@ -903,6 +964,12 @@ func Read(lpkgs []*pkg.LocalPackage, apis []string,
 		}
 	}
 
+	for _, lpkg := range lpkgs {
+		if err := cfg.readRestrictions(lpkg, settings); err != nil {
+			return cfg, err
+		}
+	}
+
 	cfg.detectAmbiguities()
 	cfg.detectViolations()
 	cfg.detectPriorityViolations()


 

----------------------------------------------------------------
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