add compatible field to configs#1812
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1812 +/- ##
==========================================
+ Coverage 72.18% 72.48% +0.29%
==========================================
Files 52 52
Lines 27744 27782 +38
Branches 6002 6002
==========================================
+ Hits 20028 20138 +110
+ Misses 7716 7644 -72
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a new compatible field to architecture configuration YAMLs to express (and validate) mutual satisfiability with other configs, including transitive compatibility chains.
Changes:
- Extend the config JSON schema to allow
compatibleas a string or list of strings. - Add
compatibleaccessor onUdb::Configand implement compatibility validation inConfiguredArchitecture#valid?. - Add unit tests covering basic compatible/incompatible and transitive compatible scenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/ruby-gems/udb/test/test_cfg_arch.rb | Adds tests for compatibility validation behavior. |
| tools/ruby-gems/udb/lib/udb/resolver.rb | Introduces pointer resolution helper for config-name vs file-path references. |
| tools/ruby-gems/udb/lib/udb/config.rb | Exposes compatible field from parsed config data. |
| tools/ruby-gems/udb/lib/udb/cfg_arch.rb | Validates compatible pointers (including transitive) during config validation. |
| spec/schemas/config_schema.json | Allows compatible in config schema (string or array of strings). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ThinkOpenly
left a comment
ThinkOpenly
left a comment
There was a problem hiding this comment.
Looks like Copilot has a point or two for consideration.
I'd love to see a few more words in the PR description. :-)
Otherwise, LGTM.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def resolve_compatible_pointer(pointer) | ||
| @config.info.resolver.cfg_arch_for_pointer( | ||
| pointer, | ||
| relative_dir: Pathname.new(@config.info.path).dirname |
| # Use dup per top-level pointer so sibling pointers each get a fresh visited set — | ||
| # each branch independently validates against any shared transitive targets. Cycle | ||
| # detection within a single chain is still enforced because visited mutates in-place | ||
| # during the recursive descent. | ||
| visited = T.let(Set.new([name]), T::Set[String]) | ||
| Array(@config.compatible).each do |pointer| | ||
| begin |
| assert result.valid, | ||
| "Expected valid when compatible pointer is the known config name 'rv64', got: #{result.reasons}" | ||
| end | ||
|
|
No description provided.