Hopefully final PR before v1.1 #669
Merged
Conversation
This file contains 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
Clarify some attribute contents and reorganize so related attributes go together. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Use a deque instead of a list when accumulating group filters during imports. This allows us to preserve the boundaries of each filter list, which in turn allows us to improve the debug output. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This reverts commit d0e6b9a. West was right, the docs are wrong/inconsistent. The commit I'm reverting was following the west documentation it referenced correctly, but the documentation itself was wrong (and in fact was inconsistent with earlier documentation on the same page). The basis for making commit d0e6b9a was that the docs currently say: In other words, let: - the submanifest resolved from self-import have group filter self-filter - the top-level manifest file have group filter top-filter - the submanifests resolved from import-1 through import-N have group filters filter-1 through filter-N respectively The final resolved group-filter value is then filter1 + filter-2 + ... + filter-N + top-filter + self-filter, where + here refers to list concatenation. But earlier in the same section, they also say: The resolved manifest has a group-filter value which is the result of concatenating the group-filter values in the top-level manifest and any imported manifests. Manifest files which appear earlier in the import order have higher precedence and are therefore concatenated later into the final group-filter. Where the "import order" is defined higher up as: Importing is done in this order: 1. Manifests from self-import are imported first. 2. The top-level manifest file’s definitions are handled next. 3. Manifests from import-1, …, import-N, are imported in that order. This means that import-1 happens **earlier** than import-N, and should therefore have **higher** precedence when it comes to computing the final group filter. But putting import-N **later** in the concanated group filter actually means import-1 has **lower** precedence. So the docs are inconsistent. To fix the docs so they are consistent and match west's current behavior, we should replace this: The final resolved group-filter value is then filter1 + filter-2 + ... + filter-N + top-filter + self-filter, where + here refers to list concatenation. with this: The final resolved group-filter value is then filter-N + ... + filter-1 + top-filter + self-filter, where + here refers to list concatenation. That is: - docs currently go from: filter-1 to filter-N - they should go from: filter-N to filter-1. Fixes: zephyrproject-rtos#663 Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The top level comment is not totally accurate. The 'project_importer' attribute, for example, doesn't change. The comment around 'project_filter' reflects an idea I had but abandoned so that project filter behavior would be more consistent with group filter behavior. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Make this option consistent with all others by only having the highest precedence configuration option take effect. We revisited this decision following review. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Clarify the user's options a bit more. Requested-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no> Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
carlescufi
approved these changes
Jun 2, 2023
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.
See individual commit messages for details
Fixes: #663
Fixes: #668