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 8 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
53 changes: 53 additions & 0 deletions build2.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?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>
<DotnetCommand>$([System.Text.RegularExpressions.Regex]::Replace($(DotnetCommand), "\s{2,}", " "))</DotnetCommand>
</PropertyGroup>
<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
</DotnetCommand>
<DotnetCommand>$([System.Text.RegularExpressions.Regex]::Replace($(DotnetCommand), "\s{2,}", " "))</DotnetCommand>
</PropertyGroup>
<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
65 changes: 50 additions & 15 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,12 +1,14 @@
<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>

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

Expand All @@ -15,41 +17,74 @@
<!--<TargetOs>Windows_NT</TargetOs>-->

<!-- 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="'$(TargetOs.ToUpper())' == 'UNIX'">$(DefineConstants);_UNIX</DefineConstants>
<DefineConstants Condition="'$(TargetOs.ToUpper())' == 'WINDOWS_NT'">$(DefineConstants);_WINDOWS</DefineConstants>
</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="'$(TargetOs.ToUpper())' == 'WINDOWS_NT'">$(TargetFrameworks);net462</TargetFrameworks>
</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)/$(TargetOs.ToLower())/</OutputPath>
</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="'$(TargetOs.ToUpper())' == 'WINDOWS_NT' AND '$(TargetFramework)' != 'net462'" />
</ItemGroup>

<!-- References ====================================================== -->
<!-- References that apply to all target frameworks -->
<ItemGroup>
<!-- References that apply to all target frameworks -->
<PackageReference Include="Azure.Core" />
<PackageReference Include="Azure.Identity" />
<PackageReference Include="Microsoft.Bcl.Cryptography" />
<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>

<!-- References that only apply to net462 -->
<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<!-- References that only apply to net462 -->
<Reference Include="System.Configuration" />
<Reference Include="System.EnterpriseServices" />
<Reference Include="System.Transactions" />
Copy link
Contributor

@mdaigle mdaigle Dec 11, 2025

Choose a reason for hiding this comment

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

System.Text.Json PackageReference is missing here. Also missing a number of References.

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.

The system.text.json override was explicitly removed in #3686

There are a few others that are missing ... notably SNI. I will investigate those. But it's worth noting that CI builds are passing 🤷‍♂️

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.

OH DUH, yeah we're not even building it in the CI builds 🤦‍♂️
Uhm, but yeah, it all works locally for me ... I will still investigate

Copy link
Contributor

@mdaigle mdaigle Dec 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

System.Text.Json was removed for .NET because it's included in the standard library, but we still need it to be package provided for .NET Framework.

<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.Memory" />
<PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" />
<!-- References that only apply to netcore -->
<ItemGroup Condition="'$(TargetFramework)' != 'net462'">
<PackageReference Include="Microsoft.SqlServer.Server" />
<PackageReference Include="System.Configuration.ConfigurationManager" />

<!-- References that only apply to netcore < 10 -->
<PackageReference Include="System.Buffers"
Condition="$([MSBuild]::VersionLessThan($([MSBuild]::GetTargetFrameworkVersion($(TargetFramework))), '10.0'))" />
<PackageReference Include="System.Memory"
Condition="$([MSBuild]::VersionLessThan($([MSBuild]::GetTargetFrameworkVersion($(TargetFramework))), '10.0'))" />
<PackageReference Include="System.Runtime.InteropServices.RuntimeInformation"
Condition="$([MSBuild]::VersionLessThan($([MSBuild]::GetTargetFrameworkVersion($(TargetFramework))), '10.0'))" />
</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