Fix #767: drop duplicates when merging '..._append' lists#778
Open
phverg wants to merge 1 commit intoolofk:mainfrom
Open
Fix #767: drop duplicates when merging '..._append' lists#778phverg wants to merge 1 commit intoolofk:mainfrom
phverg wants to merge 1 commit intoolofk:mainfrom
Conversation
When YAML-merge target inheritance left a target with both a base
list and an ``..._append`` list pulling in items already in the base
list (or with duplicates inside ``..._append`` itself), the merged
list kept the duplicates. Worked example::
sim_base: &sim_base
filesets_append: [files_dv]
...
unit_test:
<<: *sim_base
filesets_append: [files_dv]
YAML merge concatenates the two ``filesets_append`` lists, so
``files_dv`` ended up listed twice on ``unit_test``. Every file in
``files_dv`` then appeared twice in the emitted EDAM, and the EDA
backend rejected the build with a ``file already defined`` error.
The natural inheritance pattern was effectively unusable.
Deduplicate when extending the base list inside
``CoreData._append_lists``. Items unique to ``..._append`` are still
appended in order; items already present (either via the base list
or earlier in the same ``..._append``) are dropped. This keeps the
existing append-only and base-only test cases unchanged.
Fixes olofk#767.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
|
I understand what this fixes, but thinking about it I can't see how on earth we would get into this situation. The provided example kind of undermines the whole idea of inheritance as the point is that you would not have to add the fileset in the target that inherits. I realize your example is intentionally contrived, but could you (or someone else) come up with a more realistic situation where this could happen? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #767.
A natural inheritance pattern using YAML merge plus
..._append:…leaves
unit_testwithfilesets_append: [files_dv, files_dv]afterYAML's list merge. Every file in
files_dvthen ends up listed twice inthe emitted EDAM, and the EDA backend rejects the build with a
file already definederror. The same happens forfiles_appendre-listing a file already present in
files. The natural inheritancepattern is effectively unusable.
Solution
Deduplicate when extending the base list inside
CoreData._append_lists. Items that are already present (in the baselist or earlier in the same
..._append) are dropped; new items arestill appended in order. This keeps the existing append-only and
base-only test cases unchanged.
Test plan
tests/capi2_cores/misc/append.corewith three newtargets covering the duplicate cases (in-base, within-append,
append-only).
test_capi2_appendto assert duplicates are dropped andorder preserved; existing assertions still pass.
filesets_append): EDAM file list now contains each file once.pytestsuite green locally.pre-commit run -aclean.