From 85d67b90f7b4fd5aa0e14d2a92ed97bc9d6965ca Mon Sep 17 00:00:00 2001 From: Michal Gorecki Date: Mon, 19 Aug 2024 10:03:19 +0200 Subject: [PATCH 1/2] 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: From 083b8fe1a150d8f30201e869923c5bfa14e25321 Mon Sep 17 00:00:00 2001 From: Michal Gorecki Date: Fri, 30 Aug 2024 13:43:25 +0200 Subject: [PATCH 2/2] ci: Add ambiguous config test --- .github/targets/ambiguous_fail/pkg.yml | 5 ++ .github/targets/ambiguous_fail/syscfg.yml | 10 +++ .github/targets/ambiguous_fail/target.yml | 3 + .github/targets/ambiguous_success/pkg.yml | 5 ++ .github/targets/ambiguous_success/syscfg.yml | 10 +++ .github/targets/ambiguous_success/target.yml | 3 + .github/workflows/test_ambiguous_cfg.yml | 66 ++++++++++++++++++++ 7 files changed, 102 insertions(+) create mode 100644 .github/targets/ambiguous_fail/pkg.yml create mode 100644 .github/targets/ambiguous_fail/syscfg.yml create mode 100644 .github/targets/ambiguous_fail/target.yml create mode 100644 .github/targets/ambiguous_success/pkg.yml create mode 100644 .github/targets/ambiguous_success/syscfg.yml create mode 100644 .github/targets/ambiguous_success/target.yml create mode 100644 .github/workflows/test_ambiguous_cfg.yml diff --git a/.github/targets/ambiguous_fail/pkg.yml b/.github/targets/ambiguous_fail/pkg.yml new file mode 100644 index 000000000..dba5fd244 --- /dev/null +++ b/.github/targets/ambiguous_fail/pkg.yml @@ -0,0 +1,5 @@ +pkg.name: "targets/ambiguous_fail" +pkg.type: target +pkg.description: +pkg.author: +pkg.homepage: diff --git a/.github/targets/ambiguous_fail/syscfg.yml b/.github/targets/ambiguous_fail/syscfg.yml new file mode 100644 index 000000000..0ff26b6a2 --- /dev/null +++ b/.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 diff --git a/.github/targets/ambiguous_fail/target.yml b/.github/targets/ambiguous_fail/target.yml new file mode 100644 index 000000000..2d8502089 --- /dev/null +++ b/.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 diff --git a/.github/targets/ambiguous_success/pkg.yml b/.github/targets/ambiguous_success/pkg.yml new file mode 100644 index 000000000..f4f8c20d6 --- /dev/null +++ b/.github/targets/ambiguous_success/pkg.yml @@ -0,0 +1,5 @@ +pkg.name: "targets/ambiguous_success" +pkg.type: target +pkg.description: +pkg.author: +pkg.homepage: diff --git a/.github/targets/ambiguous_success/syscfg.yml b/.github/targets/ambiguous_success/syscfg.yml new file mode 100644 index 000000000..c9935d569 --- /dev/null +++ b/.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 diff --git a/.github/targets/ambiguous_success/target.yml b/.github/targets/ambiguous_success/target.yml new file mode 100644 index 000000000..2d8502089 --- /dev/null +++ b/.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 diff --git a/.github/workflows/test_ambiguous_cfg.yml b/.github/workflows/test_ambiguous_cfg.yml new file mode 100644 index 000000000..c2d4f5053 --- /dev/null +++ b/.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