Fix native lib resolution for cross-arch publish (#205)#206
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts how native LMDB binaries are copied/resolved so dotnet publish -r <rid> uses the correct target-RID native asset instead of accidentally copying a host-architecture binary via a custom MSBuild target.
Changes:
- Gate
LightningDBIncludeNativeDllso it only runs for legacy non-PackageReferenceconsumers, or when explicitly opted in. - Add
LightningDBEnableNativeCopyTarget=trueto internalProjectReferenceconsumers so local execution still gets native binaries copied. - Update
LightningDB.targetswith guidance about relying on SDK RID-based native asset resolution forPackageReferenceconsumers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/SecondProcess/SecondProcess.csproj | Opts into legacy native copy target for ProjectReference-based local execution. |
| src/LightningDB/LightningDB.targets | Gates the legacy copy target to avoid overriding SDK RID-based native asset resolution for PackageReference consumers. |
| src/LightningDB.Tests/LightningDB.Tests.csproj | Opts into legacy native copy target for tests when using ProjectReference. |
| src/LightningDB.Benchmarks/LightningDB.Benchmarks.csproj | Opts into legacy native copy target for benchmarks when using ProjectReference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <LightningDBTargetRuntimeRelativePath Condition=" '$(LightningDBTargetRuntimeRelativePath)' == '' ">\..\</LightningDBTargetRuntimeRelativePath> | ||
| </PropertyGroup> | ||
| <Target Name="LightningDBIncludeNativeDll" AfterTargets="BeforeResolveReferences"> | ||
| <!-- For PackageReference consumers, the SDK resolves runtimes/{rid}/native assets for the app's RID; set LightningDBEnableNativeCopyTarget=true to force the legacy copy behavior (e.g. packages.config or ProjectReference builds). --> |
| <OutputType>Exe</OutputType> | ||
| <TargetFrameworks>net8.0;net9.0;net10.0</TargetFrameworks> | ||
| <LangVersion>13</LangVersion> | ||
| <LightningDBEnableNativeCopyTarget>true</LightningDBEnableNativeCopyTarget> |
| <PackageId>LightningDB.Tests</PackageId> | ||
| <GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles> | ||
| <IsTestProject>true</IsTestProject> | ||
| <LightningDBEnableNativeCopyTarget>true</LightningDBEnableNativeCopyTarget> |
| <OutputType>Exe</OutputType> | ||
| <TargetFramework>net10.0</TargetFramework> | ||
| <LangVersion>13</LangVersion> | ||
| <LightningDBEnableNativeCopyTarget>true</LightningDBEnableNativeCopyTarget> |
|
Thanks for the clear diagnosis and verification work — I agree this is a real bug. The custom target selects the host OS/arch and can win over the SDK’s runtimes/{rid}/native assets during dotnet publish -r …, including the transitive case (App → ProjectReference → PackageReference(LightningDB)). I’m less inclined to disable LightningDBIncludeNativeDll for all PackageReference consumers, though. We multi-target netstandard2.0, so .NET Framework apps still consume this package. Those often have no RuntimeIdentifier, may be AnyCPU (sometimes with Prefer32Bit), and historically relied on a build/*.targets copy to get lmdb next to the output. Turning the target off for every PackageReference project fixes Core/SDK publish, but risks regressing that Framework path. I’d prefer to fix the targets file so one mechanism stays correct in both worlds. Local checks suggest the selection policy needs two parts:
Copy only runtimes/$(effectiveRid)/native/* (with a small alias for osx-x64 → our existing runtimes/osx/ layout). Drop or hard-opt-in the “Windows Arm64 host → win-x64” Parallels convenience path so a real win-arm64 RID isn’t overridden.
RID-first alone is not enough for the transitive repro. The targets import into the direct package consumer. Intermediate class libraries usually build with empty RuntimeIdentifier, so a host-arch flat CopyToOutputDirectory still runs and can poison the final app’s publish output even when the app is published with -r . |
Select the native runtime asset using RuntimeIdentifier, then PlatformTarget, Prefer32Bit, then the build host. Map osx-x64 to the package's osx layout. Skip host-native flat copies for RID-less class libraries so they cannot poison cross-RID publish of a consuming app. Fixes CoreyKaylor#205
fc213fa to
4924dac
Compare
Problem
LightningDB.targets'sLightningDBIncludeNativeDlltarget selects the native lmdb/liblmdb binary based on the build machine's OS/architecture (RuntimeInformation.OSArchitecture/MSBuild.IsOsPlatform), instead of the targetRuntimeIdentifier. This breaksdotnet publish -r <rid>when publishing for a different architecture/OS than the host (issue #205).Fix
The .NET SDK already resolves native binaries correctly per the target RID via the standard NuGet
runtimes/{rid}/nativepackage layout +.deps.jsonruntimeTargets, including in transitivePackageReferencescenarios (App -> ProjectReference -> Library -> PackageReference(LightningDB)), where the custom target's host-arch copy could otherwise win the SDK's asset conflict resolution and copy the wrong binary.To fix this while preserving compatibility for legacy
packages.config/ non-SDK-style consumers (the original reason this target was added, perbe15808, fornet45support), theLightningDBIncludeNativeDlltarget now only runs when:$(LightningDBEnableNativeCopyTarget)is explicitly set totrue(opt-in, used by this repo's ownProjectReferenceconsumers:LightningDB.Tests,SecondProcess,LightningDB.Benchmarks), orPackageReference(i.e.$(RestoreProjectStyle) != 'PackageReference'and no@(PackageReference)items) — preserving the original copy-to-output behavior forpackages.configconsumers.Modern
PackageReferenceconsumers now rely entirely on the SDK's built-in RID-based native asset resolution, which correctly handles the target RID (not the build machine's) in all publish scenarios, including transitive references.Verification
App -> Library(ProjectReference) -> PackageReference(LightningDB)), published with-r win-x86,-r win-arm64,-r linux-arm64,-r osx-x64on a win-x64 host — SHA256 hash comparison confirmed each publish output contained the correct RID-specific native binary (not the host's).dotnet packverified the packed nupkg still containsbuild/LightningDB.targetsand all native runtime assets unchanged.packages.configconsumer (-p:RestoreProjectStyle=PackagesConfig) confirmed the copy-to-output behavior still triggers correctly.dotnet testonLightningDB.Tests(net8.0): 137/138 passing (1 pre-existing unrelated failure that also reproduces onmain).LightningDB.Tests/SecondProcess/LightningDB.Benchmarks(which opt in viaLightningDBEnableNativeCopyTarget=true) still get native DLLs copied for local execution.PackageReferencescenario — all passed..NET TestsCI workflow (ubuntu-latest/windows-latest/macOS-latest) passes on this branch.Fixes #205