Change how contexts are merged - #811
Merged
Merged
Conversation
ekharkunov
force-pushed
the
issue-771-manifest-merge
branch
from
November 12, 2025 16:01
2e286c6 to
aaddc42
Compare
Summary - Extender code coverage reportSummary
Coveragecom/defold/extender - 26.3%
com/defold/extender/builders - 0%
com/defold/extender/cache - 35.4%
com/defold/extender/cache/info - 100%
com/defold/extender/log - 0%
com/defold/extender/metrics - 7.4%
com/defold/extender/process - 10.4%
com/defold/extender/remote - 0%
com/defold/extender/services - 24%
com/defold/extender/services/cocoapods - 50.6%
com/defold/extender/services/data - 80.7%
com/defold/extender/tracing - 0%
com/defold/extender/utils - 0%
|
ekharkunov
force-pushed
the
issue-771-manifest-merge
branch
from
November 12, 2025 17:34
fa6f191 to
148eaa3
Compare
ekharkunov
force-pushed
the
issue-771-manifest-merge
branch
from
November 12, 2025 17:36
148eaa3 to
9e575de
Compare
ekharkunov
commented
Nov 12, 2025
|
|
||
| echo "PATH=\"${APPENDED_PATH}\"" >> $OUTPUT_FILE | ||
|
|
||
| echo ";DM_DEBUG_COMMANDS=1" >> $OUTPUT_FILE |
Contributor
Author
There was a problem hiding this comment.
Unrelated changes. Just add commented lines to result user.env for future debugging.
| // because depends on that list we defined which extension to build and which symbols should be included into | ||
| // result binary | ||
| new PruneMapping("excludeSymbols", "", "symbols"), | ||
| new PruneMapping("frameworks", "includeFrameworks", "excludeFrameworks") |
Contributor
Author
There was a problem hiding this comment.
Added missed definition for frameworks
| // if applied manifest contains 'symbols' which were excluded before - need update 'excludeSymbols' list | ||
| // because depends on that list we defined which extension to build and which symbols should be included into | ||
| // result binary | ||
| new PruneMapping("excludeSymbols", "", "symbols"), |
Contributor
Author
There was a problem hiding this comment.
Added reverse mapping because we need to exclude symbols from excludeSymbols list in case if next manifest contains values in symbols list.
| ExtenderUtil.pruneItems(srcList, | ||
| ExtenderUtil.getStringList(context, mapping.includeName), | ||
| ExtenderUtil.getStringList(context, mapping.excludeName)) ); | ||
| ExtenderUtil.getStringList(b, mapping.includeName), |
Contributor
Author
There was a problem hiding this comment.
Main fix: apply prune mappings only from applied context, not from merged and accumulated context.
ekharkunov
marked this pull request as ready for review
November 12, 2025 17:44
ekharkunov
commented
Nov 12, 2025
| } | ||
| }; | ||
|
|
||
| private static List<PruneMapping> MAPPINGS = List.of( |
Contributor
Author
There was a problem hiding this comment.
Create list once instead of every call of mergeContexts
JCash
approved these changes
Nov 13, 2025
| assertFalse(excludeSymbols.contains("ProfilerExt")); | ||
| assertFalse(excludeSymbols.contains("ProfilerBasic")); | ||
| assertFalse(excludeSymbols.contains("ProfilerRemotery")); | ||
| } |
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
3 participants
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.
Fixes #771
Technical details
Change how manifests are applied. Now so-called prune mappings (lists which define what should include or exclude) are taken only from applied manifest. E.g.