-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Changes to emit a failure message when enabling ReadyToRun in unsupported scenarios #3123
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
Conversation
|
@nguerrera PTAL |
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets
Outdated
Show resolved
Hide resolved
|
Can you add a test covering this? |
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets
Outdated
Show resolved
Hide resolved
| </PropertyGroup> | ||
|
|
||
| <!-- TODO: Temp error to limit R2R compilation to self-contained mode only. Delete this when fixing #3109 and #3110 --> | ||
| <NETSdkError Condition="'$(SelfContained)' != 'true'" ResourceName="ReadyToRunTargetNotSuppotedError" /> |
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.
Should this check be upfront (say in prepare for publish) before we do any work for R2R compilation?
Not a significant issue, since this is only an interim check.
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.
It is upfront. It runs before the PrepareForReadyToRunCompilation task, which runs before any compilation
….5 (#3123) - Microsoft.DotNet.Cli.Runtime - 5.0.100-alpha1.19506.5
Unsupported scenarios are: