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

Stop NGENing System.Net.Http and dependencies #27537

Merged
merged 3 commits into from
Jun 7, 2018
Merged

Stop NGENing System.Net.Http and dependencies #27537

merged 3 commits into from
Jun 7, 2018

Conversation

OmarTawfik
Copy link
Contributor

This is failing NGEN in official builds, as binding redirects to dependencies are not generated correctly.
We don't actually use/load that assembly in any way, so let's stop NGENing it.

cc @jaredpar @tannergooding @jasonmalinowski

@OmarTawfik OmarTawfik added this to the 15.8 milestone Jun 6, 2018
@OmarTawfik OmarTawfik requested a review from a team as a code owner June 6, 2018 21:15
@OmarTawfik OmarTawfik requested a review from jaredpar June 6, 2018 21:17
@tannergooding
Copy link
Member

Could you retarget to dev15.8.x-preview3-vs-deps for the insertion?

@jasonmalinowski jasonmalinowski changed the base branch from master to dev15.8-preview3 June 6, 2018 21:21
@@ -41,15 +41,15 @@ folder InstallDir:\MSBuild\15.0\Bin\Roslyn
file source=$(OutputPath)\Vsix\CompilerExtension\Microsoft.Win32.Primitives.dll vs.file.ngenArchitecture=all
file source=$(OutputPath)\Vsix\CompilerExtension\System.AppContext.dll vs.file.ngenArchitecture=all
file source=$(OutputPath)\Vsix\CompilerExtension\System.Console.dll vs.file.ngenArchitecture=all
file source=$(OutputPath)\Vsix\CompilerExtension\System.Diagnostics.DiagnosticSource.dll vs.file.ngenArchitecture=all
file source=$(OutputPath)\Vsix\CompilerExtension\System.Diagnostics.DiagnosticSource.dll
Copy link
Member

Choose a reason for hiding this comment

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

We should really put comments here why these are different.

Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to explicitly opt out of NGEN vs. implicitly doing so?

@jaredpar
Copy link
Member

jaredpar commented Jun 6, 2018

Related dotnet/sdk#2227

@OmarTawfik
Copy link
Contributor Author

There is a build boss error:

14:27:29 Verifying Microsoft.CodeAnalysis.Compilers.swr
14:27:29 	Dll System.Diagnostics.DiagnosticSource.dll is missing
14:27:29 	Dll System.Net.Http.dll is missing

Investigating why.

@OmarTawfik OmarTawfik requested a review from a team as a code owner June 6, 2018 21:48
@OmarTawfik OmarTawfik merged commit 44f2bca into dotnet:dev15.8-preview3 Jun 7, 2018
@OmarTawfik OmarTawfik deleted the dev/otawfik/stop-net-http-ngen branch June 7, 2018 01:37
@tmat
Copy link
Member

tmat commented Jun 7, 2018

Why do we still include them if we know we're not gonna use them?

@jaredpar
Copy link
Member

jaredpar commented Jun 7, 2018

@tmat see dotnet/sdk#2227

This is what the SDK believes we are using. Previously we tried to be smart and exclude items we thought we weren't using. That lead to a series of different issues. There just doesn't appear to be a way to win here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants