Skip to content
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

Fix native libs selection in System.IO.Ports package #106231

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

akoeplinger
Copy link
Member

The runtime.native.System.IO.Ports package depends on all the RID-specific packages, this causes an issue when publishing for a RID like linux-musl-x64 since the RID graph has a fallback path of linux-musl-x64 -> linux-x64 -> linux so the SDK would select the native libs from both the runtime.linux-musl-x64.runtime.native.System.IO.Ports and runtime.linux-x64.runtime.native.System.IO.Ports packages which causes a conflict.

To fix this we add _._ placeholder entries for all RIDs except the one where we have a native asset so the SDK ignores the non-matching ones.

Fixes #104710

The runtime.native.System.IO.Ports package depends on all the RID-specific packages, this causes an issue when publishing for a RID like `linux-musl-x64` since
the RID graph has a fallback path of `linux-musl-x64 -> linux-x64 -> linux` so the SDK would select the native libs from _both_ the `runtime.linux-musl-x64.runtime.native.System.IO.Ports` and `runtime.linux-x64.runtime.native.System.IO.Ports` packages which causes a conflict.

To fix this we add `_._` placeholder entries for all RIDs except the one where we have a native asset so the SDK ignores the non-matching ones.

Fixes dotnet#104710
@jkotas
Copy link
Member

jkotas commented Aug 10, 2024

cc @wfurt

Comment on lines 21 to 36
<PackageSupportedRID Include="android-arm" />
<PackageSupportedRID Include="android-arm64" />
<PackageSupportedRID Include="android-x64" />
<PackageSupportedRID Include="android-x86" />
<PackageSupportedRID Include="linux-arm" />
<PackageSupportedRID Include="linux-arm64" />
<PackageSupportedRID Include="linux-bionic-arm64" />
<PackageSupportedRID Include="linux-bionic-x64" />
<PackageSupportedRID Include="linux-musl-arm" />
<PackageSupportedRID Include="linux-musl-arm64" />
<PackageSupportedRID Include="linux-musl-x64" />
<PackageSupportedRID Include="linux-x64" />
<PackageSupportedRID Include="maccatalyst-arm64" />
<PackageSupportedRID Include="maccatalyst-x64" />
<PackageSupportedRID Include="osx-arm64" />
<PackageSupportedRID Include="osx-x64" />
Copy link
Member

Choose a reason for hiding this comment

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

I would like @ericstj to comment on this PR (as I'm currently out). IMO it would be better to not rely on a static list here as that requires more things to touch when adding a new runtime RID package.

From a given RID it should be possible to get the parent RIDs either via an SDK task or via a custom msbuild task.

Copy link
Member Author

Choose a reason for hiding this comment

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

We'd need the other way round, going from parent RID to child RIDs since we need to include those in the parent-RID-package. I'm not sure that will be easier. I can try basing it off of the runtime.<RID>.runtime.native.System.IO.Ports.proj files though.

Copy link
Member Author

Choose a reason for hiding this comment

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

pushed an update

<ItemGroup>
<!-- Extract supported RIDs from the .proj file names -->
<RIDSpecificProject Include="$(MSBuildThisFileDirectory)runtime.*.runtime.native.System.IO.Ports.proj" />
<PackageSupportedRID Include="@(RIDSpecificProject->'%(Filename)'->Replace('.runtime.native.System.IO.Ports', '')->Replace('runtime.', ''))" />
Copy link
Member

Choose a reason for hiding this comment

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

Clever way of finding the full set of RIDs. I was going to suggest regex property function - but this is fine and concise.

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM.

@akoeplinger akoeplinger merged commit 8d51dc9 into dotnet:main Aug 12, 2024
84 checks passed
@akoeplinger akoeplinger deleted the fix-io-ports branch August 12, 2024 17:51
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.IO.Ports.SerialPort not working on linux-musl-arm
5 participants