Cleanup workarounds #233
Cleanup workarounds #233
Conversation
| @@ -142,7 +122,5 @@ if (!(Test-Path $msbuildArtifactsDir)) | |||
|
|
|||
| $msBuildArguments | Out-File -Encoding ASCII -FilePath $msBuildResponseFile | |||
|
|
|||
| # workaround https://github.com/dotnet/core-setup/issues/1664 | |||
There was a problem hiding this comment.
Did this get fixed? The work item this pointed to isn't closed as yet?
There was a problem hiding this comment.
I tested. Its been fixed. dotnet/core-setup#2055 It just looks like the bug hasn't been closed.
build/KoreBuild.ps1
Outdated
| $preflightClpOption='' | ||
| $msbuildClpOption='/clp:Summary' | ||
| } | ||
|
|
There was a problem hiding this comment.
This wasn't just a workaround and should not be removed. None of the CI systems display MSBuild's colours. So, this update will only increase log sizes.
Same for removing this feature on other platforms.
There was a problem hiding this comment.
We disabled color and did the 'tee' on Linux to workaround dotnet/msbuild#1792 which has been fixed since mid March.
MSBuild console logger doesn't emit much color in minimal mode, so I don't think log size is a major concern: on the test run for this PR, it added a total of 76 bytes: https://travis-ci.org/aspnet/KoreBuild/jobs/226232644.
There was a problem hiding this comment.
I made this change and dotnet/msbuild#1792 was part of the reason behind it. There's no point to colour in the CI builds.
There was a problem hiding this comment.
There's no point to colour in the CI builds.
I disagree. Color is useful, esp. when there are warnings and errors. AppVeyor is the only one that strips color. TeamCity shows color when you view logs in "console view".
There was a problem hiding this comment.
Right, we disagree here. Never seen anyone use the console view in TC.
There was a problem hiding this comment.
Ok. I'll back our the this particular change and make a separate discussion issue for this.
build/KoreBuild.sh
Outdated
| "$makeFileProj" | ||
| ENDMSBUILDARGS | ||
| echo -e "$msbuild_args" >> $msbuildResponseFile | ||
|
|
||
| __exec dotnet msbuild /nologo /t:Restore /p:PreflightRestore=true /p:NetFxVersion=$netfxversion "$makeFileProj" |
There was a problem hiding this comment.
Why doesn't this command set $(RepositoryRoot) anymore?
There was a problem hiding this comment.
It was never needed for PreflightRestore.
| @@ -14,7 +14,8 @@ PublishProfiles/ | |||
| *.cache | |||
| *.docstates | |||
| _ReSharper.* | |||
| nuget.exe | |||
| *.exe | |||
| *.dll | |||
There was a problem hiding this comment.
What DLLs and EXEs now need to be ignored?
There was a problem hiding this comment.
Nothing new. Just general cleanup.
|
Ping @dougbu |
Don't suppress color output on CI.