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

[release/6.0] Microsoft.NETCore.Platforms: support adding rids with '-' in the base part (backport #84413) #84442

Conversation

ayakael
Copy link
Contributor

@ayakael ayakael commented Apr 6, 2023

Backport of #84413

Per @tmds

Currently when trying to add a rid like 'linux-musl-x64' the rid is not understood to be base = 'linux-musl', arch = 'x64'.
Instead the parser considers a potential optional qualifier. This causes the rid to be parsed as base = 'linux', arch = 'musl', and qualifier = 'x64'.
We know the rids being added won't have a qualifier. If we take this into account while parsing, we can parse the rid correctly.

Customer impact

#77508 was merged, but its installer counterpart (dotnet/installer#14816) was NAKed due to safety concerns. This revealed a limit with Microsoft.NETCore.Platforms parsing of RID. Thus, build of source-build on linux-musl platforms is currently broken. A few approches on installer side can be implemented as workarounds, but ideally the fix should be on runtime side.

This also unblocks dotnet/installer#13074.

Testing

  • unit tests in ci
  • manual build of runtime on Alpine Linux when OutputRid=linux-musl-x64 and Source

Risk

Low, there were limited changes in that code between release/6.0 and main.

/cc: @ericst @tmds @omajid

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Apr 6, 2023
@ghost
Copy link

ghost commented Apr 6, 2023

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

Issue Details

Backport of #84413

Per @tmds

Currently when trying to add a rid like 'linux-musl-x64' the rid is not understood to be base = 'linux-musl', arch = 'x64'.

Instead the parser considers a potential optional qualifier. This causes the rid to be parsed as base = 'linux', arch = 'musl', and qualifier = 'x64'.

We know the rids being added won't have a qualifier. If we take this into account while parsing, we can parse the rid correctly.

Customer impact

#77508 was merged, but its installer counterpart (dotnet/installer#14816) was NAKed due to safety concerns. This revealed a limit with Microsoft.NETCore.Platforms parsing of RID. Thus, build of source-build on linux-musl platforms is currently broken. A few approches on installer side can be implemented as workarounds, but ideally the fix should be on runtime side.

This also unblocks dotnet/installer#13074.

Testing

  • unit tests in ci
  • manual build of runtime on Alpine Linux when OutputRid=linux-musl-x64 and Source

Risk

Low, there were limited changes in that code between release/6.0 and main.

/cc: @ericst @tmds @omajid

Author: ayakael
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@ayakael ayakael force-pushed the backport/pr-84413-to-release/6.0 branch from d2c6e3b to 94dc504 Compare April 6, 2023 19:32
@ayakael ayakael changed the base branch from release/6.0 to release/6.0-staging April 6, 2023 21:13
@ayakael ayakael changed the title [release/6.0] Microsoft.NETCore.Platforms: support adding rids with '-' in the base part (backport #84413) [release/6.0-staging] Microsoft.NETCore.Platforms: support adding rids with '-' in the base part (backport #84413) Apr 6, 2023
@ayakael ayakael force-pushed the backport/pr-84413-to-release/6.0 branch from 021a7a6 to 6c12423 Compare April 6, 2023 21:13
@ayakael ayakael changed the title [release/6.0-staging] Microsoft.NETCore.Platforms: support adding rids with '-' in the base part (backport #84413) [release/6.0] Microsoft.NETCore.Platforms: support adding rids with '-' in the base part (backport #84413) Apr 6, 2023
@tmds
Copy link
Member

tmds commented Apr 7, 2023

#77508 was merged, but its installer counterpart (dotnet/installer#14816) was NAKed due to safety concerns.

For some more context:

This change set had two goals: eliminate portable build of runtime (in installer), and update the runtime graph with the target rid (in runtime).

Because only the runtime part got merged, we're now in a situation where the portable build ensures the target rid (which is the portable rid) is in the graph.
And, for the linux-musl portable rid, there is an issue due to the - in its name. This PR is fixing that issue.

I think @ayakael has been patching his builds to work around this issue.

And the issue has now surfaced again with @omajid working on adding a source-build 6.0 CI leg for Alpine: dotnet/installer#13074.

@ayakael ayakael closed this Apr 7, 2023
@ayakael ayakael reopened this Apr 7, 2023
@ayakael ayakael force-pushed the backport/pr-84413-to-release/6.0 branch from 6c12423 to d6d9fe8 Compare April 7, 2023 12:53
@carlossanlop
Copy link
Member

carlossanlop commented Apr 10, 2023

Today is code complete for the May Release. If we want this fix to get included in that release, please get the PR ready to merge before 4pm PT today:

  • Get a Tactics approval. @wfurt can you please send the email and champion this? tell-mode.
  • Confirm the CI failures are unrelated to this change. Edit: Failures unrelated.
  • Get a sign-off from an area owner. @wfurt
  • The change affects the Microsoft.NETCore.Platforms OOB package. Make sure the required OOB package authoring changes are included in the PR. Not needed for June Release if this PR gets merged too (it already includes the OOB changes): Add Ubuntu 22.10 kinetic kudu RIDs for .NET 6 #84983

Otherwise, this will have to go into the June Release.

@ViktorHofer ViktorHofer added the Servicing-consider Issue for next servicing release review label May 15, 2023
@carlossanlop
Copy link
Member

I'm treating this as tell-mode since the affected cs file is a build task, so not customer facing. I notified Tactics anyway just so they were informed.

@carlossanlop carlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels May 16, 2023
@carlossanlop carlossanlop merged commit 52b3b98 into dotnet:release/6.0-staging May 16, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries community-contribution Indicates that the PR has been added by a community member Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants