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

Linux and Linux-MUSL Package artifacts getting produced with duplicate RIDs #45027

Closed
MattGal opened this issue Nov 20, 2020 · 12 comments
Closed
Assignees
Labels
area-Infrastructure untriaged New issue has not been triaged by the area owner

Comments

@MattGal
Copy link
Member

MattGal commented Nov 20, 2020

Initially investigated as an AzDO issue via https://github.com/dotnet/core-eng/issues/11472 (see for more context), it looks like linux and linux-musl are getting conflated in package naming.

Seems to have been introduced in #43804. @safern already has context, so assigning to him.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-PAL-coreclr untriaged New issue has not been triaged by the area owner labels Nov 20, 2020
@safern
Copy link
Member

safern commented Nov 20, 2020

Thanks, @MattGal -- I'll put up a fix soon.

FYI: @am11 @ViktorHofer

@ghost
Copy link

ghost commented Nov 20, 2020

Tagging subscribers to this area: @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Initially investigated as an AzDO issue via https://github.com/dotnet/core-eng/issues/11472 (see for more context), it looks like linux and linux-musl are getting conflated in package naming.

Seems to have been introduced in #43804. @safern already has context, so assigning to him.

Author: MattGal
Assignees: safern
Labels:

area-Infrastructure, untriaged

Milestone: -

@am11
Copy link
Member

am11 commented Nov 20, 2020

Thanks @safern. I cannot access the engineering repo link, but all CI checks were passing when PR was completed. It could be that some target does not run during the CI builds?

@safern
Copy link
Member

safern commented Nov 20, 2020

This was happening on an official build which is internal (Where we produce and sign our packages). Still investigating. I'll update the issue once I have more info.

@safern
Copy link
Member

safern commented Nov 20, 2020

So for more context on this. Some of the packages produced in the linux-musl legs contain the wrong RID when produced:

  Microsoft.NETCore.DotNetHostResolver -> /root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.DotNetHostResolver.6.0.0-alpha.1.20570.6.nupkg
  Microsoft.NETCore.DotNetHost -> /root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.DotNetHost.6.0.0-alpha.1.20570.6.nupkg
  Microsoft.NETCore.DotNetHostPolicy -> /root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.DotNetHostPolicy.6.0.0-alpha.1.20570.6.nupkg
  Microsoft.NETCore.DotNetHost -> /root/runtime/artifacts/packages/Release/Shipping/runtime.linux-arm64.Microsoft.NETCore.DotNetHost.6.0.0-alpha.1.20570.6.nupkg
  Microsoft.NETCore.DotNetHostPolicy -> /root/runtime/artifacts/packages/Release/Shipping/runtime.linux-arm64.Microsoft.NETCore.DotNetHostPolicy.6.0.0-alpha.1.20570.6.nupkg
  Microsoft.NETCore.DotNetHostResolver -> /root/runtime/artifacts/packages/Release/Shipping/runtime.linux-arm64.Microsoft.NETCore.DotNetHostResolver.6.0.0-alpha.1.20570.6.nupkg
  Microsoft.NETCore.DotNetAppHost -> /root/runtime/artifacts/packages/Release/Shipping/runtime.linux-arm64.Microsoft.NETCore.DotNetAppHost.6.0.0-alpha.1.20570.6.nupkg
  Microsoft.NETCore.DotNetAppHost -> /root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.DotNetAppHost.6.0.0-alpha.1.20570.6.nupkg
  Microsoft.NETCore.App.Ref -> 
  Successfully created package '/root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Ref.6.0.0-alpha.1.20570.6.nupkg'.
  Successfully created package '/root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Ref.6.0.0-alpha.1.20570.6.symbols.nupkg'.
  Microsoft.NETCore.App.Runtime -> 
  Successfully created package '/root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Runtime.linux-musl-arm64.6.0.0-alpha.1.20570.6.nupkg'.
  Successfully created package '/root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Runtime.linux-musl-arm64.6.0.0-alpha.1.20570.6.symbols.nupkg'.
  Microsoft.NETCore.App.Host -> 
  Successfully created package '/root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Host.linux-musl-arm64.6.0.0-alpha.1.20570.6.nupkg'.
  Successfully created package '/root/runtime/artifacts/packages/Release/Shipping/Microsoft.NETCore.App.Host.linux-musl-arm64.6.0.0-alpha.1.20570.6.symbols.nupkg'.

If you look at that, some do have linux-musl-arm64 RID and some linux-arm64 missing the -musl part. What is confusing me, is that we're specifying /p:OutputRid=linux-musl-arm64 globally.

Some thing that I do see, is that this might be wrong: https://github.com/dotnet/runtime/pull/43804/files#diff-aec399247327507f8a921926dfa34e159e9786455234791d9895fe64185d91f2R155

As, that is setting OutputRid when Portable=true to _outputRID, however _outputRID doesn't consider linux-musl at all. The only case where linux-musl is considered is: portableOS... https://github.com/dotnet/runtime/pull/43804/files#diff-aec399247327507f8a921926dfa34e159e9786455234791d9895fe64185d91f2R91 which is ignored completely for the PortableBuild output rid.

@am11
Copy link
Member

am11 commented Nov 20, 2020

That part seem to be aligned with the older version: https://github.com/dotnet/runtime/pull/43804/files#diff-c69fd8bc3ea99b48dac09936c142064d7e750d2c626a7e9286384d94ef31fa2dL60.

Tested with ./build.sh -c Release -cross /p:OutputRid=linux-musl-arm64 /v:diag | grep " OutputRid =", we get

                   OutputRid = linux-musl-arm64
                   OutputRid = linux-musl-arm64
                   OutputRid = linux-musl-arm64
                   OutputRid = linux-musl-arm64
                   OutputRid = linux-musl-arm64
                   OutputRid = linux-musl-arm64
                   OutputRid = linux-musl-arm64
                   OutputRid = linux-musl-arm64

could be somewhere else in signing target.

@safern
Copy link
Member

safern commented Nov 20, 2020

Yeah, I just validated this. OutputRid is correct. The problem is PackageRID.

So basically, we have two models here under installer. The regular packages that are built by:

<ProjectReference Include="@(ProjectReference)" AdditionalProperties="PackageTargetRuntime=$(PackageRID)" />

That uses PackageRID to set the PackageTargetRuntime property which is what then calculates the output ID for those packages:
https://github.com/dotnet/arcade/blob/4eff4ec03f24b6208d51a372842470a222fa373b/src/Microsoft.DotNet.Build.Tasks.Packaging/src/build/Packaging.targets#L28_L32

We also have the framework packages which are driven by the shared framework SDK in the sfxproj's. Those use OutputRid that is why they're getting the right output RID. Which use RuntimeIdentifier: https://github.com/dotnet/arcade/blob/d1a58bc7ed005abe2d609245adefe3de4dff5c12/src/Microsoft.DotNet.SharedFramework.Sdk/targets/Microsoft.DotNet.SharedFramework.Sdk.targets#L93
https://github.com/dotnet/arcade/blob/d1a58bc7ed005abe2d609245adefe3de4dff5c12/src/Microsoft.DotNet.SharedFramework.Sdk/targets/Microsoft.DotNet.SharedFramework.Sdk.targets#L98

And RuntimeIdentifier is set to OutputRid:

<RuntimeIdentifier>$(OutputRid)</RuntimeIdentifier>

PackageRID is used in other places, i.e:

<_buildingOnRID Include="$(PackageRID)" Condition="'$(BuildOnUnknownPlatforms)' != 'false'">

https://github.com/dotnet/runtime/search?q=PackageRID

It seems like after your change, PackageRID will never have the linux-musl prefix, which it used to have in various cases: https://github.com/dotnet/runtime/pull/43804/files#diff-6952310dc69d6c06f01efd35042d5168dfd1faa9dcfd4b8b5dce9e01a828dc0dL148

The reason why it worked before is because of this:
https://github.com/dotnet/runtime/pull/43804/files#diff-1d3554b1a5d588d73d6b8b4fd8203e48c4ba0fba262ca4cc290e341b47593985L142

It would get a linux-musl prefix here:
https://github.com/dotnet/runtime/pull/43804/files#diff-1d3554b1a5d588d73d6b8b4fd8203e48c4ba0fba262ca4cc290e341b47593985L100

I think there are 2 things we need to do here:

  1. update the host-packages.proj to use OutputRid instead
  2. fix PackageRID to have linux-musl when it must have it.

@am11
Copy link
Member

am11 commented Nov 20, 2020

Thanks for the investigation, @safern.
(i just pulled ubuntu-18.04-cross-arm64-alpine container)

I have understood the problem. Before the change; in non-installer components:

  • PackageRID is used for package restore, which in case of cross-compilation is the RID of host OS.
  • OutputRid (different casing) is used for packages, RID of target OS.

In installer, PackageRID had mixed meanings (when used for restore, it is Host's RID, when used under pkg/ directories, it was OutputRid), while OutputRid had consistent meaning.

PR aligned the meaning of these two properties.

So I think your #1 is the better option.

@safern
Copy link
Member

safern commented Nov 20, 2020

Yeah, that is exactly what I was going to comment right now. PackageRID is correct and should stay as is. I found out that linux_musl_x64 is producing the right packages, because it is not a cross build, so the PackageRID is linux_musl_x64, because the host is an alpine container, however for arm builds of alpine the host is ubuntu so PackageRID is linux_x64.

So yeah, the right fix is #1. Are you planning on putting a PR? If not I can put it out there now.

@am11
Copy link
Member

am11 commented Nov 20, 2020

Yup, I have just found from build logs how to repro it in local container. Testing it right now, will put a PR soon.

@safern
Copy link
Member

safern commented Nov 20, 2020

Testing it right now, will put a PR soon.

Thanks 😄

@safern
Copy link
Member

safern commented Nov 21, 2020

Fixed by: #45041

@safern safern closed this as completed Nov 21, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

4 participants