[release/13.2] Fix SqlClient runtime asset layout on Unix#15709
[release/13.2] Fix SqlClient runtime asset layout on Unix#15709joperezr merged 16 commits intorelease/13.2from
Conversation
Ensure bundled/prebuilt app hosts prefer runtime-specific assets when laying out NuGet packages so Microsoft.Data.SqlClient loads correctly on macOS and Linux. Add a regression test covering the runtime-target selection behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15709Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15709" |
Ensure LayoutCommand chooses the matching native runtime target for the output root even when a generic native asset with the same file name is also present. Extend the regression test to cover the override behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes the prebuilt/bundled AppHost NuGet layout so runtime-specific assets are preferred when available (avoiding loading the wrong Microsoft.Data.SqlClient implementation on macOS/Linux), while also preserving structured runtimes/ and resource assets.
Changes:
- Add a
--runtime-identifier/--ridoption to theLayoutCommandand use it to prefer matchingruntimeTargetswhen copying assemblies/native assets. - Preserve
runtimes/subtree (and resources) in the layout output in addition to producing the probing-friendly flat root layout. - Plumb the current runtime identifier through the CLI restore/layout pipeline and add a regression test for runtime target selection + structured asset preservation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.RemoteHost.Tests/LayoutCommandTests.cs | Adds regression coverage for runtime-target preference and preserving runtimes/ + resource assets. |
| tests/Aspire.Hosting.RemoteHost.Tests/Aspire.Hosting.RemoteHost.Tests.csproj | Adds reference needed to call LayoutCommand from tests. |
| src/Aspire.Managed/NuGet/Commands/LayoutCommand.cs | Implements RID-aware target selection and copies runtime/resource/native assets preserving structure. |
| src/Aspire.Cli/Projects/PrebuiltAppHostServer.cs | Passes current RID into bundled NuGet restore/layout to pick correct runtime assets. |
| src/Aspire.Cli/NuGet/BundleNuGetService.cs | Threads RID into layout invocation and cache key to avoid cross-RID collisions. |
|
Do we need to embed the RID graph here? |
Add regression coverage for project.assets.json files that contain both a base target and a runtime-specific target so LayoutCommand's RID-based target selection is validated directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Derive portable RID aliases from the requested runtime identifier rather than the current process OS, fix Windows JSON escaping in the layout command tests, and add coverage for explicit RID fallback selection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@davidfowl I believe the current resolution is simple enough to handle the cases we care about right now. Should we instead import the logic from other dependencies to be complete? Sub-question: already or later, 13.2.x or 13.3? An option could be to add this simple fallback logic to 13.2.x and the full one in 13.3 |
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
A few observations on potential issues with the current implementation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
The code looks fine to me. I don't really understand this problem space so an SME should also give sign off.
|
I'm doing a comparison with the nuget logic in the dotnet-sdk. |
davidfowl
left a comment
davidfowl
left a comment
There was a problem hiding this comment.
Review: Comparison with SDK NuGet asset resolution logic
I compared the LayoutCommand approach in this PR against the SDK's asset resolution in dotnet/sdk (ResolvePackageAssets.cs, AssetsFileResolver.cs, DependencyContextBuilder.cs, LockFileExtensions.cs). The approach here is sound.
How the SDK handles this
The SDK resolves two lock file targets from project.assets.json:
- Compile-time target:
lockFile.GetTarget(framework, runtimeIdentifier: null)— no RID - Runtime target:
lockFile.GetTarget(framework, runtimeIdentifier)— with the project's RID (if specified)
NuGet restore already produces a target with RID-resolved assets baked in. NativeLibraries and RuntimeAssemblies in the runtime target are already correct for the platform.
For framework-dependent apps (no RID), the SDK copies RuntimeTargets preserving the runtimes/{rid}/native/ directory structure. At runtime, the .NET host walks the deps.json runtime fallback graph to pick the right RID folder. The SDK itself never flattens RID-specific assets to root.
Why the LayoutCommand approach diverges — and why that's correct
Aspire's AppHost server loads integration assemblies via flat probing paths, not deps.json-based resolution. It can't rely on the .NET host's RID probing. So the LayoutCommand needs to pre-resolve at layout time what the host would normally do at runtime:
BuildBestRuntimeTargetsByFileNamescores eachRuntimeTargetby RID proximity using the embeddedRuntimeIdentifierGraph.json(same graph the SDK/host uses)CopyNativeLibrariesflattens the best-matching native asset to root, and falls back to the second loop for packages that only ship RID-specific natives (the SqlClient case —NativeLibrariesis empty, assets live exclusively inruntimes/{rid}/native/)CopyRuntimeAssembliessimilarly overlays the best RID-specific managed assembly over the genericlib/one
This correctly mirrors what DependencyContextBuilder.GetRuntimeLibrary() does in the SDK — it groups RuntimeTargets by RID into NativeLibraryGroups/RuntimeAssemblyGroups, and the host resolves them via fallback chains. The LayoutCommand just does this resolution eagerly at file-copy time.
One observation
CopyRuntimeTargets (line 283) copies all runtime targets in structured form (output/runtimes/osx-arm64/native/...), while CopyNativeLibraries also copies the best-match flat to root. The structured copies may be unnecessary for Aspire's probing-path model — they'd only matter if something downstream expects runtimes/{rid}/ layout. Not a bug, just extra disk I/O. Worth a comment explaining whether both are intentionally needed or if the structured copy is defensive/future-proofing.
Verdict
The approach correctly adapts the SDK's RID resolution semantics for Aspire's flat-layout constraint. The use of the SDK's own RuntimeIdentifierGraph.json for fallback matching (rather than a hand-rolled heuristic) is the right call. Test coverage looks solid for the core scenarios.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
We don’t need our own RID graph anymore because we stopped doing RID resolution in LayoutCommand. Instead we rely on Nuget restore logic (@eerhardt suggested it). Before, layout only had generic package data plus runtimeTargets, so it had to answer: “for osx-arm64, which asset should I pick?” That required expanding RID fallbacks itself, which is why we embedded the graph. Now restore runs with the current RID. NuGet uses its RID graph during restore and writes a RID-specific target like net10.0/osx-arm64 into project.assets.json. That target already contains the resolved runtime and native assets to use. So layout just does: select target, copy listed files. NuGet owns RID fallback; our code no longer reimplements it. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sebastienros
changed the title
sebastienros
changed the title
|
🎬 CLI E2E Test Recordings — 53 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23827070469 |
joperezr
added
Servicing-approved
Description
Fixes bundled/prebuilt AppHost package layout so runtime-specific assets are preferred when available. This fixes
Microsoft.Data.SqlClientloading in the SQL Server scenario on macOS/Linux, where the previous flat layout could select the non-runtime-specific assembly and fail withPlatformNotSupportedException.This PR also includes the review follow-ups needed to make the fix more correct and maintainable:
Fixes #15670
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: