Port Microsoft.Build to .NET Core #159
Conversation
| path = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "MSBuild.exe"); | ||
| #else | ||
| path = Path.GetDirectoryName(Process.GetCurrentProcess().MainModule.FileName); |
There was a problem hiding this comment.
Is it safe to just use this all the time? That'd be easier.
There was a problem hiding this comment.
Is it safe to just use this all the time? That'd be easier.
@rainersigwald I'm not sure. In general I'm wary of changing things for the .NET Framework build unless I'm confident they won't cause breaking changes if we merge them back into the next release of MSBuild for .NET Framework.
There was a problem hiding this comment.
I like that philosophy. We need to figure out how to balance it with not creating too much long-term debt.
(This is mostly because I hit a similar issue and just changed the product code. 😇)
|
LGTM |
| <Compile Include="Definition\ToolsetConfigurationReader.cs" /> | ||
| <Compile Include="..\Shared\ToolsetElement.cs"> | ||
| <Compile Include="Definition\ToolsetConfigurationReader.cs" Condition="$(FeatureSystemConfiguration) == 'true'"/> | ||
| <Compile Include="..\Shared\ToolsetElement.cs" Condition="$(FeatureSystemConfiguration) == 'true'"> |
There was a problem hiding this comment.
I think if I had to pick I would say #if the contents of the file over a condition in the csproj to avoid confusion in VS. If there's a good reason for this though I'm fine with it.
There was a problem hiding this comment.
I wondered about that, too. I came down on the other side of it but not for any good reason.
There was a problem hiding this comment.
So far what I've been doing is that if the whole file needs to be included or excluded based on a feature, I would do that with a condition in MSBuild. For groups of files like are in the SharedUtilities/Compat folder that feels better to me. For one-off (or two-off) files like this one I don't have much of a strong opinion.
I'm going to go ahead and merge this, we can always discuss and make a change later.
|
LGTM. I would just like to know the reasoning behind Compile include vs #if inside the file. |
Port Microsoft.Build to .NET Core
commit 7b112c5 ``` Microsoft.NET.Sdk.Web: 3.1.100-rtm.19570.3 (from 3.1.100-rtm.19569.1) Microsoft.DotNet.Cli.Runtime: 3.1.100-rtm.19570.4 (from 3.1.100-rtm.19568.3) ```
This pull request finishes the changes necessary to successfully compile Microsoft.Build for .NET Core. Lots of code which depended on features which weren't available was simply disabled, so more work will be needed to make sure that the code works on .NET Core.