Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure the setup config load order #818

Conversation

tbates-redarc
Copy link
Contributor

@tbates-redarc tbates-redarc commented Oct 4, 2023

Refactor Configuration Load Order

Description

Improve the order in which the default configs and user-defined configs are loaded and merged in setupinator.

Fixes #536, with thanks to my colleague @chuyingy

Previous structure

Previously, the config load order and result priority were as follows:

Previous config load procedure:

  1. Merge project.yml with DEFAULT_CEEDLING_CONFIG
  2. Populate with DEFAULT_TOOLS_*, CMock, and Unity settings.
  3. Merge with plugin settings.
  4. Merge with plugin defaults.

Resulting priority order (highest first):

  • project.yml
  • DEFAULT_CEEDLING_CONFIG
  • DEFAULT_TOOLS_*, CMock, and Unity
  • plugin settings
  • plugin defaults

This would result in some default settings having a higher priority than plugin settings. It also made the differences in behaviour between plugin settings and plugin defaults confusing as they were almost but not quite the same.

Proposed changes

The proposed change will first merge the user-defined configs and default configs separately, and then combine them. This approach ensures all user-defined configurations take precedence over any default settings.

CMock and Ceedling settings are divided into internal configuration, and defaults. The internal config section contains the cmock plugin and include settings, the vendor and plugin paths, etc. as we felt it was unlikely that the user would intend to replace these in project.yml but rather add to them.

New config loading procedure:

  1. Populate project.yml with Ceedling and CMock internal settings, then merge with plugins --> config
  2. Merge DEFAULT_CEEDLING_CONFIG with DEFAULT_TOOLS_*, CMock and Unity defaults, and plugin defaults --> defaults
  3. Populate config with defaults

New priority:

config:

  • project.yml
  • CMock and Ceedling internal configuration
  • plugin settings

defaults:

  • plugin defaults
  • DEFAULT_CEEDLING_CONFIG, DEFAULT_TOOLS_*,
  • CMock and Unity defaults

All config parameters take precedence over and replace all defaults, i.e. are not deep_merged with them. This allows plugin to have the choice to add to the config as if specified by the user in project.yml, or add to the defaults but still have those values be able to be replaced by the user or another plugin.

Impact

  • Settings provided in project.yml or plugin config will replace those in DEFAULT_CEEDLING_CONFIG, DEFAULT_TOOLS_* etc instead of being merged with them.
    • Most of the defaults are either atoms that can't be merged, or empty lists anyway. Those that were not have been moved to 'internal configuration' which is merged.
  • Settings provided in plugin defaults will be merged with (if mergeable) or replace (if not) Ceedling and other plugin defaults instead of being ignored in favour of the Ceedling or other plugin defaults

@tbates-redarc tbates-redarc changed the base branch from master to test/ceedling_0_32_rc December 1, 2023 07:11
Change the structure to merge both the default configs and the user-defined configs separately. Then combine the two sets of merged configs.
@tbates-redarc tbates-redarc force-pushed the change_setup_config_load_order branch from 588c2e1 to 576568f Compare March 4, 2024 05:34
mkarlesky added a commit that referenced this pull request Jun 19, 2024
- Implemented the logic and ordering of PR #818. To oversimplify, user configuration is assembled first and then all defaults are applied. A handful of methods have been broken into two renamed methods, one for default handling and another for user configuration handling.
- Added lots of comments and updated `Configurator` method names to use _populate_, _merge_, etc. consistently.
- Refactored `TestRunnerManager` to move configuration validation into Configurator & friends
- Renamed Backtrace `gdb` tool definition to match other tools and added support for the tool argument shortcut in project configuration.
@mkarlesky
Copy link
Member

@tbates-redarc & @chuyingy: I am happy to report that your hard work is now part of the latest Ceedling prerelease. The names of the methods and new internal hash are different than in your PR, as is how the code is segmented. But, all your ideas and code are present. This effort is the last major backlog item before publishing the next major Ceedling release (a handful of smaller items remain).

We were not able to merge this PR because the underlying project had diverged too greatly in the intervening months since your PR. So, I will close this PR rarther than merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix plugin config load order
3 participants