-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Use SupportedOSPlatforms property to generate assembly attributes #41209
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
Changes from all commits
c280248
a1bb7bf
7e95fd6
2feacc8
66a0773
f75b56c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,23 +22,25 @@ | |
| </AssemblyMetadata> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Adds SupportedOSPlatform attribute for Windows Specific libraries --> | ||
| <ItemGroup Condition="'$(IsWindowsSpecific)' == 'true' and '$(IsTestProject)' != 'true'"> | ||
| <AssemblyAttribute Include="System.Runtime.Versioning.SupportedOSPlatform"> | ||
| <_Parameter1>windows</_Parameter1> | ||
| </AssemblyAttribute> | ||
| <!-- Adds SupportedOSPlatform or UnsupportedOSPlatform attributes --> | ||
| <ItemGroup Condition="'$(IsTestProject)' != 'true'"> | ||
| <_supportedOSPlatforms Include="$(AssemblySupportedOSPlatformAttributes)" /> | ||
| <_unsupportedOSPlatforms Include="$(AssemblyUnsupportedOSPlatformAttributes)" Condition="'$(_assemblySupportedOSPlatformAttributesOverride)' != 'true'" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <_unsupportedOSPlatforms Include="$(UnsupportedOSPlatforms)" /> | ||
| </ItemGroup> | ||
|
|
||
| <Target Name="AddUnsupportedOSPlatformAttribute" BeforeTargets="GenerateAssemblyInfo" AfterTargets="PrepareForBuild"> | ||
| <Target Name="AddAssemblyOSPlatformAttributes" BeforeTargets="GenerateAssemblyInfo" AfterTargets="PrepareForBuild"> | ||
| <ItemGroup> | ||
| <AssemblyAttribute Include="System.Runtime.Versioning.UnsupportedOSPlatform" Condition="'@(_unsupportedOSPlatforms)' != ''"> | ||
| <!-- When there are Supported OSPlatforms, transform each of those into an attribute --> | ||
| <AssemblyAttribute Include="System.Runtime.Versioning.SupportedOSPlatform" Condition="'@(_supportedOSPlatforms)' != ''"> | ||
| <_Parameter1>%(_supportedOSPlatforms.Identity)</_Parameter1> | ||
| </AssemblyAttribute> | ||
| <!-- When there are not any Supported OSPlatforms, but there are Unsupported OSPlatforms, transform each of those into an attribute --> | ||
| <AssemblyAttribute Include="System.Runtime.Versioning.UnsupportedOSPlatform" Condition="'@(_supportedOSPlatforms)' == '' and '@(_unsupportedOSPlatforms)' != ''"> | ||
| <_Parameter1>%(_unsupportedOSPlatforms.Identity)</_Parameter1> | ||
| </AssemblyAttribute> | ||
jeffhandley marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+38
to
40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we produce a build warning when both
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea |
||
| </ItemGroup> | ||
| <Warning Condition="'@(_supportedOSPlatforms)' != '' and '@(_unsupportedOSPlatforms)' != ''" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would just Error. We set warnAsError for the global build, so it is the same, we usually try to error for this kind of stuff as warnings could slip sometimes. |
||
| Text="The 'AssemblySupportedOSPlatformAttributes' and 'AssemblyUnsupportedOSPlatformAttributes' properties cannot both be defined; only the AssemblySupportedOSPlatformAttributes was used." /> | ||
| </Target> | ||
|
|
||
| <Target Name="DecideIfWeNeedToIncludeDllSafeSearchPathAttribute" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,8 +111,11 @@ public static IHostBuilder CreateDefaultBuilder(string[] args) | |
|
|
||
| if (isWindows) | ||
| { | ||
| #pragma warning disable CA1416 // Platform compat analyzer | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? Can the analyzer not recognize this pattern?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Analyzer can track result save in local, it shouldn't warn
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was warning on this. Remind me; can the analyzer also recognize the old
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes it can
That is strange, i added exact same test and it is not warning
Sure
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Filed an issue for the bug dotnet/roslyn-analyzers#4090 |
||
| // Add the EventLogLoggerProvider on windows machines | ||
| logging.AddEventLog(); | ||
| #pragma warning restore CA1416 // Platform compat analyzer | ||
|
|
||
| } | ||
|
|
||
| logging.Configure(options => | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| <Project> | ||
| <Import Project="..\Directory.Build.props" /> | ||
| <PropertyGroup> | ||
| <IsWindowsSpecific>true</IsWindowsSpecific> | ||
| <AssemblySupportedOSPlatformAttributes>windows</AssemblySupportedOSPlatformAttributes> | ||
| </PropertyGroup> | ||
| </Project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -262,6 +262,7 @@ public static TcpListener Create(int port) | |
| return listener; | ||
| } | ||
|
|
||
| [SupportedOSPlatform("windows")] | ||
| private void SetIPProtectionLevel(bool allowed) | ||
| => _serverSocket!.SetIPProtectionLevel(allowed ? IPProtectionLevel.Unrestricted : IPProtectionLevel.EdgeRestricted); | ||
|
|
||
|
|
@@ -274,7 +275,7 @@ private void CreateNewSocketIfNeeded() | |
| _serverSocket.ExclusiveAddressUse = true; | ||
| } | ||
|
|
||
| if (_allowNatTraversal != null) | ||
| if (_allowNatTraversal != null && OperatingSystem.IsWindows()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we concerned about behavior change here at all? Previously someone would get a |
||
| { | ||
| SetIPProtectionLevel(_allowNatTraversal.GetValueOrDefault()); | ||
| _allowNatTraversal = null; // Reset value to avoid affecting more sockets | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.