Fix native lib resolution for cross-arch publish (#205)#206
Open
JamestsaiTW wants to merge 3 commits into
Open
Fix native lib resolution for cross-arch publish (#205)#206JamestsaiTW wants to merge 3 commits into
JamestsaiTW wants to merge 3 commits into
Conversation
) Stop packing LightningDB.targets as a build asset. The custom target runs in projects that directly reference the package, where a consuming application's RuntimeIdentifier may not be available, and can copy a host-architecture binary into cross-RID publish output. Keep the standard runtimes/<rid>/native assets in the package so the .NET SDK selects the correct binary for the final application's target RID, including transitive PackageReference scenarios. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep LightningDB.targets in the package for packages.config consumers, but skip its host-architecture copy logic when PackageReference is in use so the SDK can resolve runtimes/<rid>/native assets for the final target RID. Repository projects explicitly opt in because they consume LightningDB through ProjectReference rather than the packed runtime assets. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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> |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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