mark Windows-specific APIs as such #39265

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

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

…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

src/libraries/System.Console/src/System/Console.cs

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs

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

# 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.

src/libraries/System.Console/src/System/Console.cs

src/libraries/Directory.Build.targets

src/libraries/System.Console/src/System/Console.cs

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

src/libraries/Directory.Build.targets

…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!

src/libraries/Directory.Build.targets

ViktorHofer

LGTM besides my two comments.

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

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

src/libraries/System.Private.CoreLib/src/System/Runtime/Versioning/PlatformAttributes.cs Outdated Show resolved Hide resolved

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

src/libraries/System.Console/src/System/Console.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 14, 2020

View reviewed changes

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 14, 2020

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Diagnostics/CodeAnalysis/NullableAttributes.cs Show resolved Hide resolved

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

src/libraries/System.Console/src/System/Console.cs Outdated Show resolved Hide resolved

src/libraries/Directory.Build.targets Outdated Show resolved Hide resolved

src/libraries/System.Console/src/System/Console.cs Outdated Show resolved Hide resolved

adamsitnik commented Jul 15, 2020

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Runtime/Versioning/PlatformAttributes.cs Show resolved Hide resolved

safern reviewed Jul 15, 2020

View reviewed changes

src/libraries/Directory.Build.targets Show resolved Hide resolved

safern reviewed Jul 15, 2020

View reviewed changes

src/libraries/Directory.Build.targets Outdated Show resolved Hide resolved

ViktorHofer reviewed Jul 15, 2020

View reviewed changes

src/libraries/Directory.Build.targets Outdated Show resolved Hide resolved

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

src/libraries/Directory.Build.targets Outdated Show resolved Hide resolved

safern reviewed Jul 16, 2020

View reviewed changes

src/libraries/Directory.Build.targets Outdated Show resolved Hide resolved

ViktorHofer approved these changes Jul 16, 2020

View reviewed changes

safern reviewed Jul 16, 2020

View reviewed changes

src/libraries/Microsoft.IO.Redist/src/Microsoft.IO.Redist.csproj Show resolved Hide resolved

safern approved these changes Jul 16, 2020

View reviewed changes

adamsitnik and 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:master Jul 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

mark Windows-specific APIs as such #39265

mark Windows-specific APIs as such #39265

Uh oh!

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!

Uh oh!

mark Windows-specific APIs as such #39265

mark Windows-specific APIs as such #39265

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!

Uh oh!