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

Add msbuild task to generate binary runtimeconfig format #49544

Merged
merged 10 commits into from
Mar 17, 2021

Conversation

fanyang-mono
Copy link
Member

Fixes #49235

@ghost
Copy link

ghost commented Mar 12, 2021

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

Issue Details

Fixes #49235

Author: fanyang-mono
Assignees: -
Labels:

area-AssemblyLoader-mono

Milestone: -

@ViktorHofer
Copy link
Member

What is this for? The runtimeconfig.json file is created by a task in the SDK, why would we do this here in dotnet/runtime?

1 similar comment
@ViktorHofer
Copy link
Member

What is this for? The runtimeconfig.json file is created by a task in the SDK, why would we do this here in dotnet/runtime?

@fanyang-mono
Copy link
Member Author

What is this for? The runtimeconfig.json file is created by a task in the SDK, why would we do this here in dotnet/runtime?

This is needed for mobile targets to honor the settings from runtimeconfig.json file.

@CoffeeFlux
Copy link
Contributor

CoffeeFlux commented Mar 12, 2021

Viktor this is for the mobile products, where we don't want to embed a json parser and pay the associated startup cost. The idea is to generate a file at publish time that is embedded in the APK and Mono can parse in easily. It should be transparent to users; they can just edit the json in their product like normal.

@jkotas
Copy link
Member

jkotas commented Mar 12, 2021

The runtimeconfig.json file is created by a task in the SDK, why would we do this here in dotnet/runtime?

This is in the same category as other tasks under src\tasks. These tasks contain parts of mobile publishing pipeline logic that is tightly coupled with the runtime.

It is a valid question to ask whether all these tasks should live in SDK instead. If these tasks were in SDK, updating them would be much more painful. We are actually on that plan with crossgen task and it is pain to coordinate the updates. I think we may want to look into having the crossgen task on the same plan as these mobile tasks so that it easier to update (cc @dotnet/crossgen-contrib).

@ViktorHofer
Copy link
Member

ViktorHofer commented Mar 15, 2021

It is a valid question to ask whether all these tasks should live in SDK instead. If these tasks were in SDK, updating them would be much more painful.

Absolutely agree in regards to crossgen2 and even the linker. What about keeping the msbuild files (targets, tasks) for crossgen2 and the mobile tasks in dotnet/runtime and letting dotnet/sdk consume them (the same way the sdk consumes the linker package and its props/targets) so that they are sequenced into the SDK but live next to the component's sources?

In the example of linker:

  1. Bundle props, targets and tasks as an msbuild sdk package: https://github.com/mono/linker/blob/main/src/ILLink.Tasks/. ILLink.Tasks is a nuget sdk package which contains props, targets and tasks. For linker most of the targets code still lives in dotnet/sdk but presumably that could be moved back into mono/linker. cc @sbomer
  2. Consume that sdk package in dotnet/sdk via the msbuild sdk resolver: https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.props#L14. Note that this doesn't mean that the package is consumed via nuget. It is automagically added into the sdk folder (dotnet/sdk's GenerateLayout handles that).

fanyang-mono and others added 3 commits March 15, 2021 14:53
Co-authored-by: Ryan Lucia <ryan@luciaonline.net>
Co-authored-by: Ryan Lucia <ryan@luciaonline.net>
Co-authored-by: Ryan Lucia <ryan@luciaonline.net>
@sbomer
Copy link
Member

sbomer commented Mar 15, 2021

Agreed that this would make it much less painful to iterate on the build logic. The linker targets were moved to the SDK in the first place because:

  • They depended details of the publish logic that kept changing in the SDK and breaking the linker setup. Now that the SDK explicitly depends on ILLink at a specific place, this may be more stable.
  • The SDK had better testing infra for the end-to-end msbuild logic. I still think it's desirable to have tests for the MSBuild targets live in the same place as the targets.

Based on this, we decided at the time that the task would live in the linker repo, while most of the props/targets (the ones with actual logic, not just imports) would live in the sdk, but it has meant that updates to both needed to be done in multiple steps.

@fanyang-mono
Copy link
Member Author

Lane https://github.com/dotnet/runtime/issues/42677 failed, due to the know issue #42677

@fanyang-mono fanyang-mono merged commit ac5c33d into dotnet:main Mar 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 16, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@fanyang-mono fanyang-mono deleted the runtimeconfig branch May 27, 2021 14:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create msbuild task that generates binary runtimeconfig format
7 participants