-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Use Arcade-provided IBCMerge support for System.Private.CoreLib. #2308
Conversation
@@ -20,7 +20,7 @@ | |||
<UsingToolIbcOptimization>true</UsingToolIbcOptimization> | |||
<UsingToolXliff>false</UsingToolXliff> | |||
<!-- Paths used during restore --> | |||
<EnableNgenOptimization Condition="'$(EnableNgenOptimization)' == '' and '$(ConfigurationGroup)' == 'Release'">true</EnableNgenOptimization> | |||
<EnableNgenOptimization Condition="'$(EnableNgenOptimization)' == '' and ('$(ConfigurationGroup)' == 'Release' or '$(Configuration)' == 'Release')">true</EnableNgenOptimization> |
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 will soon be able to clean Configuration vs ConfigurationGroup up :)
Well, I'll wait until tomorrow to give people some more time to take a glance at this. |
@@ -106,7 +108,8 @@ | |||
<MicrosoftPrivateIntellisensePackage>microsoft.private.intellisense</MicrosoftPrivateIntellisensePackage> | |||
<MicrosoftDotNetXUnitConsoleRunnerPackage>Microsoft.DotNet.XUnitConsoleRunner</MicrosoftDotNetXUnitConsoleRunnerPackage> | |||
<MicrosoftNETCoreAppPackage>Microsoft.NETCore.App</MicrosoftNETCoreAppPackage> | |||
<optimizationIBCCoreCLRPackage>optimization.IBC.CoreCLR</optimizationIBCCoreCLRPackage> | |||
<WindowsOptimizationIBCCoreCLRPackage>optimization.windows_nt-x64.IBC.CoreCLR</WindowsOptimizationIBCCoreCLRPackage> | |||
<LinuxOptimizationIBCCoreCLRPackage>optimization.linux-x64.IBC.CoreCLR</LinuxOptimizationIBCCoreCLRPackage> |
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.
These property names seem oddly inconsistent with the names for the CoreFx packages, which have similar package names.
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'd prefer to keep the names a little more verbose about being IBC data instead of just "optimization data" since we also have the PGO data package for CoreCLR.
Hello @jkoritzinsky! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
All of the failures are known infra issues. I'm going to merge this in. |
Move System.Private.CoreLib to use the built-in IBCMerge support provided by Arcade instead of the custom scripting in build.cmd.
Fixes dotnet/coreclr#24751