[tools] Simplify the creation of the list of frameworks a bit.#25618
[tools] Simplify the creation of the list of frameworks a bit.#25618rolfbjarne wants to merge 8 commits into
Conversation
We don't need to know whether we're building for a simulator or device when creating the list of frameworks, because now it's possible to specify different values for simulator vs device. This makes it possible to simplify the code a little bit. Most of this is indentation changes, so it's easier to review if hiding whitespace changes.
There was a problem hiding this comment.
Pull request overview
This PR refactors framework list retrieval in the tooling code, aiming to remove the need to know whether the current build is for simulator vs device when constructing the frameworks list (by encoding simulator/device availability per framework entry instead).
Changes:
- Replaced the iOS frameworks getter method with a lazily-initialized
iOSFrameworksproperty. - Introduced
TryGetFrameworks (ApplePlatform, out Frameworks)and updated a few internal callers to use it. - Refactored Mac Catalyst framework initialization to build from the iOS list and then apply Catalyst-specific adjustments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/common/Frameworks.cs | Refactors iOS + Mac Catalyst framework lists and replaces GetFrameworks with a TryGetFrameworks pattern. |
| tools/common/Driver.cs | Updates framework retrieval to use Frameworks.TryGetFrameworks. |
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| catalyst_frameworks = iOSFrameworks; | ||
| // We're going to mutate the value returned from iOSFrameworks, so clear the cached value so the next time iOSFrameworks is called it's re-generated. | ||
| ios_frameworks = null; | ||
| // not present in iOS but present in catalyst | ||
| catalyst_frameworks.Add ("CoreWlan", "CoreWLAN", 15, 0); |
| public static bool TryGetFrameworks (ApplePlatform platform, [NotNullWhen (true)] out Frameworks? frameworks) | ||
| { | ||
| switch (platform) { | ||
| case ApplePlatform.iOS: | ||
| return GetiOSFrameworks (is_simulator_build); | ||
| frameworks = iOSFrameworks; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
✅ [PR Build #58a293a] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #58a293a] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [PR Build #58a293a] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build #58a293a] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 6 tests failed, 187 tests passed. Failures❌ introspection tests [attempt 2]4 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (macOS) [attempt 2]2 tests failed, 21 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
We don't need to know whether we're building for a simulator or device when
creating the list of frameworks, because now it's possible to specify
different values for simulator vs device.
This makes it possible to simplify the code a little bit.
Most of this is indentation changes, so it's easier to review if hiding whitespace changes.