Skip to content

[dotnet] Tell .NET to not generate files we don't need. Fixes #9687. #11693

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.

Sign up for GitHub

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

Jump to bottom
Merged
rolfbjarne merged 2 commits into from
May 31, 2021
Merged

[dotnet] Tell .NET to not generate files we don't need. Fixes #9687. #11693

rolfbjarne merged 2 commits into from
May 31, 2021

Conversation

@rolfbjarne
Copy link
Member

@rolfbjarne rolfbjarne commented May 26, 2021

Tell .NET to not generate files we don't need:

  • Runtime configuration file (*.runtimeconfig.json).
  • Dependency file (*.deps.json).
  • Reference assemblies for executable projects.

Fixes #9687.

@rolfbjarne
[dotnet] Tell .NET to not generate files we don't need. Fixes dotnet#…
2aaa7a3 
…9687.

Tell .NET to not generate files we don't need:

* Runtime configuration file (*.runtimeconfig.json).
* Dependency file (*.deps.json).
* Reference assemblies for executable projects.

Fixes dotnet#9687.
@rolfbjarne rolfbjarne added the not-notes-worthy Ignore for release notes label May 26, 2021
spouliot
spouliot suggested changes May 26, 2021
View reviewed changes
dotnet/targets/Xamarin.Shared.Sdk.targets
<UseNativeHttpHandler Condition="'$(_PlatformName)' != 'macOS' And '$(UseNativeHttpHandler)' == ''">true</UseNativeHttpHandler>

<!-- We don't need to generate reference assemblies for apps or app extensions -->
<ProduceReferenceAssembly Condition="'$(ProduceReferenceAssembly)' == '' And ('$(OutputType)' == 'Exe' Or '$(IsAppExtension)' == 'true')">false</ProduceReferenceAssembly>
Copy link
Contributor

@spouliot spouliot May 26, 2021

Choose a reason for hiding this comment

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

hmm... iirc we're using .dll for the main assembly for net6 so that would never work (except for app extensions)

Copy link
Member Author

@rolfbjarne rolfbjarne May 26, 2021

Choose a reason for hiding this comment

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

We use OutputType=Exe:

https://github.com/xamarin/xamarin-macios/blob/9d97d43f74c28fa4e0c1cc9fa70a0983f24b7ad9/tests/dotnet/MySingleView/MySingleView.csproj#L6

We also depend on this elsewhere in this exact file:

https://github.com/xamarin/xamarin-macios/blob/2aaa7a31b2313dc7bcd50af04c85b4db6d2c4ab0/dotnet/targets/Xamarin.Shared.Sdk.targets#L15-L35

Copy link
Contributor

@spouliot spouliot May 26, 2021

Choose a reason for hiding this comment

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

hmm... is that recent ?

because size comparison shows MySingleView.dll for .net

or maybe the issue is elsewhere... <OutputType> not doing what seems logical ?

Copy link
Member Author

@rolfbjarne rolfbjarne May 26, 2021

Choose a reason for hiding this comment

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

hmm... is that recent ?

No, it's not recent, it's always been this way.

because size comparison shows MySingleView.dll for .net

or maybe the issue is elsewhere... <OutputType> not doing what seems logical ?

Yes, OutputType is doing something different now. I'm not quite sure what, but I believe it doesn't affect the extension for the output assembly anymore.

@rolfbjarne rolfbjarne added not-notes-worthy Ignore for release notes and removed not-notes-worthy Ignore for release notes labels May 26, 2021
filipnavara
filipnavara reviewed May 26, 2021
View reviewed changes
dotnet/targets/Xamarin.Shared.Sdk.props Outdated
<_XamarinSdkRootOnMac>$(_XamarinSdkRoot)</_XamarinSdkRootOnMac>

<!-- We don't need any runtime configuration files -->
<GenerateRuntimeConfigurationFiles Condition="'$(GenerateRuntimeConfigurationFiles)' ==''">false</GenerateRuntimeConfigurationFiles>
Copy link
Member

@filipnavara filipnavara May 26, 2021

Choose a reason for hiding this comment

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

You will actually need it at some point. There's a Mono task that converts the JSON into binary blob and that can be passed to the Mono runtime. I recently implemented it in AppleAppBuilder on the dotnet/runtime side.

Copy link
Member

@filipnavara filipnavara May 26, 2021

Choose a reason for hiding this comment

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

PR (for example usage): dotnet/runtime#53172
Tracking issue: dotnet/runtime#49237

It's necessary for features like EnableUnsafeUTF7Encoding and EnableUnsafeBinaryFormatterSerialization.

Copy link
Member Author

@rolfbjarne rolfbjarne May 31, 2021

Choose a reason for hiding this comment

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

So I guess we'll have to:

  • Keep generating *.runtimeconfig.json file
  • Use Mono's task to convert the json to a binary blob.
  • Bundle the blob in the .app
  • Don't bundle the json in the app.

Is that correct?

Copy link
Member Author

@rolfbjarne rolfbjarne May 31, 2021

Choose a reason for hiding this comment

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

I've filed #11745 to track this.

Copy link
Member

@filipnavara filipnavara May 31, 2021

Choose a reason for hiding this comment

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

Sounds correct to me.

@vs-mobiletools-engineering-service2
Copy link
Collaborator

vs-mobiletools-engineering-service2 commented May 26, 2021

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API & Generator diff

✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)

Test results

1 tests failed, 106 tests passed.

Failed tests

  • link sdk/iOS Unified 64-bits - simulator/Release [dotnet]: Failed

Pipeline on Agent XAMBOT-1100.BigSur'
Merge 2aaa7a3 into 9d97d43

@rolfbjarne rolfbjarne mentioned this pull request May 31, 2021
Closed
@vs-mobiletools-engineering-service2
Copy link
Collaborator

vs-mobiletools-engineering-service2 commented May 31, 2021

❌ [PR Build] Tests failed on Build ❌

Tests failed on Build.

API diff

✅ API Diff from stable

View API diff

API & Generator diff

✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)

GitHub pages

Results can be found in the following github pages (it might take some time to publish):

Test results

1 tests failed, 106 tests passed.

Failed tests

  • monotouch-test/Mac [dotnet]/Debug (CoreCLR) [dotnet]: Failed (Test run failed.
    Tests run: 2444 Passed: 2356 Inconclusive: 7 Failed: 2 Ignored: 86)

Pipeline on Agent XAMBOT-1094.BigSur'
Merge a351a01 into 8d6893c

@rolfbjarne
Copy link
Member Author

rolfbjarne commented May 31, 2021

Test failure is unrelated and will be fixed with #11732.

@rolfbjarne rolfbjarne merged commit 71a0659 into dotnet:main May 31, 2021
@rolfbjarne rolfbjarne deleted the issue-9687 branch May 31, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@filipnavara filipnavara filipnavara left review comments

@spouliot spouliot spouliot approved these changes

@mandel-macaque mandel-macaque mandel-macaque approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

not-notes-worthy Ignore for release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[.NET] Set a few MSBuild variables to make .NET stop generating unnecessary files

5 participants

@rolfbjarne @vs-mobiletools-engineering-service2 @spouliot @filipnavara @mandel-macaque