-
Notifications
You must be signed in to change notification settings - Fork 519
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
[dotnet] Tell .NET to not generate files we don't need. Fixes #9687. #11693
Conversation
…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.
@@ -132,6 +132,9 @@ | |||
<StartupHookSupport Condition="'$(StartupHookSupport)' == ''">false</StartupHookSupport> | |||
<UseSystemResourceKeys Condition="'$(UseSystemResourceKeys)' == ''">true</UseSystemResourceKeys> | |||
<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> |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use OutputType=Exe
:
We also depend on this elsewhere in this exact file:
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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 .netor 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.
@@ -22,5 +22,11 @@ | |||
<_XamarinSdkRoot Condition="'$(_XamarinSdkRoot)' == ''">$(_XamarinSdkRootDirectory)</_XamarinSdkRoot> | |||
<!-- _XamarinSdkRootOnMac this should be passed to tasks that need to access the Xamarin Sdk dir on the Mac, this value will be overriden from Windows --> | |||
<_XamarinSdkRootOnMac>$(_XamarinSdkRoot)</_XamarinSdkRootOnMac> | |||
|
|||
<!-- We don't need any runtime configuration files --> | |||
<GenerateRuntimeConfigurationFiles Condition="'$(GenerateRuntimeConfigurationFiles)' ==''">false</GenerateRuntimeConfigurationFiles> |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds correct to me.
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API & Generator diff✅ API Diff (from PR only) (no change) Test results1 tests failed, 106 tests passed.Failed tests
Pipeline on Agent XAMBOT-1100.BigSur' |
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diff✅ API Diff (from PR only) (no change) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results1 tests failed, 106 tests passed.Failed tests
Pipeline on Agent XAMBOT-1094.BigSur' |
Test failure is unrelated and will be fixed with #11732. |
Tell .NET to not generate files we don't need:
Fixes #9687.