From 85d67b90f7b4fd5aa0e14d2a92ed97bc9d6965ca Mon Sep 17 00:00:00 2001
From: Michal Gorecki <michal.gorecki@codecoup.pl>
Date: Mon, 19 Aug 2024 10:03:19 +0200
Subject: [PATCH] Report ambiguous syscfg configuration

This addresses: https://github.com/apache/mynewt-newt/issues/565
---
 newt/cli/target_cmds.go |  4 +++-
 newt/syscfg/syscfg.go   |  4 +++-
 newt/ycfg/ycfg.go       | 21 ++++++++++++++++++---
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/newt/cli/target_cmds.go b/newt/cli/target_cmds.go
index b3a13e543..75ed4e06e 100644
--- a/newt/cli/target_cmds.go
+++ b/newt/cli/target_cmds.go
@@ -97,7 +97,9 @@ func pkgVarSliceString(pack *pkg.LocalPackage, key string) string {
 func amendSysCfg(value string, t *target.Target) error {
 	// Get the current syscfg.vals name-value pairs
 	sysVals, err := t.Package().SyscfgY.GetValStringMapString("syscfg.vals", nil)
-	util.OneTimeWarningError(err)
+	if err != nil {
+		return err
+	}
 
 	// Convert the input syscfg into name-value pairs
 	amendSysVals, err := syscfg.KeyValueFromStr(value)
diff --git a/newt/syscfg/syscfg.go b/newt/syscfg/syscfg.go
index e7f62ff09..886699cca 100644
--- a/newt/syscfg/syscfg.go
+++ b/newt/syscfg/syscfg.go
@@ -708,7 +708,9 @@ func (cfg *Cfg) readValsOnce(lpkg *pkg.LocalPackage,
 	lsettings := cfg.settingsForLpkg(lpkg, settings)
 
 	values, err := yc.GetValStringMap("syscfg.vals", lsettings)
-	util.OneTimeWarningError(err)
+	if err != nil {
+		return err
+	}
 
 	for k, v := range values {
 		switch v.(type) {
diff --git a/newt/ycfg/ycfg.go b/newt/ycfg/ycfg.go
index 4ae8f7ca9..08d334771 100644
--- a/newt/ycfg/ycfg.go
+++ b/newt/ycfg/ycfg.go
@@ -469,7 +469,13 @@ func (yc *YCfg) GetStringMap(
 				Expr:  mapEntry.Expr,
 			}
 
-			// XXX: Report collisions?
+			if _, exists := result[k]; exists {
+				if (entry.Value != result[k].Value) && (result[k].Expr != nil) {
+					return nil, fmt.Errorf("Setting %s collision - two conditions true:\n[%s, %s]\n"+
+						"Conflicting file: %s",
+						k, entry.Expr.String(), result[k].Expr.String(), yc.name)
+				}
+			}
 			result[k] = entry
 		}
 	}
@@ -607,7 +613,13 @@ func (yc *YCfg) GetStringMapString(key string,
 				Expr:  mapEntry.Expr,
 			}
 
-			// XXX: Report collisions?
+			if _, exists := result[k]; exists {
+				if (entry.Value != result[k].Value) && (result[k].Expr != nil) {
+					return nil, fmt.Errorf("Setting %s collision - two conditions true:\n[%s, %s]\n"+
+						"Conflicting file: %s",
+						k, entry.Expr.String(), result[k].Expr.String(), yc.name)
+				}
+			}
 			result[k] = entry
 		}
 	}
@@ -623,6 +635,9 @@ func (yc *YCfg) GetValStringMapString(key string,
 	settings *cfgv.Settings) (map[string]string, error) {
 
 	entryMap, getErr := yc.GetStringMapString(key, settings)
+	if getErr != nil {
+		return nil, getErr
+	}
 
 	valMap := make(map[string]string, len(entryMap))
 	for k, v := range entryMap {
@@ -631,7 +646,7 @@ func (yc *YCfg) GetValStringMapString(key string,
 		}
 	}
 
-	return valMap, getErr
+	return valMap, nil
 }
 
 // FullName calculates a node's name with the following form: