-
Notifications
You must be signed in to change notification settings - Fork 537
NativeDLLs: Fixes Conditionally include win-x64 native DLLs based on RuntimeIdentifier #5553
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
kirankumarkolli
merged 33 commits into
master
from
copilot/fix-unconditional-dll-copying
Apr 23, 2026
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
c99316b
Initial plan
Copilot 9b90613
Make win-x64 native DLLs conditional on Windows runtime
Copilot 9c8e4f5
Merge latest changes from master
Copilot c052b0a
Add unit tests for Microsoft.Azure.Cosmos.targets conditional DLL inc…
Copilot 6c8d151
Merge branch 'master' into copilot/fix-unconditional-dll-copying
kirankumarkolli 5c932d1
Replace XML parsing tests with integration tests for publish validation
Copilot c2460bf
Add CosmosClient instantiation in test projects to ensure SDK usage
Copilot b65f049
Add command tracing to publish tests for better diagnostics
Copilot 6ccd8be
Fixing the build errors
kirankumarkolli d690b7e
Fix .NET 6 compatibility and update condition to include empty Runtim…
Copilot 4c72670
Restore warning header, add documentation, and refactor tests with Da…
Copilot 5c3efb0
Add null check for Process.Start and fix stylecop warnings with docum…
Copilot d542a9f
Small styleop fixes
kirankumarkolli f1ffd62
Fix process handling and add DoNotParallelize attribute to tests
Copilot 7e9abb5
Remove unreachable code and add null-forgiving operator
Copilot 6f569c9
Fix CosmosTargetsPublishTests to use local NuGet package
kirankumarkolli 08b2ee0
Merge branch 'master' into copilot/fix-unconditional-dll-copying
kirankumarkolli 56667b7
Merge branch 'master' into copilot/fix-unconditional-dll-copying
kirankumarkolli 74befb4
Merge branch 'master' into copilot/fix-unconditional-dll-copying
kirankumarkolli 74fbc6e
Merge branch 'master' into copilot/fix-unconditional-dll-copying
kundadebdatta b96bb10
Address PR review comments: fix deadlock, add no-RID test, update TFM
kirankumarkolli 6286b22
Merge branch 'master' into copilot/fix-unconditional-dll-copying
kirankumarkolli 2f85656
Merge branch 'master' into copilot/fix-unconditional-dll-copying
kirankumarkolli 2c40593
Revert documentation changes to simpler approved version
Copilot 8d710e9
Restore original indentation for msvcp140, vcruntime140, vcruntime140…
Copilot ef37530
Address PR feedback: assert empty errors, simplify Program.cs, inline…
Copilot eb9e9a4
addressing the comment part
kirankumarkolli ef8b1b7
Tests: Refactors CosmosTargetsPublishTests to use async/await
kirankumarkolli bc56530
Tests: Adds publish output validation to prevent vacuous test passes
kirankumarkolli 1608b4f
Tests: Refactors process management into RunDotnetCommandAsync helper
kirankumarkolli ae02b3e
Tests: Refactors CreateTestProject and PublishProjectAsync into singl…
kirankumarkolli 7afaa25
Merge branch 'master' into copilot/fix-unconditional-dll-copying
kirankumarkolli ce0f87f
Renaming the file
kirankumarkolli 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
268 changes: 268 additions & 0 deletions
268
...ure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/MSBuild/CosmosTargetsInteropPublishTests.cs
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,268 @@ | ||
| //------------------------------------------------------------ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| //------------------------------------------------------------ | ||
|
|
||
| namespace Microsoft.Azure.Cosmos.Tests.MSBuild | ||
| { | ||
| using System; | ||
| using System.Diagnostics; | ||
| using System.IO; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
|
|
||
| /// <summary> | ||
| /// Integration tests that verify Windows native DLLs are only copied when publishing | ||
| /// for Windows RuntimeIdentifiers, and not for Linux/macOS targets. | ||
| /// These tests pack the SDK into a local NuGet package to properly test the .targets file behavior. | ||
| /// </summary> | ||
| [TestClass] | ||
|
kirankumarkolli marked this conversation as resolved.
|
||
| [TestCategory("LongRunning")] | ||
| public class CosmosTargetsInteropPublishTests | ||
| { | ||
| private static string testProjectsRoot; | ||
| private static string localNugetPackagePath; | ||
| private static string packageVersion; | ||
| private static readonly string[] WindowsNativeDlls = new[] | ||
| { | ||
| "Microsoft.Azure.Cosmos.ServiceInterop.dll", | ||
| "Cosmos.CRTCompat.dll", | ||
| "msvcp140.dll", | ||
| "vcruntime140.dll", | ||
| "vcruntime140_1.dll" | ||
| }; | ||
|
|
||
| [ClassInitialize] | ||
| public static async Task ClassInitialize(TestContext _) | ||
| { | ||
| testProjectsRoot = Path.Combine(Path.GetTempPath(), "CosmosTargetsTests_" + Guid.NewGuid().ToString("N")); | ||
| Directory.CreateDirectory(testProjectsRoot); | ||
|
|
||
| // Create local NuGet package from the SDK | ||
| await CreateLocalNuGetPackageAsync(); | ||
| } | ||
|
|
||
| [ClassCleanup] | ||
| public static void ClassCleanup() | ||
| { | ||
| if (Directory.Exists(testProjectsRoot)) | ||
| { | ||
| try | ||
| { | ||
| Directory.Delete(testProjectsRoot, recursive: true); | ||
| } | ||
| catch | ||
| { | ||
| // Ignore cleanup errors | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests that Windows native DLLs are not copied when publishing for non-Windows platforms. | ||
| /// </summary> | ||
| /// <param name="runtimeIdentifier">The runtime identifier to test (e.g., linux-x64, osx-x64).</param> | ||
| [TestMethod] | ||
| [DataRow("linux-x64")] | ||
| [DataRow("linux-arm64")] | ||
| [DataRow("osx-x64")] | ||
|
kirankumarkolli marked this conversation as resolved.
|
||
| [DataRow("osx-arm64")] | ||
| public async Task Publish_WithNonWindowsRuntimeIdentifier_DoesNotCopyWindowsDlls(string runtimeIdentifier) | ||
| { | ||
| string publishPath = await this.CreateAndPublishTestProjectAsync($"NonWinTest_{runtimeIdentifier}", runtimeIdentifier); | ||
|
|
||
| this.AssertWindowsDllsNotPresent(publishPath, runtimeIdentifier); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests that Windows native DLLs are copied when publishing for Windows platforms. | ||
| /// </summary> | ||
| /// <param name="runtimeIdentifier">The runtime identifier to test (e.g., win-x64, win-x86).</param> | ||
| [TestMethod] | ||
| [DataRow("win-x64")] | ||
| [DataRow("win-x86")] | ||
| [DataRow("win-arm64")] | ||
| public async Task Publish_WithWindowsRuntimeIdentifier_CopiesWindowsDlls(string runtimeIdentifier) | ||
| { | ||
| string publishPath = await this.CreateAndPublishTestProjectAsync($"WinTest_{runtimeIdentifier}", runtimeIdentifier); | ||
|
|
||
| this.AssertWindowsDllsPresent(publishPath, runtimeIdentifier); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Tests that Windows native DLLs are copied when publishing without a RuntimeIdentifier, | ||
| /// which is the most common developer scenario (regular 'dotnet publish' without -r). | ||
| /// </summary> | ||
| [TestMethod] | ||
| public async Task Publish_WithoutRuntimeIdentifier_CopiesWindowsDlls() | ||
| { | ||
| string publishPath = await this.CreateAndPublishTestProjectAsync("NoRidTest", runtimeIdentifier: null); | ||
|
|
||
| this.AssertWindowsDllsPresent(publishPath, "no RuntimeIdentifier"); | ||
| } | ||
|
|
||
| private static async Task CreateLocalNuGetPackageAsync() | ||
| { | ||
| string repoRoot = GetRepositoryRoot(); | ||
| string cosmosProjectPath = Path.Combine(repoRoot, "Microsoft.Azure.Cosmos", "src", "Microsoft.Azure.Cosmos.csproj"); | ||
| string packOutputDir = Path.Combine(testProjectsRoot, "nuget-packages"); | ||
| Directory.CreateDirectory(packOutputDir); | ||
|
|
||
| // Use a unique version to avoid cache conflicts | ||
| packageVersion = $"99.0.0-test.{DateTime.UtcNow:yyyyMMddHHmmss}"; | ||
|
|
||
| await RunDotnetCommandAsync( | ||
| $"pack \"{cosmosProjectPath}\" -c Release -o \"{packOutputDir}\" /p:Version={packageVersion} /p:PackageVersion={packageVersion}", | ||
| timeoutMinutes: 10); | ||
|
|
||
| localNugetPackagePath = packOutputDir; | ||
| } | ||
|
|
||
| private async Task<string> CreateAndPublishTestProjectAsync(string projectName, string runtimeIdentifier) | ||
| { | ||
| string projectDir = Path.Combine(testProjectsRoot, projectName); | ||
| Directory.CreateDirectory(projectDir); | ||
|
|
||
| string projectFile = Path.Combine(projectDir, $"{projectName}.csproj"); | ||
| string programFile = Path.Combine(projectDir, "Program.cs"); | ||
| string nugetConfigFile = Path.Combine(projectDir, "nuget.config"); | ||
|
|
||
| // Create nuget.config to use local package source | ||
| File.WriteAllText(nugetConfigFile, $@"<?xml version=""1.0"" encoding=""utf-8""?> | ||
| <configuration> | ||
| <packageSources> | ||
| <clear /> | ||
| <add key=""local"" value=""{localNugetPackagePath}"" /> | ||
| <add key=""nuget.org"" value=""https://api.nuget.org/v3/index.json"" /> | ||
| </packageSources> | ||
| </configuration>"); | ||
|
kirankumarkolli marked this conversation as resolved.
|
||
|
|
||
| // Create a simple console app project that references the local NuGet package | ||
| File.WriteAllText(projectFile, $@"<Project Sdk=""Microsoft.NET.Sdk""> | ||
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <TargetFramework>net8.0</TargetFramework> | ||
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include=""Microsoft.Azure.Cosmos"" Version=""{packageVersion}"" /> | ||
| <PackageReference Include=""Newtonsoft.Json"" Version=""13.0.3"" /> | ||
| </ItemGroup> | ||
| </Project>"); | ||
|
|
||
| // Create a minimal Program.cs | ||
| File.WriteAllText(programFile, @"System.Console.WriteLine(""Test app for verifying Cosmos SDK package behavior"");"); | ||
|
|
||
| // Publish the project | ||
| string publishDir = Path.Combine(projectDir, "bin", "publish", runtimeIdentifier ?? "no-rid"); | ||
| string ridArgument = runtimeIdentifier != null ? $"-r {runtimeIdentifier} --self-contained false" : string.Empty; | ||
| await RunDotnetCommandAsync( | ||
| $"publish \"{projectFile}\" -c Release -o \"{publishDir}\" {ridArgument}".Trim(), | ||
| timeoutMinutes: 5); | ||
|
|
||
| return publishDir; | ||
| } | ||
|
|
||
| private void AssertWindowsDllsNotPresent(string publishPath, string runtimeIdentifier) | ||
| { | ||
| Assert.IsTrue(Directory.Exists(publishPath), $"Publish directory does not exist: {publishPath}"); | ||
| this.AssertPublishOutputIsValid(publishPath, runtimeIdentifier); | ||
|
|
||
| foreach (string dll in WindowsNativeDlls) | ||
| { | ||
| string dllPath = Path.Combine(publishPath, dll); | ||
| Assert.IsFalse(File.Exists(dllPath), | ||
| $"Windows native DLL '{dll}' should NOT be present when publishing for {runtimeIdentifier}, but was found at: {dllPath}"); | ||
| } | ||
| } | ||
|
|
||
| private void AssertWindowsDllsPresent(string publishPath, string runtimeIdentifier) | ||
| { | ||
| Assert.IsTrue(Directory.Exists(publishPath), $"Publish directory does not exist: {publishPath}"); | ||
| this.AssertPublishOutputIsValid(publishPath, runtimeIdentifier); | ||
|
|
||
| foreach (string dll in WindowsNativeDlls) | ||
| { | ||
| string dllPath = Path.Combine(publishPath, dll); | ||
| Assert.IsTrue(File.Exists(dllPath), | ||
| $"Windows native DLL '{dll}' SHOULD be present when publishing for {runtimeIdentifier}, but was NOT found at: {dllPath}"); | ||
| } | ||
| } | ||
|
|
||
| private void AssertPublishOutputIsValid(string publishPath, string runtimeIdentifier) | ||
| { | ||
| string[] publishedFiles = Directory.GetFiles(publishPath); | ||
| Assert.IsTrue(publishedFiles.Length > 0, | ||
| $"Publish directory is empty for {runtimeIdentifier}. Publish may have silently failed: {publishPath}"); | ||
|
|
||
| string sdkDllPath = Path.Combine(publishPath, "Microsoft.Azure.Cosmos.Client.dll"); | ||
| Assert.IsTrue(File.Exists(sdkDllPath), | ||
| $"Microsoft.Azure.Cosmos.Client.dll not found in publish output for {runtimeIdentifier}. " + | ||
| $"Publish may not have included the SDK package correctly: {publishPath}"); | ||
| } | ||
|
|
||
| private static string GetRepositoryRoot() | ||
| { | ||
| string currentDir = AppDomain.CurrentDomain.BaseDirectory; | ||
|
|
||
| while (currentDir != null && !File.Exists(Path.Combine(currentDir, "Microsoft.Azure.Cosmos.sln"))) | ||
| { | ||
| DirectoryInfo parent = Directory.GetParent(currentDir); | ||
| if (parent == null) | ||
| { | ||
| break; | ||
| } | ||
| currentDir = parent.FullName; | ||
| } | ||
|
|
||
| Assert.IsNotNull(currentDir, "Could not find repository root"); | ||
| return currentDir; | ||
| } | ||
|
|
||
| private static async Task RunDotnetCommandAsync(string arguments, int timeoutMinutes = 5) | ||
| { | ||
| ProcessStartInfo processInfo = new ProcessStartInfo | ||
| { | ||
| FileName = "dotnet", | ||
| Arguments = arguments, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
|
|
||
| string commandLine = $"dotnet {arguments}"; | ||
| Console.WriteLine($"Executing: {commandLine}"); | ||
|
|
||
| Process process = Process.Start(processInfo); | ||
| if (process == null) | ||
| { | ||
| Assert.Fail($"Failed to start process: {commandLine}"); | ||
| } | ||
|
|
||
| using (process) | ||
| { | ||
| Task<string> outputTask = process.StandardOutput.ReadToEndAsync(); | ||
| Task<string> errorTask = process.StandardError.ReadToEndAsync(); | ||
|
|
||
| bool exited = process.WaitForExit((int)TimeSpan.FromMinutes(timeoutMinutes).TotalMilliseconds); | ||
| if (!exited) | ||
| { | ||
| process.Kill(); | ||
| Assert.Fail($"Command timed out after {timeoutMinutes} minutes.\nCommand: {commandLine}"); | ||
| } | ||
|
|
||
| string output = await outputTask; | ||
| string error = await errorTask; | ||
|
|
||
| if (process.ExitCode != 0) | ||
| { | ||
| Assert.Fail($"Command failed with exit code {process.ExitCode}.\nCommand: {commandLine}\nOutput: {output}\nError: {error}"); | ||
| } | ||
|
|
||
| Assert.IsTrue(string.IsNullOrEmpty(error), $"Command had unexpected error output.\nCommand: {commandLine}\nError: {error}"); | ||
| Console.WriteLine($"Command succeeded: {commandLine}"); | ||
| } | ||
| } | ||
| } | ||
| } | ||
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.