Skip to content

[LoongArch64] Add proj to generate nupkg runtime.linux-loongarch64.runtime.native.System.IO.Ports#106850

Merged
akoeplinger merged 1 commit into
dotnet:mainfrom
chenguohui:la64-ioports
Aug 23, 2024
Merged

[LoongArch64] Add proj to generate nupkg runtime.linux-loongarch64.runtime.native.System.IO.Ports#106850
akoeplinger merged 1 commit into
dotnet:mainfrom
chenguohui:la64-ioports

Conversation

@chenguohui

Copy link
Copy Markdown
Contributor

There is missing the generation of runtime.linux-loongarch64.runtime.native.System.IO.Ports nupkg on LoongArch. Add this for fix it.

@shushanhf

…ntime.native.System.IO.Ports

Change-Id: I62eb691a0818b73333999f0b8a61b42bdb9d4da7
@ghost ghost added the area-System.IO label Aug 23, 2024
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Aug 23, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

@shushanhf shushanhf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM
Thanks

@jkotas Could you please review this PR?
Thanks!

@jkotas

jkotas commented Aug 23, 2024

Copy link
Copy Markdown
Member

@akoeplinger @ericstj Could you please review this change? I am not sure whether it is going to work given how we construct the package.

@akoeplinger

Copy link
Copy Markdown
Member

yes this works. note that since we don't build this RID on our official builds the package needs to be built by the distributor.

@akoeplinger

Copy link
Copy Markdown
Member

/azp run runtime-dev-innerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@am11

am11 commented Aug 23, 2024

Copy link
Copy Markdown
Member

Is there a way to avoid separate files per rid? e.g. with a list in a single file as we have one in src/installer/pkg/projects/netcoreappRIDs.prop

@akoeplinger

Copy link
Copy Markdown
Member

afaik that would need a .pkgproj which we want to get rid of.

@akoeplinger

Copy link
Copy Markdown
Member

/ba-g test failures are not related to System.IO.Ports.

@akoeplinger akoeplinger merged commit ac63269 into dotnet:main Aug 23, 2024
@am11

am11 commented Aug 23, 2024

Copy link
Copy Markdown
Member

afaik that would need a .pkgproj which we want to get rid of.

It can use <MSBuild task using batching likeTargetName=%(Rid->Identity) with a single .proj file, where Rid is an ItemGroup. I'll try; as we will need to add linux-riscv64, freebsd-x64/arm64 and illumos-x64 as well.

@chenguohui chenguohui deleted the la64-ioports branch August 23, 2024 11:01
@chenguohui

Copy link
Copy Markdown
Contributor Author

Appreciate for your replies and effort.

@jkotas

jkotas commented Aug 23, 2024

Copy link
Copy Markdown
Member

yes this works. note that since we don't build this RID on our official builds the package needs to be built by the distributor.

I do not think it works. The main package references all platform-specific packages unconditionally. It means that all platform-specific packages have to exist. It makes it impossible for them to support platforms outside the official build matrix. (Also, it is sub-optimal user experience since one has to download a lot more than what's actually needed. runtime.json-based packages do not have this problem.)

Repro:

  • dotnet new console
  • Copy over https://github.com/dotnet/runtime/blob/main/NuGet.config to get the daily feeds
  • Add <PackageReference Include="System.IO.Ports" Version="9.0.0-rc.1.24423.2" /> to the project. This is the first daily build with this change.
  • dotnet restore

Result:

C:\repro>dotnet restore
  Determining projects to restore...
C:\repro\repro.csproj : error NU1101: Unable to find package runtime.linux-loongarch64.runtime.native.System.IO.Ports.
No packages exist with this id in source(s): C:\Program Files\dotnet\library-packs, dotnet-eng, dotnet-libraries, dotne
t-libraries-transport, dotnet-public, dotnet-tools, dotnet10, dotnet10-transport, dotnet9, dotnet9-transport
  Failed to restore C:\repro\repro.csproj (in 7.06 sec).

@am11

am11 commented Aug 23, 2024

Copy link
Copy Markdown
Member

This package includes all runtimes as dependencies instead of following the native assets conventionally to only restore the required runtime.

image

that forces it to restore all native runtime.* packages on a given system.

@jkotas

jkotas commented Aug 23, 2024

Copy link
Copy Markdown
Member

We build the RID-specific System.IO.Ports assets into separate packages to avoid multi-machine build joins. It still follows the conventions described in the doc. Introducing a build join to merge everything into a single package would not really solve the loongarch issue.

dotnet restore tries to download RID-specific assets for all possible RIDs by default. I do not think the convention described in this doc helps with downloading only what one actually needs. The problem is in the design of the default dotnet build experience.

@am11

am11 commented Aug 23, 2024

Copy link
Copy Markdown
Member

Ah, I thought by convention it creates native package per RID, only that it doesn't add them as dependencies (i.e. no <dependencies> node created in nuspec), but rather let the nuget / dotnet-restore infra pull only the corresponding runtime based on the naming convention (runtime.$RID.$lower-case-name).

Perhaps <group> element in nuspec (https://learn.microsoft.com/en-us/nuget/reference/nuspec#dependency-groups) can add support for runtimeIdentifier attribute.

@akoeplinger

Copy link
Copy Markdown
Member

yeah unfortunately the runtime.json approach only works well for packages that are used in self-contained publish scenarios or when RuntimeIdentifier is set...

I don't think we have a good way to support architectures we don't build ourselves until nuget implements RID-specific dependencies: NuGet/Home#10571

One workaround I can think of is to tell users to manually reference the runtime.linux-loongarch64.runtime.native.System.IO.Ports package the distributor builds, this will make sure the native lib is getting copied.

@jkotas

jkotas commented Aug 25, 2024

Copy link
Copy Markdown
Member

NuGet/Home#10571

There are more problems beyond what is touched on in this proposal:

  • How does runtime.linux-loongarch64.runtime.native.System.IO.Ports get published? It cannot be published by the non-Microsoft distributor on nuget.org.
  • How are cross-platform apps built and published? Building these apps includes every available RID today.

@shushanhf

Copy link
Copy Markdown
Contributor
  • How does runtime.linux-loongarch64.runtime.native.System.IO.Ports get published? It cannot be published by the non-Microsoft distributor on nuget.org.

Right now it's published by ourself.

  • How are cross-platform apps built and published? Building these apps includes every available RID today.

If you must build dotnet by official released OS-image, you can't build and publish dotnet for LoongArch64.
But if you construct a new OS-image by all the latest upstream's repositories, it is OK.
I think it is very near to get the OS-image released by official.

@am11

am11 commented Aug 26, 2024

Copy link
Copy Markdown
Member
  • How does runtime.linux-loongarch64.runtime.native.System.IO.Ports get published? It cannot be published by the non-Microsoft distributor on nuget.org.

It is same as non-portable RIDs, anyone can publish runtime-RID.PackageID. There is no restriction who has published the native subpackage but we can always open a dispute with them if there is a misuse.

https://learn.microsoft.com/en-us/nuget/nuget-org/id-prefix-reservation - perhaps this facility can be extended to support “suffix” as well if, e.g. the naming (runtime-RID.PackageID) is standardized.

@github-actions github-actions Bot locked and limited conversation to collaborators Sep 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-loongarch64 area-System.IO community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants