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

Do not require msbuild.exe.config on .NET Core #883

Merged
merged 1 commit into from
Aug 9, 2016

Conversation

rainersigwald
Copy link
Member

All of the code that could read configuration data out of
msbuild.exe.config is dependent on System.Configuration, which isn't
available in .NET Core. The reading code was all #if'ed out, but we still
required that the file be physically present, or MSBuild would fail on
initialization.

I made IsValidMSBuildPath dependent on the same compile-time check as
src/Shared/ToolsetElement.cs.

All of the code that could read configuration data out of
msbuild.exe.config is dependent on System.Configuration, which isn't
available in .NET Core. The reading code was all #if'ed out, but we still
required that the file be physically present, or MSBuild would fail on
initialization.

I made `IsValidMSBuildPath` dependent on the same compile-time check as
src/Shared/ToolsetElement.cs.
@rainersigwald
Copy link
Member Author

@AndyGerlicher does this look reasonable to you?

I thought we were still reading toolsets on Core but we clearly weren't because all the toolset-reading code is not built. We shouldn't require an auxiliary file that we don't need . . .

@AndyGerlicher
Copy link
Contributor

:shipit:

@AndyGerlicher
Copy link
Contributor

@dotnet-bot test OSX Build for CoreCLR please

@ghost
Copy link

ghost commented Aug 9, 2016

@rainersigwald, @AndyGerlicher, that OSX build break http://dotnet-ci.cloudapp.net/job/Microsoft_msbuild/job/_OSX_CoreCLR_prtest/86/ was same as #676 (http://dotnet-ci.cloudapp.net/job/Microsoft_msbuild/job/_OSX_CoreCLR_prtest/89/). This probably is intermittent issue, but is there a tracking issue for this or should we have one? It might be the issue with CoreCLR etc. but it probably needs more investigation. :)

@TheRealPiotrP
Copy link
Contributor

is there a nupkg we can reference?

@rainersigwald rainersigwald deleted the core-exe-config branch August 10, 2016 19:04
@rainersigwald
Copy link
Member Author

@piotrpMSFT I kicked off a build, will let you know the version when it's done.

@rainersigwald
Copy link
Member Author

@piotrpMSFT 0.1.0-preview-00030-160810 should contain this change (and up to 5dc5fd5).

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

Successfully merging this pull request may close these issues.

4 participants