Report ambiguous syscfg configuration

.github/targets/ambiguous_fail/pkg.yml

@@ -0,0 +1,5 @@
+pkg.name: "targets/ambiguous_fail"
+pkg.type: target
+pkg.description: 
+pkg.author: 
+pkg.homepage: 

.github/targets/ambiguous_fail/syscfg.yml

@@ -0,0 +1,10 @@
+syscfg.vals:
+ SHELL_PROMPT_MODULE: 1
+ SHELL_TASK: 1
+
+syscfg.vals.SHELL_PROMPT_MODULE:
+ CONSOLE_IMPLEMENTATION: stub
+
+# This should fail
+syscfg.vals.SHELL_TASK:
+ CONSOLE_IMPLEMENTATION: full

.github/targets/ambiguous_fail/target.yml

@@ -0,0 +1,3 @@
+target.app: "@apache-mynewt-nimble/apps/btshell"
+target.bsp: "@apache-mynewt-core/hw/bsp/nordic_pca10056"
+target.build_profile: optimized

.github/targets/ambiguous_success/pkg.yml

@@ -0,0 +1,5 @@
+pkg.name: "targets/ambiguous_success"
+pkg.type: target
+pkg.description: 
+pkg.author: 
+pkg.homepage: 

.github/targets/ambiguous_success/syscfg.yml

@@ -0,0 +1,10 @@
+syscfg.vals:
+ SHELL_PROMPT_MODULE: 1
+ SHELL_TASK: 1
+
+syscfg.vals.SHELL_PROMPT_MODULE:
+ CONSOLE_IMPLEMENTATION: stub
+
+# This should not fail as it sets the same value as previous conditional config
+syscfg.vals.SHELL_TASK:
+ CONSOLE_IMPLEMENTATION: stub

.github/targets/ambiguous_success/target.yml

@@ -0,0 +1,3 @@
+target.app: "@apache-mynewt-nimble/apps/btshell"
+target.bsp: "@apache-mynewt-core/hw/bsp/nordic_pca10056"
+target.build_profile: optimized

.github/workflows/test_ambiguous_cfg.yml

@@ -0,0 +1,66 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+name: Test ambiguous config
+
+on: [push, pull_request]
+
+jobs:
+ test_ambiguous_cfg:
+ name: other
+ strategy:
+ fail-fast: false
+ matrix:
+ os: [ubuntu-latest, windows-latest, macos-latest]
+ runs-on: ${{ matrix.os }}
+ steps:
+ - uses: actions/checkout@v3
+ - uses: actions/setup-go@v3
+ with:
+ go-version: 'stable'
+ - uses: carlosperate/arm-none-eabi-gcc-action@48db4484a55750df7a0ccca63347fcdea6534d78
+ with:
+ release: '12.2.Rel1'
+ - name: Install Dependencies
+ if: matrix.os == 'ubuntu-latest'
+ run: |
+ sudo apt-get update
+ sudo apt-get install -y gcc-multilib
+ - name: Build newt
+ working-directory: newt
+ shell: bash
+ run: |
+ go version
+ go build
+ echo ${GITHUB_WORKSPACE}/newt >> $GITHUB_PATH
+ - name: Test ambiguous cfg (success)
+ shell: bash
+ run: |
+ newt new project
+ cp -r .github/targets/ambiguous_success project/targets
+ cd project/
+ newt upgrade -v
+ newt build ambiguous_success
+ - name: Test ambiguous cfg (fail)
+ shell: bash
+ run: |
+ cp -r .github/targets/ambiguous_fail project/targets
+ cd project/
+ ! newt build ambiguous_fail &> tmp.txt
+ grep "Error: Setting CONSOLE_IMPLEMENTATION collision - two conditions true:" tmp.txt -q

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)

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

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: