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

ToolTasks Can Log MSB4181 When Cancelled #5508

Closed
danmoseley opened this issue Jul 12, 2020 · 5 comments · Fixed by #6968
Closed

ToolTasks Can Log MSB4181 When Cancelled #5508

danmoseley opened this issue Jul 12, 2020 · 5 comments · Fixed by #6968
Assignees
Labels
Area: Debuggability Issues impacting the diagnosability of builds, including logging and clearer error messages. Area: Tasks Issues impacting the tasks shipped in Microsoft.Build.Tasks.Core.dll. For consideration Used for items on the backlog to raise them to the top of that list for discussion triaged
Milestone

Comments

@danmoseley
Copy link
Member

Steps to reproduce

Assume there is a tool named "sleep" in the path. Build this project, then hit Ctrl-C.

<Project>

  <Target Name="t">
    <Exec Command="sleep 1000000"/>
  </Target>

</Project>

Actual behavior

C:\proj>dotnet msbuild test.proj
Microsoft (R) Build Engine version 16.7.0-preview-20330-08+96403c598 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

Attempting to cancel the build...
C:\proj\test.proj(4,5): warning MSB5021: Terminating the task executable "cmd" and its child processes because the build was canceled.
C:\proj\test.proj(4,5): error MSB4181: The "Exec" task returned false but did not log an error.

Expected behavior

If the task was canceled, suppress MSB4181.

MSB4181 is a grumble that the task does not follow the contract of "return false if an only if you logged an error". But in the case of cancelation, this error is useless for the user, who just hit Ctrl-C, and for the task author, who returned immediately exactly as the engine required them to. We don't want task authors to start logging bogus errors on cancelation. Just continue and fail the build with MSB4188: Build was canceled in the normal way.

Environment data

C:\proj>dotnet msbuild /version
Microsoft (R) Build Engine version 16.7.0-preview-20330-08+96403c598 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

16.7.0.33008
@rainersigwald rainersigwald added Area: Debuggability Issues impacting the diagnosability of builds, including logging and clearer error messages. Area: Tasks Issues impacting the tasks shipped in Microsoft.Build.Tasks.Core.dll. labels Jul 15, 2020
@rainersigwald rainersigwald added this to the MSBuild 16.8 milestone Jul 15, 2020
@rainersigwald
Copy link
Member

Someone else brought this up with me but I can't find it now. I agree.

@jeffkl
Copy link
Contributor

jeffkl commented Aug 12, 2020

The Copy task does this as well when ContineOnError is set to true

<Project>
  <Target Name="Build">
    <ItemGroup>
      <SourceFiles Include="Foo" />
    </ItemGroup>
    <Copy SourceFiles="@(SourceFiles)"
          DestinationFolder="$(MSBuildBinPath)"
          ContinueOnError="true"
          Retries="0" />
  </Target>
</Project>
D:\>msbuild D:\Stuff\MSB4181.proj /clp:v=m;summary;forcenoalign
Microsoft (R) Build Engine version 16.8.0-preview-20411-05+9a32a8063 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

D:\Stuff\MSB4181.proj(6,5): warning MSB3021: Unable to copy file "Foo" to "C:\Program Files (x86)\Visual Studio 2019 Preview\MSBuild\Current\Bin\Foo". Access to the path 'C:\Program Files (x86)\Visual Studio 2019 Preview\MSBuild\Current\Bin\Foo' is denied.
D:\Stuff\MSB4181.proj(6,5): warning MSB4181: The "Copy" task returned false but did not log an error.

Build succeeded.

D:\Stuff\MSB4181.proj(6,5): warning MSB3021: Unable to copy file "Foo" to "C:\Program Files (x86)\Visual Studio 2019 Preview\MSBuild\Current\Bin\Foo". Access to the path 'C:\Program Files (x86)\Visual Studio 2019 Preview\MSBuild\Current\Bin\Foo' is denied.
D:\Stuff\MSB4181.proj(6,5): warning MSB4181: The "Copy" task returned false but did not log an error.
    2 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.38

When ContinueOnError is set, I wouldn't expect to get this warning as errors were technically mutated to a warning. When I saw this, a user had suppressed MSB3021 but now their build fails because of the new warning.

@benvillalobos
Copy link
Member

Poking at this a little bit with msbuild msbuild.dev.slnf and cancelling at random times, I hit the same thing with Csc. Whatever the fix is should work with any task that's cancelled.

MSB4188 is never emitted, right?

@benvillalobos benvillalobos self-assigned this Oct 4, 2021
@benvillalobos
Copy link
Member

benvillalobos commented Oct 14, 2021

@danmoseley I don't repro this issue. This may have been fixed with our various taskhost HasLoggedErrors PR's. Closing this in favor of #5912 which I think is a larger issue with tooltasks.

EDIT: It occurred to me the other issue is specific to LC (not necessarily cancelling), so I'll stick with this as the canonical issue that ANY tooltask that is cancelled can log this error.

@benvillalobos benvillalobos added the For consideration Used for items on the backlog to raise them to the top of that list for discussion label Oct 14, 2021
@benvillalobos benvillalobos changed the title Canceling an "Exec" task causes MSB4181 instead of MSB4188 ToolTasks Can Log MSB4181 When Cancelled Oct 14, 2021
@benvillalobos benvillalobos reopened this Oct 14, 2021
@danmoseley
Copy link
Member Author

Yes that seems different. I do still see this from time to time and we keep relatively up to date with SDK builds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Debuggability Issues impacting the diagnosability of builds, including logging and clearer error messages. Area: Tasks Issues impacting the tasks shipped in Microsoft.Build.Tasks.Core.dll. For consideration Used for items on the backlog to raise them to the top of that list for discussion triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants