Skip to content

Alter RuntimeComponentManifest.targets to use the correct runtime pack#56431

Merged
radical merged 7 commits intodotnet:mainfrom
steveisok:fix-runtimecomponent-lookup
Jul 29, 2021
Merged

Alter RuntimeComponentManifest.targets to use the correct runtime pack#56431
radical merged 7 commits intodotnet:mainfrom
steveisok:fix-runtimecomponent-lookup

Conversation

@steveisok
Copy link
Member

@steveisok steveisok commented Jul 27, 2021

When trying to process what runtime components are available, we were assuming %(ResolvedRuntimePack.PackageDirectory) would give us the correct Microsoft.NETCore.App runtime pack. Unfortunately, the ordering was likely altered when we started setting items in the Mono workload. Since relying on position of the items isn't reliable, we will instead use %(ResolvedFrameworkReference) to tell us which runtime pack is active and where it's located.

Fixes #56299

Alter RuntimeComponentManifest.targets to use the correct Runtime Pack
d135e27 
When trying to process what runtime components are available, we were assuming %(ResolvedRuntimePack.PackageDirectory) would
give us the correct Microsoft.NETCore.App runtime pack.  Unfortunately, the ordering was likely altered when we started
setting <KnownRuntimePack> items in the Mono workload.  Since relying on position of the items isn't reliable, we will instead use %(ResolvedFrameworkReference) to tell us which runtime pack is active and where it's located.

Fixes dotnet#56299
@steveisok steveisok requested a review from marek-safar as a code owner July 27, 2021 20:28
@ghost
Copy link

ghost commented Jul 27, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

lambdageek
lambdageek approved these changes Jul 27, 2021
View reviewed changes
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

Should we have a check somewhere that @(ResolvedFrameworkReference->WithMetadataValue('Identity', 'Microsoft.NETCore.App')->Count()) == 1 ?

rolfbjarne
rolfbjarne approved these changes Jul 28, 2021
View reviewed changes
Copy link
Member

@rolfbjarne rolfbjarne left a comment

Choose a reason for hiding this comment

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

If I apply this fix locally, it works.

@radical
Copy link
Member

radical commented Jul 28, 2021

Wasm workload tests are failing with:

 The "MonoRuntimeComponentManifestReadTask" task failed unexpectedly. [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
 System.IO.DirectoryNotFoundException: Could not find a part of the path '/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/ [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
         /datadisks/disk1/work/B43109B2/p/dotnet-workload/packs/Microsoft.NETCore.App.Runtime.Mono.browser-wasm/6.0.0-ci\runtimes\browser-wasm\build\RuntimeComponentManifest.json [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
       '. [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
    at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String path, OpenFlags flags, Int32 mode) in System.Private.CoreLib.dll:token 0x60000d5+0x0 [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
    at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize) in System.Private.CoreLib.dll:token 0x60000d9+0x31 [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
    at System.IO.Strategies.OSFileStreamStrategy..ctor(String path, FileMode mode, FileAccess access, FileShare share, FileOptions options, Int64 preallocationSize) in System.Private.CoreLib.dll:token 0x6006142+0x78 [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
    at System.IO.File.OpenRead(String path) in System.Private.CoreLib.dll:token 0x6005cbb+0x9 [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
    at JsonToItemsTaskFactory.JsonToItemsTaskFactory.JsonToItemsTask.TryGetJson(String jsonFilePath, JsonModelRoot& json) in JsonToItemsTaskFactory.dll:token 0x6000019+0x2 [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
    at JsonToItemsTaskFactory.JsonToItemsTaskFactory.JsonToItemsTask.Execute() in JsonToItemsTaskFactory.dll:token 0x6000018+0x1f [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute() in Microsoft.Build.dll:token 0x60015fb+0x3e [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]
    at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask) in Microsoft.Build.dll:token 0x600147a+0x804 [/datadisks/disk1/work/B43109B2/w/A32D0938/e/qql1xr5s.l1p/rebuild_Release_True.csproj]

And that seems to be because the json path is:

                   Set Property: _MonoRuntimeComponentManifestJsonFilePath=
                           /datadisks/disk1/work/B43109B2/p/dotnet-workload/packs/Microsoft.NETCore.App.Runtime.Mono.browser-wasm/6.0.0-ci\runtimes\browser-wasm\build\RuntimeComponentManifest.json

                   Initializing task factory "JsonToItemsTaskFactory.JsonToItemsTaskFactory" from assembly "/datadisks/disk1/work/B43109B2/p/dotnet-workload/packs/Microsoft.NET.Runtime.MonoTargets.Sdk/6.0.0-ci/Sdk/../tasks/net6.0/JsonToItemsTaskFactory.dll".

Note the newlines in the path.

fix:

  1. don't use newlines when setting the path
  2. The task should check the path, and log an error

lambdageek
lambdageek approved these changes Jul 28, 2021
View reviewed changes
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

:shipit:

@radical
Copy link
Member

radical commented Jul 28, 2021

wasm Workload tests failing with:

/datadisks/disk1/work/AB8D0934/p/dotnet-workload/packs/Microsoft.NET.Runtime.MonoTargets.Sdk/6.0.0-ci/Sdk/RuntimeComponentManifest.targets(3,5): error : Empty ResolveFrameworkReference.RuntimePackPath while trying to read runtime components manifest

I'll try this out locally.

@radical radical merged commit d0f2752 into dotnet:main Jul 29, 2021
@jaredpar jaredpar mentioned this pull request Aug 17, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Aug 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@radical radical radical approved these changes

@rolfbjarne rolfbjarne rolfbjarne approved these changes

@lambdageek lambdageek lambdageek approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@akoeplinger akoeplinger Awaiting requested review from akoeplinger akoeplinger is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Mono: The logic to find the RuntimeComponentManifest.json file is incorrect

4 participants

@steveisok @radical @rolfbjarne @lambdageek