-
Notifications
You must be signed in to change notification settings - Fork 330
Consolidate OS detection to OsConstants #4255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
paulmedynski
wants to merge
4
commits into
main
Choose a base branch
from
dev/paul/os-constants
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+132
−44
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
4c08ef5
Consolidate OS detection to OsConstants
paulmedynski 8f5e2d0
Address Copilot OS guard feedback
paulmedynski 49c3f72
Expose OsConstants flags as intrinsic properties for IL trimming
paulmedynski 6d509a8
Use OperatingSystem.Is*() on .NET for JIT-intrinsic OS checks
paulmedynski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
107 changes: 107 additions & 0 deletions
107
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/OsConstants.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; | ||
| using System.Runtime.InteropServices; | ||
|
|
||
| namespace Microsoft.Data.SqlClient; | ||
|
|
||
| /// <summary> | ||
| /// Provides platform detection flags for OS-specific code paths. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This type exists to keep OS detection terse and in one place: callers write | ||
| /// <c>OsConstants.IsWindows</c> instead of repeating <c>RuntimeInformation.IsOSPlatform(...)</c> | ||
| /// throughout the codebase. Centralizing it also gives us a single seam to adjust if a specific | ||
| /// OS needs special handling (for example, a different detection mechanism, a finer-grained | ||
| /// flag, or an override for testing) without touching every call site. | ||
| /// </remarks> | ||
| /// <remarks> | ||
| /// Each flag is exposed as a property that directly returns an OS check. On .NET, that check is | ||
| /// <c>OperatingSystem.Is*()</c>, which the JIT treats as an intrinsic and constant-folds (and which | ||
| /// the IL trimmer also recognizes). On .NET Framework — where the <see cref="System.OperatingSystem"/> | ||
| /// platform-check helpers do not exist — the check falls back to | ||
| /// <see cref="RuntimeInformation.IsOSPlatform"/>. Either way, returning the check directly is | ||
| /// deliberate: caching the results in static fields (for example, via a static constructor) would | ||
| /// defeat both the JIT folding and the trimmer — the trimmer cannot analyze a static constructor, | ||
| /// so it would be unable to prove which branches are dead and would keep OS-specific code (including | ||
| /// Windows-only native dependencies) in apps published for other platforms. | ||
|
paulmedynski marked this conversation as resolved.
|
||
| /// </remarks> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// IL trimming note: when a downstream app is published for a specific OS (a RID-specific publish), | ||
| /// the IL trimmer substitutes the underlying <see cref="RuntimeInformation.IsOSPlatform"/> / | ||
| /// <c>OperatingSystem.Is*</c> checks with constant <c>true</c>/<c>false</c> for the target OS and | ||
| /// then removes the dead branch (along with everything it transitively references, such as | ||
| /// Windows-only native entry points). This is what lets a single cross-platform assembly trim | ||
| /// away the OS-specific code that does not apply to the published target. | ||
| /// </para> | ||
| /// <para> | ||
| /// For this to work, each guard must be used <b>inline</b> in the same method as the OS-specific | ||
| /// code it gates. The trimmer's constant folding is shallow: it reasons within the method that | ||
| /// contains the guard, so a guard hidden behind a helper traps the constant inside that helper and | ||
| /// leaves the protected code reachable (and therefore not trimmed). | ||
| /// </para> | ||
| /// <para> | ||
| /// Do — guard inline, so the trimmer can drop the branch and its dependencies: | ||
| /// <code> | ||
| /// if (OsConstants.IsWindows) | ||
| /// { | ||
| /// WindowsOnlyNativeCall(); // becomes `if (false) { ... }` off-Windows, then removed | ||
| /// } | ||
| /// </code> | ||
| /// </para> | ||
| /// <para> | ||
| /// Don't — hide the guard in a throw helper; the trimmer cannot prove the call below is dead: | ||
| /// <code> | ||
| /// static void ThrowIfNotWindows() | ||
| /// { | ||
| /// if (!OsConstants.IsWindows) throw new PlatformNotSupportedException(); | ||
| /// } | ||
| /// // caller: | ||
| /// ThrowIfNotWindows(); | ||
| /// WindowsOnlyNativeCall(); // still reachable to the trimmer; kept even off-Windows | ||
| /// </code> | ||
| /// </para> | ||
| /// </remarks> | ||
| internal static class OsConstants | ||
| { | ||
| /// <summary> | ||
| /// Gets a value indicating whether the runtime is executing on Windows. | ||
| /// </summary> | ||
| #if NET | ||
| internal static bool IsWindows => OperatingSystem.IsWindows(); | ||
| #else | ||
| internal static bool IsWindows => RuntimeInformation.IsOSPlatform(OSPlatform.Windows); | ||
| #endif | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether the runtime is executing on Linux. | ||
| /// </summary> | ||
| #if NET | ||
| internal static bool IsLinux => OperatingSystem.IsLinux(); | ||
| #else | ||
| internal static bool IsLinux => RuntimeInformation.IsOSPlatform(OSPlatform.Linux); | ||
| #endif | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether the runtime is executing on macOS. | ||
| /// </summary> | ||
| #if NET | ||
| internal static bool IsMacOS => OperatingSystem.IsMacOS(); | ||
| #else | ||
| internal static bool IsMacOS => RuntimeInformation.IsOSPlatform(OSPlatform.OSX); | ||
| #endif | ||
|
|
||
| #if NET | ||
| /// <summary> | ||
| /// Gets a value indicating whether the runtime is executing on FreeBSD. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// FreeBSD support is only available in .NET 5+ and later. This property will be | ||
| /// <c>false</c> on .NET Framework or if the runtime does not support FreeBSD detection. | ||
| /// </remarks> | ||
| internal static bool IsFreeBSD => OperatingSystem.IsFreeBSD(); | ||
| #endif | ||
| } | ||
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
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
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
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
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
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't true - TFM != OS. We may happen to only support .NET Framework when running on Windows, but we shouldn't be conflating the two.