Repository navigation
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/area-system-runtime |
Added a HashSet for all Apple platform names and updated tests to verify Apple platform detection.
Add Ref and tests for public IsApplePlatform API
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
IsApplePlatforms publicly in OperatingSystemIsApplePlatform publicly in OperatingSystem
|
I assume we are going to need a corresponding pull request from someone for the compatibility analyzer? I don't know if the analyzer will handle this without changes. https://github.com/dotnet/dotnet/blob/4fd6fbb479abbacd43b32fdc0ef6f68414cc028d/src/sdk/src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs |
…Pal.Unix.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>
|
|
I think it was a fine change - for consistency. We do not currently build and test for watch OS, but the code for it still there. |
It looks odd when one place in the same file assumes that watchOS does not exist, but other place treats it as a thing. |
Yeah, exactly. That’s why I initially added watchOS. However, after thinking about @vcsjones 's comment on the compatibility analyzer, I decided to remove Let's assume the platform compatibility analyzer supports I wanted to avoid this situation. After all, new APIs won't explicitly include |
Sure, I'll follow up on this, thanks! |
I do not think that this has any impact on platform compatibility analyzer user experience. The platform compatibility analyzer only checks compatibly for platforms specified via |
@jkotas Thanks for the clear explanation! I hadn’t realized that the analyzer’s platform checks are scoped by Given that, how would you prefer to handle watchOS in |
Option 1: Add TARGET_WATCHOS to the ifdef in IsApplePlatform Option 2: Delete TARGET_WATCHOS ifdef in IsWatchOS() and replace with a comment that says watchOS is no longer supported. Also, delete watchOS references in src\libraries\System.Private.CoreLib\src\System\Runtime\InteropServices\RuntimeInformation.Unix.cs, src\libraries\System.Runtime\tests\System.Runtime.Extensions.Tests\System\OperatingSystemTests.cs, src\native\eventpipe\ep-event-source.c, src\native\libs\System.Native\pal_time.c, src\native\libs\System.Security.Cryptography.Native.Apple\pal_ssl.c. Pick the one you like better - either one is fine with me. |
I prefer option 2 - to do a deeper cleanup of watchOS. |
IsApplePlatform publicly in OperatingSystemOperatingSystem.IsApplePlatform publicly and Clean up watchOS
Closes #113262.
As discussed in #113262 and the video, we're good to make this helper exposed publicly in
OperatingSystem.cc @bartonjs
This pull request refactors platform checks throughout the codebase to consolidate Apple-related OS checks into a single method,
OperatingSystem.IsApplePlatform(). It also removes support for watchOS, updating both implementation and tests accordingly. This streamlines platform detection logic, reduces duplication, and clarifies current platform support.Platform detection refactoring:
Introduced and made public
OperatingSystem.IsApplePlatform(), replacing multiple checks for individual Apple platforms (IsMacOS,IsIOS,IsTvOS,IsMacCatalyst) in both product and test code. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Updated comments and documentation to reflect the consolidated Apple platform detection, and removed references to watchOS where appropriate. [1] [2] [3] [4]
watchOS deprecation/removal:
IsWatchOSnow always returns false and is documented as unsupported), in version checks, and in native code. [1] [2] [3] [4]Test updates:
IsApplePlatform()method and removed watchOS from test cases and assertions. [1] [2] [3]These changes improve maintainability by centralizing Apple platform logic and clarify that watchOS is no longer a supported target.