mark Windows-specific APIs as such by adamsitnik · Pull Request #39265 · dotnet/runtime

adamsitnik · 2020-07-14T10:52:32Z

@jeffhandley this is just a draft now, I will mark it as ready and remove the "no merge" label as soon as I annotate all types and methods (a matter of hours I hope)

…referenced in < .NET 5 libraries

…NET 5 builds

…bute for Windows Specific libraries

Dotnet-GitSync-Bot · 2020-07-14T10:52:38Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

…ss namespace

…amespace

…les namespace

…mespace

…namespace

…Certificates namespace

…iagnostics.Process namespace" This reverts commit 891848f.

…sting SYSTEM_PRIVATE_CORELIB

adamsitnik · 2020-07-14T16:03:59Z

@jeffhandley it's ready, I've applied the attributes to everything that was on the list except of System.ServiceModel* which lives in a different repo

# Conflicts: # src/libraries/System.IO.FileSystem.DriveInfo/ref/System.IO.FileSystem.DriveInfo.cs # src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.Unix.cs

jkotas · 2020-07-15T11:53:55Z

simplify the attribute name (in places with more than 2 usages)

Nit: I do not think it is worth making distinction between 1 vs 2+ usages. I believe that we typically import namespace even just for 1 usage.

jeffhandley

This is great, @adamsitnik! I just recommend converting all of the src attributes to drop the "Attribute" suffix and use using statements, but otherwise it looks great to me.

…rrors

…ts where this file is included

… from other projects

jeffhandley

LGTM but I defer to @ViktorHofer and @safern on the infrastructure details. Thanks for trimming up the syntax on the attributes!

ViktorHofer

LGTM besides my two comments.

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

* move all OSPlatformAttributes to a single file so they can be easily referenced in < .NET 5 libraries * Adds System.Runtime.Versioning*Platform* annotation attributes to < .NET 5 builds * introduce MinimiumSupportedWindowsPlatform * introduce IsWindowsSpecific setting that adds MinimumOSPlatform attribute for Windows Specific libraries * mark all Windows-specific libraries as such * annotate Windows-specific System.Console methods and properites * annotate Windows-specific DpapiProtectedConfigurationProvider type * annotate throwing Windows-specific APIs from System.Diagnostics.Process namespace * annotate throwing Windows-specific APIs from System.IO.MemoryMappedFiles namespace * annotate Windows-specific APIs from System.IO.Pipes namespace * the new attributes should support enums * annotate Windows-specific APIs from HttpListenerTimeoutManager class * annotate Windows-specific APIs from System.Net.Sockets namespace * annotate Windows-specific APIs from System.Runtime.InteropServices namespace * annotate Windows-specific APIs from System.Security.Cryptography.Csp namespace * annotate Windows-specific APIs from System.Security.Cryptography.X509Certificates namespace * annotate Windows-specific APIs from System.Threading namespace * address code review feedback: dont introduce new constants, reuse existing SYSTEM_PRIVATE_CORELIB * code review: import the namespace, simplify the attribute name * add missing Socket.DuplicateAndClose and Socket(SocketInformation) * fix the test name (it does not throw) * add missing DriveInfo.set_VolumeLabel * add System.IO.FileSystem File Encrypt and Descrypt methods * simplify the < .NET 5 check * enable nullable in explicit way as it's not enabled in all the projects where this file is included * include platform attributes in projects that ask for it in explicit way * set IncludePlatformAttributes to true for projects that include files from other projects * use suggestions from Viktor and Santi Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

adamsitnik added 5 commits July 14, 2020 12:08

move all OSPlatformAttributes to a single file so they can be easily …
1b726fa
…referenced in < .NET 5 libraries

Adds System.Runtime.Versioning*Platform* annotation attributes to < .…
45fa943
…NET 5 builds

introduce MinimiumSupportedWindowsPlatform
77a3977

introduce IsWindowsSpecific setting that adds MinimumOSPlatform attri…
a7faf6a
…bute for Windows Specific libraries

mark all Windows-specific libraries as such
3dc0639

adamsitnik added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 14, 2020

annotate Windows-specific System.Console methods and properites
62ff5d1

jeffhandley self-requested a review July 14, 2020 11:52

adamsitnik added 8 commits July 14, 2020 13:53

annotate Windows-specific DpapiProtectedConfigurationProvider type
3a44806

annotate throwing Windows-specific APIs from System.Diagnostics.Proce…
41d6c39
…ss namespace

annotate nops Windows-specific APIs from System.Diagnostics.Process n…
891848f
…amespace

annotate throwing Windows-specific APIs from System.IO.MemoryMappedFi…
fc992c4
…les namespace

annotate Windows-specific APIs from System.IO.Pipes namespace
0e1cee8

the new attributes should support enums
ce1c855

annotate Windows-specific APIs from HttpListenerTimeoutManager class
4d837ee

annotate Windows-specific APIs from System.Net.Sockets namespace
fbe827c

jkotas reviewed Jul 14, 2020
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Runtime/Versioning/PlatformAttributes.cs Outdated

adamsitnik added 6 commits July 14, 2020 16:47

annotate Windows-specific APIs from System.Runtime.InteropServices na…
bec1e53
…mespace

annotate Windows-specific APIs from System.Security.Cryptography.Csp …
1bb6eac
…namespace

annotate Windows-specific APIs from System.Security.Cryptography.X509…
883fce0
…Certificates namespace

annotate Windows-specific APIs from System.Threading namespace
4452218

Revert (partially) "annotate nops Windows-specific APIs from System.D…
51a28ce
…iagnostics.Process namespace" This reverts commit 891848f.

address code review feedback: dont introduce new constants, reuse exi…
bfe2749
…sting SYSTEM_PRIVATE_CORELIB

adamsitnik removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 14, 2020

adamsitnik marked this pull request as ready for review July 14, 2020 16:03

stephentoub reviewed Jul 14, 2020
View reviewed changes

Comment threadsrc/libraries/System.Console/src/System/Console.cs Outdated

stephentoub reviewed Jul 14, 2020
View reviewed changes

Comment threadsrc/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs Outdated

stephentoub reviewed Jul 14, 2020
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis/NullableAttributes.cs

adamsitnik mentioned this pull request Jul 15, 2020
Mark Windows-specific APIs dotnet/designs#138
Merged

Merge remote-tracking branch 'upstream/master' into windowsSpecificAPIs
99590d5
# Conflicts: # src/libraries/System.IO.FileSystem.DriveInfo/ref/System.IO.FileSystem.DriveInfo.cs # src/libraries/System.IO.FileSystem.DriveInfo/src/System/IO/DriveInfo.Unix.cs

jeffhandley approved these changes Jul 15, 2020
View reviewed changes

Comment threadsrc/libraries/System.Console/src/System/Console.cs Outdated
Comment threadsrc/libraries/Directory.Build.targets
Comment threadsrc/libraries/System.Console/src/System/Console.cs Outdated

adamsitnik commented Jul 15, 2020
View reviewed changes

Comment threadsrc/libraries/System.Private.CoreLib/src/System/Runtime/Versioning/PlatformAttributes.cs

safern reviewed Jul 15, 2020
View reviewed changes

Comment threadsrc/libraries/Directory.Build.targets

safern reviewed Jul 15, 2020
View reviewed changes

Comment threadsrc/libraries/Directory.Build.targets Outdated

ViktorHofer reviewed Jul 15, 2020
View reviewed changes

Comment threadsrc/libraries/Directory.Build.targets Outdated

adamsitnik added 7 commits July 15, 2020 20:43

simplify the < .NET 5 check
b585dd4

don't include .cs files in .ilprojects this leads to crazy compiler e…
83588f8
…rrors

enable nullable in explicit way as it's not enabled in all the projec…
2b3bb5e
…ts where this file is included

include platform attributes in projects that ask for it in explicit way
5490f70

don't use full name of the attribute in the src files
a1dcef8

set IncludePlatformAttributes to true for projects that include files…
b3da567
… from other projects

last place where a full name was used in src
e4df6a9

jeffhandley reviewed Jul 15, 2020
View reviewed changes

jaredpar mentioned this pull request Jul 16, 2020
Errors installing the SDK during builds #34015
Closed

ViktorHofer reviewed Jul 16, 2020
View reviewed changes

Comment threadsrc/libraries/Directory.Build.targets Outdated

safern reviewed Jul 16, 2020
View reviewed changes

Comment threadsrc/libraries/Directory.Build.targets Outdated

ViktorHofer approved these changes Jul 16, 2020
View reviewed changes

safern reviewed Jul 16, 2020
View reviewed changes

Comment threadsrc/libraries/Microsoft.IO.Redist/src/Microsoft.IO.Redist.csproj

safern approved these changes Jul 16, 2020
View reviewed changes

adamsitnikand others added 2 commits July 16, 2020 22:41

use suggestion from Viktor and Santi
ede86ec
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

apply last suggestion
f2b3386

adamsitnik merged commit 52d4d8c into dotnet:masterJul 17, 2020

adamsitnik deleted the windowsSpecificAPIs branch July 17, 2020 13:11

jeffhandley mentioned this pull request Jul 20, 2020
[release/5.0-preview8] Mark Windows-specific APIs as such (#39265) #39657
Merged

karelz added this to the 5.0.0 milestone Aug 18, 2020

ghost locked as resolved and limited conversation to collaborators Dec 8, 2020

Navigation Menu

Uh oh!

mark Windows-specific APIs as such - #39265

mark Windows-specific APIs as such#39265
adamsitnik merged 37 commits into
dotnet:masterfrom
adamsitnik:windowsSpecificAPIs

adamsitnik commented Jul 14, 2020

Uh oh!

Dotnet-GitSync-Bot commented Jul 14, 2020

Uh oh!

Uh oh!

adamsitnik commented Jul 14, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas commented Jul 15, 2020

Uh oh!

jeffhandley left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeffhandley left a comment

Uh oh!

Uh oh!

Uh oh!

ViktorHofer left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

adamsitnik commented Jul 14, 2020

Uh oh!

Dotnet-GitSync-Bot commented Jul 14, 2020

Uh oh!

Uh oh!

adamsitnik commented Jul 14, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas commented Jul 15, 2020

Uh oh!

jeffhandley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeffhandley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ViktorHofer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants