ccollins476ad opened a new pull request #350: Don't emit spurious YCfg parse warnings
URL: https://github.com/apache/mynewt-newt/pull/350
### Background
When newt processes a YAML file, it converts it into a tree form called a YCfg. For example, this YAML excerpt:
```
OS_MAIN_STACK_SIZE: 100
OS_MAIN_STACK_SIZE.'BLE_MAX_CONNECTIONS > 3': 200
OS_MAIN_STACK_SIZE.SHELL_TASK: 300
```
Is represented as the following tree:
```
[OS_MAIN_STACK_SIZE (100)]
/ \
[BLE_DEVICE > 3 (200)] [SHELL_TASK (300)]
```
This YCfg object contains three nodes. Some of these nodes may be "inactive" in the sense that a required setting is not enabled (e.g., if `SHELL_TASK` is disabled then the rightmost node is inactive). We don't know which nodes are inactive because we don't have a list of enabled settings when the YCfg is created.
We only know which nodes are active when someone retrieves a value from the tree with a "Get" function. All the Get functions take a `settings` map parameter which allows the tree to be properly interpreted. With the list of settings available, the Get functions can identify problems in the tree. For example, if `BLE_DEVICE` is configured with a value of "foo", then the expression
```
BLE_DEVICE > 3
```
cannot be evaluated. When the Get functions spot a problem like this, they print a warning to the console and ignore the problematic node.
(Perhaps we should convert the YCfg to another "more processed" form after we have calculated the list of settings. That way we wouldn't need to do any processing in the Get functions. But that is probably a bigger discussion for another time.)
### Problem
It's a little odd to be doing so much evaluation in a Get function, but this design has worked for the most part. However, a problem arises when a condition involving a numeric comparison is applied to `pkg.deps`. For example (from `sys/log/full/pkg.yml`):
```
pkg.deps.'LOG_CONSOLE && LOG_VERSION > 2':
```
This is a problem because newt retrieves `pkg.deps` from each package before it has done syscfg resolution. The issue is that we can't know what syscfg is until we know the full list of packages included in the build (since each package defines and overrides settings). We also can't know the full list of packages until we know what syscfg is, since some dependencies are conditional on syscfg settings (as in the example above). So newt has to repeat a few steps in the process until everything has been fully resolved. But because newt doesn't know the syscfg definition the first time it encounters the above `pkg.deps` specifier, the `LOG_VERSION` setting is not defined. The comparison of an undefined setting agains `2` fails and newt prints a warning. This warning is wrong; `LOG_VERSION` is a valid setting with a numeric value, newt just doesn't know that yet.
### Solution
The solution is to:
1. Remove the warning-printing code from the YCfg Get functions.
2. Change the YCfg Get functions so that they return the parse warnings as an `error` object.
Most code that retrieves values from YCfgs just takes the returned error and prints a warning. In these cases there is no change in behavior.
During dependency resolution however, any returned errors do not get printed to the screen. Instead, these warnings are retained for later use. The previous cycle's warnings are cleared each time a new cycle starts. After resolution has completed, the retained warnings are printed. In this way, none of the printed warnings are spurious because they were reported during the final iteration.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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