-
Notifications
You must be signed in to change notification settings - Fork 330
Merge | Not Supported Binaries / Test Targets #3997
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
Merged
Merged
Changes from 11 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
b0afd95
Stubbing out additional targets for build2.proj
benrr101 a7a9bf7
Fix issue with target OS and target frameworks in common MDS build
benrr101 4065b08
Adding targets to build2.proj for testing MDS, removing AKV provider …
benrr101 83cc907
Adding build target for MDS ref project
benrr101 154b150
Get common ref project building to artifacts folder, a la common mds …
benrr101 04ebcb7
Add stub for notsupported project
benrr101 86e75e3
Add GenAPI projects to the solution file under notsupported folder
benrr101 392723b
Move GenAPI output path to bin folder for project - they are not "art…
benrr101 1b406ce
Not supported assemblies are building via new project
benrr101 a214c38
Do not build not supported assembly and genapi projects as part of so…
benrr101 406d810
Ignore the notsupported GenAPI files
benrr101 82d73ae
Comments from copilot
benrr101 5c3e3c2
Remove Directory.Build.props and move target frameworks listing to th…
benrr101 7187e3e
Conditionally enable net462 on Azure test project
benrr101 a904283
Removing Directory.Build.props from test/tools
benrr101 3c45064
Add trim docs for intellisense to ref project (just inline it)
benrr101 f47ae65
Rework the package version arguments in build2.proj - this will make …
benrr101 1689a0a
Merge branch 'main' into dev/russellben/common/notsupported
benrr101 a50381e
Fix GenAPI for legacy build
benrr101 8706133
No clue whatsoever why this fixes functional tests on net462, but hey…
benrr101 2e332c6
Actionable PR comments
benrr101 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
Some comments aren't visible on the classic Files Changed page.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -363,3 +363,6 @@ MigrationBackup/ | |
|
|
||
| # Config Json file | ||
| **/config.json | ||
|
|
||
| # MDS "Not Supported" GenAPI code | ||
| **/notsupported/*.cs | ||
Large diffs are not rendered by default.
Oops, something went wrong.
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
208 changes: 208 additions & 0 deletions
208
src/Microsoft.Data.SqlClient/notsupported/Microsoft.Data.SqlClient.csproj
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,208 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <!-- | ||
| Everybody hold onto your pants because this is going to get a bit ugly. | ||
|
|
||
| The purpose of this project is to generate a "Not Supported" version of the MDS assembly. At a | ||
| high level, this is done by: | ||
| * Building the ref project (as a dependency to this project) | ||
| * Feeding the ref assembly to GenAPI (built separately) | ||
| * GenAPI generates a single cs file with the entire API surface implemented to throw | ||
| PlatformNotSupportedExceptions | ||
| * This cs file is included as a compilation item in this project. | ||
|
|
||
| The big question might be: Why do we need a "Not Supported" version of MDS? | ||
| In some situations, most importantly PowerShell, including a NuGet package will automatically | ||
| reference the appropriate assembly in the /lib folder of the package, which is inherently OS- | ||
| agnostic. MDS implementation assemblies are placed in the /runtimes folder to provide the OS- | ||
| specific versions of MDS. The "Not Supported" binary is thus placed in the lib folder so that | ||
| in OS-agnostic situations, the user is reminded to reference an OS-specific version of MDS. | ||
|
|
||
| The next big question might be: Why is this a separate project? | ||
| In previous iterations, generating the "Not Supported" binaries was part of the netcore ref | ||
| project and netcore src project. This made it necessary to conditionally switch the project | ||
| and all associated variables from "implementation" mode into "not supported" mode. It was | ||
| very difficult to understand what was going on. Creating a separate project just for | ||
| "not supported" mode simplifies this considerably. It also makes it much cleaner to excise | ||
| when this project is no longer required. | ||
|
|
||
| The last big question might be: Why does this need to be a project at all? | ||
| Since it looks like we're just running a static GenAPI program against the ref binaries, surely | ||
| we could just do that as an arbitrary command execution in the build.proj file, right? Well, | ||
| not really. GenAPI needs a list of folders to find built-in reference libraries. To get that | ||
| list correctly, we need access to @(ReferencePath), which is most easily done by passing it in | ||
| as an input to a target in a csproj. What's more, we need to compile the source that GenAPI | ||
| generates. Unless we want to manually call csc (and who wants to do that) we need a project. | ||
|
|
||
| NOTE: If you are trying to build this from your IDE - YOU ARE DOING IT WRONG. | ||
| This project depends on the ref binaries and the GenAPI project in a way that is not conducive | ||
| to building in a project. Build the not supported binaries in one command via build2.proj | ||
|
paulmedynski marked this conversation as resolved.
Outdated
|
||
|
|
||
| > msbuild build2.proj /t:BuildMdsNotSupported | ||
|
|
||
| @TODO: Remove this project | ||
| The primary prerequisite for removing this project is making the "common" MDS project OS- | ||
| agnostic. Once that happens, the lib binary will be able to switch OS implementation at runtime | ||
| and a "not supported" error is no longer necessary. | ||
|
|
||
| alternatively: | ||
| @TODO: Replace the GenAPI project with a C# Source Generator | ||
| --> | ||
|
|
||
| <!-- General Properties ============================================== --> | ||
| <PropertyGroup> | ||
| <AssemblyName>Microsoft.Data.SqlClient</AssemblyName> | ||
| <TargetFrameworks>net462;net8.0;net9.0;netstandard2.0</TargetFrameworks> | ||
|
paulmedynski marked this conversation as resolved.
|
||
|
|
||
| <!-- | ||
| Files to compile will be generated per-framework by the GenerateNotSupportedSource target and | ||
| included via Compile tags. Leaving this enabled will cause warnings about files being | ||
| included more than once. | ||
| --> | ||
| <EnableDefaultCompileItems>false</EnableDefaultCompileItems> | ||
|
|
||
| <!-- Disable obsolete member usage warning --> | ||
| <NoWarn>$(NoWarn);CS0618</NoWarn> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- BUild Output ==================================================== --> | ||
|
benrr101 marked this conversation as resolved.
Outdated
|
||
| <PropertyGroup> | ||
| <!-- | ||
| MSBuild will add the target framework to the end of this path by default. Telling it not to | ||
| is possible but also requires specifying the IntermediateOutputPath. So while it would be | ||
| nice to have a flatter directory structure, it's more hassle than its worth. | ||
| --> | ||
| <OutputPath>$(RepoRoot)artifacts/$(AssemblyName).notsupported/$(Configuration)/</OutputPath> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- References ====================================================== --> | ||
| <!-- | ||
| Reference the ref project to cause it to be built, so we can use it as the contract to build | ||
| not supported assemblies against. Note, this must be set to output item type of none otherwise | ||
| the assemblies for the ref project will be compied to the output for this project, causing | ||
|
benrr101 marked this conversation as resolved.
Outdated
|
||
| conflicts that fail the build. | ||
| --> | ||
| <PropertyGroup> | ||
| <RefProjectPath>$(RepoRoot)src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj</RefProjectPath> | ||
| <RefArtifactPath>$(RepoRoot)artifacts/Microsoft.Data.SqlClient.ref/$(Configuration)/$(TargetFramework)/Microsoft.Data.SqlClient.dll</RefArtifactPath> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <ProjectReference Include="$(RefProjectPath)" | ||
| ReferenceOutputAssembly="false" | ||
| Private="false" | ||
| OutputItemType="None" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- | ||
| Reference to Abstractions package. Even though the ref project has a reference to the | ||
| abstractions package, the binaries will not be copied into this project's output, so we need to | ||
| make our own reference to the package to ensure it is brought in for compilation. | ||
| --> | ||
| <ItemGroup> | ||
| <ProjectReference Include="$(RepoRoot)src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Abstractions.csproj" | ||
| Condition="'$(ReferenceType)' != 'Package'" /> | ||
| <PackageReference Include="Microsoft.Data.SqlClient.Extensions.Abstractions" | ||
| Condition="'$(ReferenceType)' == 'Package'" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- References to external packages that the API refers to --> | ||
| <!-- References for netframework --> | ||
| <ItemGroup Condition="'$(TargetFramework)' == 'net462'"> | ||
| <Reference Include="System" /> | ||
| <Reference Include="System.Configuration" /> | ||
| <Reference Include="System.Data" /> | ||
| <Reference Include="System.EnterpriseServices" /> | ||
| <Reference Include="System.Runtime.Caching" /> | ||
| <Reference Include="System.Runtime.Serialization" /> | ||
| <Reference Include="System.Security" /> | ||
| <Reference Include="System.Transactions" /> | ||
| <Reference Include="System.Xml" /> | ||
|
|
||
| <PackageReference Include="Microsoft.Bcl.Cryptography" /> | ||
| <PackageReference Include="Microsoft.Data.SqlClient.SNI" | ||
| PrivateAssets="all" | ||
| IncludeAssets="runtime;build;native;contentfiles;analyzers;buildtransitive" /> | ||
| <PackageReference Include="Microsoft.Extensions.Caching.Memory" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" /> | ||
| <PackageReference Include="System.Buffers" /> | ||
| <PackageReference Include="System.Diagnostics.DiagnosticSource" /> | ||
| <PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" /> | ||
| <PackageReference Include="System.Security.Cryptography.Pkcs" /> | ||
| <PackageReference Include="System.Text.Json" /> | ||
| <PackageReference Include="System.Threading.Channels" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- References for netstandard --> | ||
| <ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'"> | ||
| <PackageReference Include="Microsoft.Bcl.Cryptography" /> | ||
| <PackageReference Include="Microsoft.Data.SqlClient.SNI.runtime" /> | ||
| <PackageReference Include="Microsoft.Extensions.Caching.Memory" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" /> | ||
| <PackageReference Include="Microsoft.SqlServer.Server" /> | ||
| <PackageReference Include="System.Configuration.ConfigurationManager" /> | ||
| <PackageReference Include="System.Security.Cryptography.Pkcs" /> | ||
| <PackageReference Include="System.Text.Json" /> | ||
| <PackageReference Include="System.Threading.Channels" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- References for netcore --> | ||
| <ItemGroup Condition="'$(TargetFramework)' != 'net462' AND '$(TargetFramework)' != 'netstandard2.0'"> | ||
| <PackageReference Include="Microsoft.Bcl.Cryptography" /> | ||
| <PackageReference Include="Microsoft.Data.SqlClient.SNI.runtime" /> | ||
| <PackageReference Include="Microsoft.Extensions.Caching.Memory" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" /> | ||
| <PackageReference Include="Microsoft.SqlServer.Server" /> | ||
| <PackageReference Include="System.Configuration.ConfigurationManager" /> | ||
| <PackageReference Include="System.Security.Cryptography.Pkcs" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Generate "Not Supported" Source ================================= --> | ||
| <PropertyGroup> | ||
| <NotSupportedSourceFile>$(AssemblyName).$(TargetFramework).notsupported.cs</NotSupportedSourceFile> | ||
| </PropertyGroup> | ||
|
|
||
| <Target Name="GenerateNotSupportedSource" | ||
| BeforeTargets="CoreCompile" | ||
| DependsOnTargets="ResolveReferences"> | ||
|
|
||
| <!-- Verify that GenAPI and ref projects were built successfully --> | ||
| <Error Text="Ref project output was not found at: '$(RefArtifactPath)'" | ||
| Condition="!Exists('$(RefArtifactPath)')" /> | ||
| <Error Text="GenAPI project output was not found at: '$(GenApiPath)'" | ||
| Condition="!Exists('$(GenApiPath)')" /> | ||
|
|
||
| <!-- | ||
| Cleanup the list of reference path directories by trimming trailing \ and removing | ||
| duplicates. | ||
| --> | ||
| <ItemGroup> | ||
| <_referencePathDirectoriesWithDuplicates | ||
| Include="@(ReferencePath->'%(RootDir)%(Directory)'->TrimEnd('\'))" /> | ||
| <_referencePathDirectories | ||
| Include="%(_referencePathDirectoriesWithDuplicates.Identity)" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Execute GenAPI against the ref assembly to generate the source file --> | ||
| <PropertyGroup> | ||
| <GenApiCommand> | ||
| $(DotnetPath)dotnet "$(GenApiPath)" | ||
| $(RefArtifactPath) | ||
| -l:"@(_referencePathDirectories)" | ||
| -o:"$(NotSupportedSourceFile)" | ||
| -t:"Microsoft.Data.SqlClient is not supported on this platform." | ||
| </GenApiCommand> | ||
| <!-- Convert more than one whitespace character into one space --> | ||
| <GenApiCommand>$([System.Text.RegularExpressions.Regex]::Replace($(GenApiCommand), "\s+", " "))</GenApiCommand> | ||
| </PropertyGroup> | ||
|
|
||
| <Message Text=">>> Building MDS ref binaries via command: $(DotnetCommand)"/> | ||
|
benrr101 marked this conversation as resolved.
Outdated
|
||
| <Exec Command="$(GenApiCommand)" ConsoleToMsBuild="true" /> | ||
|
|
||
| <!-- Include the "Not Supported" source file into this project --> | ||
| <ItemGroup> | ||
| <Compile Include="$(NotSupportedSourceFile)" /> | ||
| </ItemGroup> | ||
| </Target> | ||
| </Project> | ||
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.
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.