Source-build: Don't invoke MSBuild from within the CLI

Created on 7 Mar 2017 · 15Comments · Source: dotnet/source-build

Today, we invoke MSBuild.dll directly from within the CLI. We do this (vs dotnet msbuild) because dotnet msbuild pushes some additional stuff into your environment. This causes downstream issues in repositories that don't expect these things to be set to the values CLI uses, since they use different build toolsets.

@piotrpMSFT and @eerhardt have asked that we don't do this, as they want the freedom to move stuff within the CLI around and don't want to be a general purpose MSBuild distribution mechanism. Their concern is if folks see another "dotnet" project doing this they will think it's a good and right thing to do.

So instead, we need to start restoring and publish MSBuild, and then invoke that MSBuild.dll (we can do this after building our tasks but before jumping into build.proj.

area-build

Source

ellismg

Most helpful comment

In general we need to be killing environment variables. Letting them influence our build this way is just creating a new razzle. Not our goal.

Instead use explicit parameters

jaredpar on 22 Sep 2017

🎉1 👍1

All 15 comments

I still have a inkling that the CLI could do something better to support this scenario. Maybe an option that doesn't set these env vars?

For sure, the "CscToolExe" env var is a hack. This doesn't work for non-built-in languages like F#, so why do we do C# this way?

The other 2 env vars MSBuildExtensionsPath and MSBuildSDKsPath are necessary though, it tells dotnet msbuild where to find its assets.

I'd hate tell people that they can't/shouldn't use dotnet msbuild to build their MSBuild projects, and they they have to go about finding/restoring/installing their own MSBuild just to build an MSBuild project. You should be able to use dotnet msbuild in the normal cases.

eerhardt on 7 Mar 2017

👍1

@eerhardt do you know if this is still the case?

karajas on 23 Aug 2017

It is still the case that we don't want anyone being dependent on the internal folder layout of the .NET Core SDK. So if you are still assuming that MSBuild.dll is in a specific location of the SDK, then it still needs to be changed.

eerhardt on 23 Aug 2017

👍1

@eerhardt I agree that we should try to use dotnet msbuild instead of assuming its location in the CLI and I also prefer that over downloading our own msbuild to use. What do you think is the right solution here? Should we fix the repos that don't work with this env variable or fix the CLI to stop setting them? Or do we have some extension point to try and clear those variables?

weshaggard on 22 Sep 2017

In general we need to be killing environment variables. Letting them influence our build this way is just creating a new razzle. Not our goal.

Instead use explicit parameters

jaredpar on 22 Sep 2017

🎉1 👍1

What do you think is the right solution here?

I think the right solution is what I said on Mar 7. https://github.com/dotnet/source-build/issues/58#issuecomment-284881167

I think the CLI needs to change to allow for this scenario.

eerhardt on 22 Sep 2017

If that is the case it sounds like we need a CLI issue tracking this so we can make some progress. cc @livarcocc @nguerrera

weshaggard on 22 Sep 2017

That being said, I also think we need to fix certain repos - like corefx - that don't use the CLI's dotnet to build on Windows, but use it to build on Linux. I think building corefx should be consistent across platforms. Instead today it uses whatever MSBuild is on the path, which is different between machines, and different between command prompts on the same machine.

eerhardt on 22 Sep 2017

That being said, I also think we need to fix certain repos - like corefx - that don't use the CLI's dotnet to build on Windows, but use it to build on Linux. I think building corefx should be consistent across platforms.

Totally agree that is something that is being worked on by @joperezr

weshaggard on 22 Sep 2017

👍1

I would like to understand why it's a requirement to let any constituent repos of source build override the compiler that is used. Ultimately, everything should be building with the same LKG toolset.

nguerrera on 28 Sep 2017

Because as it currently exists, different repos use different toolsets.

For example, coreclr and corefx use BuildTools, which currently brings its own MSBuild, Roslyn, etc. And on Windows, these repos currently use whatever MSBuild.exe is on the $Path, so there's another toolset.

The cli repo uses a different toolset to build - it uses a previously build .NET Core SDK which has a different MSBuild, Roslyn, etc.

I agree we should be moving towards using the same toolset, but it is taking a long time to get there. And to me it doesn't look like it is happening any time soon.

eerhardt on 28 Sep 2017

Thanks for the explanation. In the longer term, I imagine the one lkg toolset to be recent and updated frequently. And updates to the toolset could be gated on a successful full stack build.

Just as we coordinate product dependencies across the stack, we should coordinate build dependencies.

Regardless, I don't disagree about fixing those hacked env vars and we can of course start there.

Get Outlook for iOShttps://aka.ms/o0ukef

From: Eric Erhardt notifications@github.com
Sent: Wednesday, September 27, 2017 8:09:43 PM
To: dotnet/source-build
Cc: Nick Guerrera; Mention
Subject: Re: [dotnet/source-build] Don't invoke MSBuild from within the CLI (#58)

Because as it currently exists, different repos use different toolsets.

The cli repo uses a different toolset to build - it uses a previously build .NET Core SDK which has a different MSBuild, Roslyn, etc.

I agree we should be moving towards using the same toolset, but it is taking a long time to get there. And to me it doesn't look like it is happening any time soon.

Another reason is because there are varying needs between the repos, and the same toolset may not work for all repos at all times. For example, right now corefx is using C# 7.2 and the latest Roslyn compiler. However, the latest Roslyn compiler is breaking coreclr. See dotnet/coreclr#14002 (comment)https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fcoreclr%2Fpull%2F14002%23issuecomment-331005573&data=02%7C01%7CNick.Guerrera%40microsoft.com%7C96b8e8f425cf495e576308d5061e5e98%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636421649863976060&sdata=y2T56wTNewOzu0r5y9PoXE9uBajCohuusP2o%2BGaQugw%3D&reserved=0. So in this case, we would be blocking corefx from moving forward because coreclr can't take the updated compiler.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fsource-build%2Fissues%2F58%23issuecomment-332715234&data=02%7C01%7CNick.Guerrera%40microsoft.com%7C96b8e8f425cf495e576308d5061e5e98%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636421649863976060&sdata=Ms3anVIak7AZssTHG76ugn69EFFtVGEQNHDPzvpYeaA%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAEmzl4yRFQlekT6nMtzuyMlMBBc_0ykks5smw33gaJpZM4MWEF9&data=02%7C01%7CNick.Guerrera%40microsoft.com%7C96b8e8f425cf495e576308d5061e5e98%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636421649863976060&sdata=%2Bwh58K%2FhwE9F5vQPwIiY2x23lUdaZ01jeXhXIOT%2B9eU%3D&reserved=0.

nguerrera on 28 Sep 2017

👍1

https://github.com/dotnet/cli/pull/7311 by @khyperia will remove the setting of CscToolExe. :) It is going in to 15.5 SDK. I think we wait until source build can use a 15.5+ SDK as LKG and then switch source-build to plain dotnet msbuild. I'm not worried about dotnet path/to/msbuild.dll in the meantime because 2.0 SDK layout won't ever change.

nguerrera on 2 Oct 2017

We just took a PR in master from @khyperia where we are no longer setting CscToolExe.