Skip to content

Merge Project | Build the Common Project #3837

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.

Sign up for GitHub

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
benrr101 wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Merge Project | Build the Common Project #3837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
871859c
Add reference to MSS for netcore to fix InvalidUdtException reference…
benrr101 Dec 4, 2025
8250468
SqlDependencyUtils.* are all netcore files, rename as such, and wrap …
benrr101 Dec 4, 2025
d253767
Set build output path
benrr101 Dec 5, 2025
477219d
Add OS-specific check for target frameworks
benrr101 Dec 5, 2025
70fa18f
Builds! Whoohooo!
benrr101 Dec 5, 2025
61978e1
Common project build targets
benrr101 Dec 5, 2025
946f41b
Add common project back to the solution build targets
benrr101 Dec 9, 2025
2e92020
Comments from PR
benrr101 Dec 10, 2025
8d4ca14
Rework package references as per comments, add normalized target os v…
benrr101 Dec 16, 2025
4c25b0f
Merge branch 'dev/russellben/build-common-project' of github.com:dotn…
benrr101 Dec 16, 2025
2a0ff49
Add message on exec
benrr101 Dec 16, 2025
ca43c20
Add strong name signing and triple check package references
benrr101 Dec 17, 2025
392702b
Unconditional assembly originator key file, clarify command
benrr101 Dec 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions build2.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="Current" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<!-- Build Parameters ============================================== -->
<PropertyGroup>
<!-- Configuration: Which build configuration to build -->
<!-- Allowed values: Debug, Release -->
<!-- Default value: Debug -->
<Configuration Condition="'$(Configuration)' == ''">Debug</Configuration>

<!-- DotnetPath: Path to folder containing the `dotnet` binary. Override this if a -->
<!-- specific version (eg, x86) is required. Otherwise, this defaults to using the -->
<!-- dotnet binary in the PATH variable. The provided path should end with a `\` (or -->
<!-- `/`) character. Eg. C:\x86\ -->
<DotnetPath Condition="'$(DotnetPath)' == ''"></DotnetPath>
</PropertyGroup>

<!-- Imports ======================================================= -->
<Import Project="src/Directory.Build.props" />

<!-- Microsoft.Data.SqlClient Build Targets ======================== -->
<PropertyGroup>
<MdsProjectPath>$(RepoRoot)src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj</MdsProjectPath>
</PropertyGroup>

<!-- BuildMds: Builds all binaries for MDS -->
<Target Name="BuildMds" DependsOnTargets="BuildMdsUnix;BuildMdsWindows" />

<!-- BuildMdsUnix: Builds all unix-specific MDS binaries -->
<Target Name="BuildMdsUnix">
<PropertyGroup>
<DotnetCommand>
$(DotnetPath)dotnet build $(MdsProjectPath)
-p:Configuration=$(Configuration)
-p:TargetOs=Unix
</DotnetCommand>
<!-- Convert more than one whitespace character into one space -->
<DotnetCommand>$([System.Text.RegularExpressions.Regex]::Replace($(DotnetCommand), "\s+", " "))</DotnetCommand>
</PropertyGroup>
<Message Text=">>> Building MDS for Unix via command: $(DotnetCommand)"/>
<Exec ConsoleToMsBuild="true" Command="$(DotnetCommand)" />
</Target>

<!-- BuildMdsWindows: Builds all windows-specific MDS binaries -->
<Target Name="BuildMdsWindows">
Copy link
Member

@cheenamalhotra cheenamalhotra Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget strong name key signing is a MUST.

Copy link
Contributor Author

@benrr101 benrr101 Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch. I will get that in on this PR. Otherwise, I'll definitely forget.

<PropertyGroup>
<DotnetCommand>
$(DotnetPath)dotnet build $(MdsProjectPath)
-p:Configuration=$(Configuration)
-p:TargetOs=Windows_NT
</DotnetCommand>
<!-- Convert more than one whitespace character into one space -->
<DotnetCommand>$([System.Text.RegularExpressions.Regex]::Replace($(DotnetCommand), "\s+", " "))</DotnetCommand>
</PropertyGroup>
<Message Text=">>> Building MDS for Windows via command: $(DotnetCommand)"/>
<Exec ConsoleToMsBuild="true" Command="$(DotnetCommand)" />
</Target>
</Project>
8 changes: 7 additions & 1 deletion src/Microsoft.Data.SqlClient.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Microsoft Visual Studio Solution File, Format Version 12.00
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.0.31912.275
MinimumVisualStudioVersion = 10.0.40219.1
Expand Down Expand Up @@ -561,6 +561,12 @@ Global
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Release|Any CPU.ActiveCfg = Release|Any CPU
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Release|x64.ActiveCfg = Release|Any CPU
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Release|x86.ActiveCfg = Release|Any CPU
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Debug|Any CPU.Build.0 = Debug|Any CPU
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Debug|x64.Build.0 = Debug|Any CPU
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Debug|x86.Build.0 = Debug|Any CPU
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Release|Any CPU.Build.0 = Release|Any CPU
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Release|x64.Build.0 = Release|Any CPU
{9A8996A8-6484-4AA7-B50F-F861430EDE2F}.Release|x86.Build.0 = Release|Any CPU
{4461063D-2F2B-274C-7E6F-F235119D258E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{4461063D-2F2B-274C-7E6F-F235119D258E}.Debug|Any CPU.Build.0 = Debug|Any CPU
{4461063D-2F2B-274C-7E6F-F235119D258E}.Debug|x64.ActiveCfg = Debug|x64
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,11 +815,11 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlDependencyUtils.cs">
<Link>Microsoft\Data\SqlClient\SqlDependencyUtils.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlDependencyUtils.AppDomain.cs">
<Link>Microsoft\Data\SqlClient\SqlDependencyUtils.AppDomain.cs</Link>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlDependencyUtils.AppDomain.netcore.cs">
<Link>Microsoft\Data\SqlClient\SqlDependencyUtils.AppDomain.netcore.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlDependencyUtils.AssemblyLoadContext.cs">
<Link>Microsoft\Data\SqlClient\SqlDependencyUtils.AssemblyLoadContext.cs</Link>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlDependencyUtils.AssemblyLoadContext.netcore.cs">
<Link>Microsoft\Data\SqlClient\SqlDependencyUtils.AssemblyLoadContext.netcore.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlEnclaveAttestationParameters.Crypto.cs">
<Link>Microsoft\Data\SqlClient\SqlEnclaveAttestationParameters.Crypto.cs</Link>
Expand Down
97 changes: 75 additions & 22 deletions src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,55 +1,108 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net462;net8.0;net9.0</TargetFrameworks>
<!-- General Properties ============================================== -->
<PropertyGroup>
<Configurations>Debug;Release;</Configurations>
Copy link
Contributor

@paulmedynski paulmedynski Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this property documented - what do we think it's doing?

Copy link
Contributor Author

@benrr101 benrr101 Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was defining the allowed configurations that the project can be built in. But... yeah, I don't see it in the docs anywhere. I will remove it.

Copy link
Member

@cheenamalhotra cheenamalhotra Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dotnet build properties will apply here, look here for docs: https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-build

Configurations should be specified to allow Release configuration to be picked up for building.

Copy link
Contributor

@paulmedynski paulmedynski Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Abstractions and Azure projects don't specify these and they have no problem building in Debug or Release configuration. Are we sure this isn't from the legacy .NET Framework style projects?

Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Configurations property has a trailing semicolon after 'Release;'. While this doesn't cause a build error in MSBuild, it creates an empty configuration string in the list. The trailing semicolon should be removed for cleaner configuration handling.

Copilot uses AI. Check for mistakes.
<RootNamespace />
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<RootNamespace />
</PropertyGroup>

<!-- Strong name signing ============================================= -->
<PropertyGroup>
<AssemblyOriginatorKeyFile>$(SigningKeyPath)</AssemblyOriginatorKeyFile>
</PropertyGroup>
<PropertyGroup Condition="'$(CDP_BUILD_TYPE)' == 'Official'">
<SignAssembly>true</SignAssembly>
<KeyFile>$(SigningKeyPath)</KeyFile>
</PropertyGroup>


<!-- OS Constants ==================================================== -->
<PropertyGroup>
<!-- @TODO: Move to directory.build.props? -->
<!-- If a target OS was not specified, use the current OS as the target OS. -->
<TargetOs Condition="'$(TargetOs)' == ''">$(OS)</TargetOs>

<!-- Uncomment the following line to override the OS you are developing for -->
<!--<TargetOs>Unix</TargetOs>-->
<!--<TargetOs>Windows_NT</TargetOs>-->

<NormalizedTargetOs>$(TargetOs.ToLower())</NormalizedTargetOs>

<!-- NOTE: These constants are prefixed with _ to keep them separate from .NET 5+ precompiler -->
<!-- flags. Those only apply to OS-specific target frameworks, and would interfere here. -->
<DefineConstants Condition="'$(TargetOs.ToUpper())' == 'UNIX'">$(DefineConstants),_UNIX</DefineConstants>
<DefineConstants Condition="'$(TargetOs.ToUpper())' == 'WINDOWS_NT'">$(DefineConstants),_WINDOWS</DefineConstants>
<!-- flags. Those only apply to OS-specific target frameworks, and would interfere here. -->
<DefineConstants Condition="'$(NormalizedTargetOs)' == 'unix'">$(DefineConstants);_UNIX</DefineConstants>
<DefineConstants Condition="'$(NormalizedTargetOs)' == 'windows_nt'">$(DefineConstants);_WINDOWS</DefineConstants>
Comment on lines +33 to +34
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefineConstants should use commas as separators, not semicolons. The existing netcore and netfx projects consistently use the format '$(DefineConstants),_UNIX' with a comma. Using semicolons may cause the preprocessor constants to not be recognized correctly. Change the semicolon to a comma in both lines.

Copilot uses AI. Check for mistakes.
</PropertyGroup>

<!-- Target Frameworks =============================================== -->
<PropertyGroup>
<!-- net462 is only supported on Windows, so we will only add it if we're building for Windows -->
<TargetFrameworks>net8.0;net9.0</TargetFrameworks>
<TargetFrameworks Condition="'$(NormalizedTargetOs)' == 'windows_nt'">$(TargetFrameworks);net462</TargetFrameworks>
Comment on lines +40 to +41
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The target frameworks should follow the conventional order from oldest to newest. Currently, the base frameworks are 'net8.0;net9.0' with 'net462' conditionally appended. Consider using 'net462;net8.0;net9.0' when on Windows to maintain consistency with the existing netcore and netfx projects which list net462 first. This ordering is also consistent with the original TargetFrameworks definition that was replaced.

Suggested change
<TargetFrameworks>net8.0;net9.0</TargetFrameworks>
<TargetFrameworks Condition="'$(NormalizedTargetOs)' == 'windows_nt'">$(TargetFrameworks);net462</TargetFrameworks>
<TargetFrameworks Condition="'$(NormalizedTargetOs)' != 'windows_nt'">net8.0;net9.0</TargetFrameworks>
<TargetFrameworks Condition="'$(NormalizedTargetOs)' == 'windows_nt'">net462;net8.0;net9.0</TargetFrameworks>

Copilot uses AI. Check for mistakes.
</PropertyGroup>

<!-- Build Output ==================================================== -->
<PropertyGroup>
<!-- @TODO: Move to directory.build.props? -->
<ArtifactPath>$(RepoRoot)artifacts/</ArtifactPath>

<!-- MSBuild will add the target framework to the end of this path by default. Telling it not -->
<!-- to is possible but requires also specifying the IntermediateOutputPath. So while it -->
<!-- would be nice to have a flat directory structure, it's more hassle than its worth. -->
<OutputPath>$(ArtifactPath)$(AssemblyName)/$(Configuration)/$(NormalizedTargetOs)/</OutputPath>
Comment on lines +46 to +52
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TODO comment suggests moving ArtifactPath to directory.build.props. However, this property is being used in a non-standard way and conflicts with the existing Artifacts property defined in Directory.Build.props (line 38). The existing convention uses $(Artifacts) which includes the ReferenceType folder. This should be reconciled before moving to a shared location to avoid breaking existing builds.

Suggested change
<!-- @TODO: Move to directory.build.props? -->
<ArtifactPath>$(RepoRoot)artifacts/</ArtifactPath>
<!-- MSBuild will add the target framework to the end of this path by default. Telling it not -->
<!-- to is possible but requires also specifying the IntermediateOutputPath. So while it -->
<!-- would be nice to have a flat directory structure, it's more hassle than its worth. -->
<OutputPath>$(ArtifactPath)$(AssemblyName)/$(Configuration)/$(NormalizedTargetOs)/</OutputPath>
<!-- Project-specific artifact root; kept local to avoid conflicts with global $(Artifacts) conventions. -->
<SqlClientArtifactPath>$(RepoRoot)artifacts/</SqlClientArtifactPath>
<!-- MSBuild will add the target framework to the end of this path by default. Telling it not -->
<!-- to is possible but requires also specifying the IntermediateOutputPath. So while it -->
<!-- would be nice to have a flat directory structure, it's more hassle than its worth. -->
<OutputPath>$(SqlClientArtifactPath)$(AssemblyName)/$(Configuration)/$(NormalizedTargetOs)/</OutputPath>

Copilot uses AI. Check for mistakes.
</PropertyGroup>

<!-- Embedded resources ============================================== -->
<ItemGroup>
<!-- Linker directives to replace UseManagedNetworking with a constant if consumer specifies -->
<!-- the app context switch in their csproj. This file only applies to netcore on windows. -->
<!-- This file does not support pre-processor directives, so it must be conditionally -->
<!-- included into the build. -->
<EmbeddedResource Include="Resources/ILLink.Substitutions.xml"
Condition="'$(TargetOs.ToUpper())' == 'WINDOWS_NT'" />
Condition="'$(NormalizedTargetOs)' == 'windows_nt' AND '$(TargetFramework)' != 'net462'" />
</ItemGroup>

<!-- References ====================================================== -->
<ItemGroup>
<!-- References that apply to all target frameworks -->
<!-- References for netframework -->
<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<Reference Include="System.Configuration" />
<Reference Include="System.EnterpriseServices" />
<Reference Include="System.Transactions" />

<PackageReference Include="Azure.Core" />
<PackageReference Include="Azure.Identity" />
<PackageReference Include="Microsoft.Bcl.Cryptography" />
<PackageReference Include="Microsoft.Data.SqlClient.SNI">
<PrivateAssets>All</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.Extensions.Caching.Memory" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
<PackageReference Include="System.Configuration.ConfigurationManager" />
<PackageReference Include="System.Security.Cryptography.Pkcs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<!-- References that only apply to net462 -->
<Reference Include="System.Configuration" />
<Reference Include="System.Transactions" />
<PackageReference Include="System.ValueTuple" />
<PackageReference Include="System.Threading.Channels" />
</ItemGroup>

<!-- System.Buffers, System.Memory, and System.Runtime.InteropServices.RuntimeInformation are included in .NET 10+ BCL -->
<ItemGroup Condition="$([MSBuild]::VersionLessThan($([MSBuild]::GetTargetFrameworkVersion($(TargetFramework))), '10.0'))">
<PackageReference Include="System.Buffers" />
<PackageReference Include="System.Diagnostics.DiagnosticSource" />
<PackageReference Include="System.Memory" />
<PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" />
<PackageReference Include="System.Security.Cryptography.Pkcs" />
Copy link
Contributor

@mdaigle mdaigle Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's follow the conversation at #3843 and make changes here accordingly. Doesn't necessarily need to be in this PR.

<PackageReference Include="System.Text.Json" />
<PackageReference Include="System.Threading.Channels" />
<PackageReference Include="System.ValueTuple" />
</ItemGroup>
Comment on lines 32 to +90
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package reference organization contains duplicate references between the net462 and netcore sections. Both sections include identical packages like Azure.Core, Azure.Identity, Microsoft.Bcl.Cryptography, Microsoft.Extensions.Caching.Memory, Microsoft.IdentityModel.JsonWebTokens, Microsoft.IdentityModel.Protocols.OpenIdConnect, and System.Security.Cryptography.Pkcs. These shared packages should be moved to a common ItemGroup that applies to all target frameworks to reduce duplication and maintenance burden.

Copilot uses AI. Check for mistakes.

<!-- References for netcore -->
<ItemGroup Condition="'$(TargetFramework)' != 'net462'">
<PackageReference Include="Azure.Core" />
<PackageReference Include="Azure.Identity" />
<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>

<!-- CodeGen Targets ================================================= -->
<Import Project="$(ToolsDir)targets\GenerateThisAssemblyCs.targets" />
</Project>
25 changes: 14 additions & 11 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,24 +69,27 @@ internal static partial class ADP
internal const int MaxBufferAccessTokenExpiry = 600;

#region UDT
#if NETFRAMEWORK
private static readonly MethodInfo s_method = typeof(InvalidUdtException).GetMethod("Create", BindingFlags.NonPublic | BindingFlags.Static);
#endif

#if NETFRAMEWORK
private static readonly MethodInfo s_udtFactory =
typeof(InvalidUdtException).GetMethod("Create", BindingFlags.NonPublic | BindingFlags.Static);
#endif

/// <summary>
/// Calls "InvalidUdtException.Create" method when an invalid UDT occurs.
/// </summary>
internal static InvalidUdtException CreateInvalidUdtException(Type udtType, string resourceReasonName)
{
// @TODO: Can we adopt the netcore version?
InvalidUdtException e =
#if NETFRAMEWORK
(InvalidUdtException)s_method.Invoke(null, new object[] { udtType, resourceReasonName });
ADP.TraceExceptionAsReturnValue(e);
#else
InvalidUdtException.Create(udtType, resourceReasonName);
#endif
#if NETFRAMEWORK
InvalidUdtException e = (InvalidUdtException)s_udtFactory.Invoke(null, [udtType, resourceReasonName]);
#else
InvalidUdtException e = InvalidUdtException.Create(udtType, resourceReasonName);
#endif

TraceExceptionAsReturnValue(e);
return e;
}

#endregion

static private void TraceException(string trace, Exception e)
Expand Down
4 changes: 4 additions & 0 deletions ...SqlClient/SqlDependencyUtils.AppDomain.cs → ...t/SqlDependencyUtils.AppDomain.netcore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#if NET

using System;

namespace Microsoft.Data.SqlClient
Expand All @@ -18,3 +20,5 @@ partial void SubscribeToAppDomainUnload()
}
}
}

#endif
0 ...SqlDependencyUtils.AssemblyLoadContext.cs → ...dencyUtils.AssemblyLoadContext.netcore.cs
View file
Open in desktop
File renamed without changes.
Loading