Skip to content

Conversation

pranavkm
Copy link

Recent builds of dotnet-watch contain more than one runtime in the runtimconfig.json. This trips up the way RetargetTools currently works. Changing this to a bespoke task rather than one that finds-and-replaces text.

@pranavkm pranavkm requested review from dsplaisted and sfoslund June 15, 2021 20:50
@dsplaisted
Copy link
Member

Yes, this changed with dotnet/sdk#17982

Copy link
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

If there are any tools that don't use ASP.NET, then this probably needs to be updated to handle both framework and frameworks in the json. Otherwise looks great.

{
var text = File.ReadAllText(file);
JObject config = JObject.Parse(text);
var frameworks = config["runtimeOptions"]?["frameworks"];
Copy link
Member

Choose a reason for hiding this comment

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

If there is only one framework, it will be in the framework element instead of having a list in the frameworks:

https://github.com/dotnet/sdk/blob/ffca47e9a36652da2e7041360f2201a2ba197194/src/Tasks/Microsoft.NET.Build.Tasks/GenerateRuntimeConfigurationFiles.cs#L246-L253

Probably this task needs to handle both cases.

@pranavkm pranavkm force-pushed the prkrishn/fixup-shared-runtime branch from 041f3cc to 4cbc873 Compare June 16, 2021 21:57
@pranavkm pranavkm merged commit 97675db into main Jun 17, 2021
@pranavkm pranavkm deleted the prkrishn/fixup-shared-runtime branch June 17, 2021 16:37
pranavkm added a commit that referenced this pull request Jun 22, 2021
pranavkm added a commit that referenced this pull request Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants