Adds default outputProperties Item #12030
Conversation
…rks if the project is multitargetted. Is used by console logger
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets
Outdated
Show resolved
Hide resolved
…targets changes item name to a more clear representation of it's purpose Co-authored-by: Nirmal Guru <[email protected]>
…ir to save time in console logger
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets
Outdated
Show resolved
Hide resolved
dsplaisted
left a comment
dsplaisted
left a comment
There was a problem hiding this comment.
It would be great to add test coverage for this. That would require waiting for the MSBuild change to go in and flow to this repo.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets
Outdated
Show resolved
Hide resolved
…targets Co-authored-by: Daniel Plaisted <[email protected]>
…targets Co-authored-by: Rainer Sigwald <[email protected]>
|
@laurenprinn has the msbuild change flowed in yet so you can add tests? |
They were. I'm working on writing tests. |
…argetFrameworks is defined
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…#7297) * Don't space first ProjectConfigurationDescription * Log project-file messages with disambiguator * Improve diagnosability of ProcessConsoleLoggerSwitches * Default loggers ProjectConfigurationDescription dotnet/sdk#12030 discovered that the .NET SDK's default build process didn't log TargetFramework details for normal projects. This turned out to be because the .NET SDK attaches a custom logger, which caused MSBuild to set up a ConfigurableForwardingLogger in front of the ConsoleLogger that forwarded based on verbosity which wasn't high enough to include the ProjectEvaluationFinished event. Pass a new ConfigurableForwardingLogger parameter from the command line handler to instruct it to pass those events even at lower verbosities.
Also make it more durable by avoiding specifying old .NET Core versions.
|
Tests are currently failing because the MSBuild.exe used for full-framework tests don't have dotnet/msbuild#7297 yet. They should pick it up in the next engineering services rollout, I think. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rainersigwald
left a comment
rainersigwald
left a comment
There was a problem hiding this comment.
@dsplaisted / @marcpopMSFT I think this is good to go now. One of y'all want to take a look?
This could go to 6.0.4xx, but I think we should just take it to the 7 previews to get better preview coverage and feedback before wide deployment.
Nirmal4G
left a comment
Nirmal4G
left a comment
There was a problem hiding this comment.
Doesn't seem like it's going to break anything so adding it to 6.0.4xx seems okay with me!
|
@rainersigwald am I correct that the only non-formatting changes in this are the test and the ProjectConfigurationDescription ItemGroup? Just wanted to make sure there wasn't anything else to review. I could be convinced either way on 6.0.4xx as the PR's been open a long time so not really a rush. |
Grr, sorry! I didn't realize I'd been reviewing with ignore whitespace. I'll fix those.
Correct. I feel pretty strongly we shouldn't put this in 6.0.400--I think it might break GitHub Actions log parsing: actions/setup-dotnet#97 will need to come back. |
src/Tests/Microsoft.NET.Build.Tests/ProjectConfigurationDescriptionTest.cs
Outdated
Show resolved
Hide resolved
|
Didn't know that actions do reading from console. What about VS Integration and Pipeline tasks (VSBuild, DotNetCLI)? Do we have to notify them as well? |
We do not. The Azure DevOps tasks that wrap MSBuild inject a custom logger rather than reading stdout. |
|
The last tests failed because they ran on a machine with |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@marcpopMSFT I think this is ready to go in, unless we think it's too late for 7.0.100. |
|
I think we still have time in 7.0.100 as we're not yet locking down. |
|
Pardon the noisy comment. Just wanted to drop in and say this is a major quality of life improvement for me working on dotnet/runtime. Thanks y'all for adding this! |
8 participants
The console logger is adding an additional feature where users can define properties to be printed in error/warning messages by editing the Item outputProperties. This PR will have outputProperties default to at least TargetFrameworks if the project is multitargeted.
dotnet/msbuild#5414