csharpier icon indicating copy to clipboard operation
csharpier copied to clipboard

csharpier as msbuild dependency doesn't fail the github action anymore

Open OneCyrus opened this issue 1 year ago • 1 comments

Environments:

  • CSharpier Version: 0.29.2
  • Running CSharpier via: dotnet
  • Operating System: Ubuntu 22.04
  • .csharpierrc Settings:
  • .editorconfig Settings:

Steps to reproduce:

dotnet build --configuration Release /p:CSharpier_LogLevel=Error

Expected behavior:

we are running csharpier as msbuild dependency and want the build to fail of the code is not formatted. it used to work before but currently even when an error is detected the build succeeds and the github action workflow doesn't fail.

formatting error is detected correctly:

Error /home/runner/work/automation-data/automation-data/Poseidon/Poseidon/GqlSchema/ITSM/Itsm.cs - Was not formatted.
    ----------------------------- Expected: Around Line 1 -----------------------------
    using System.Linq.Expressions;
    using System.Reflection;
    ----------------------------- Actual: Around Line 1 -----------------------------
    using Aveniq.Poseidon.GqlSchema.CMDB;
    using Aveniq.Poseidon.GqlSchema.SMDB;

build fails:

Build failed.

    0 Warning(s)
    1 Error(s)

Actual behavior:

build succeeds and the exit code doesn't fail the github action job.

Build succeeded.

    0 Warning(s)
    0 Error(s)

OneCyrus avatar Oct 09 '24 11:10 OneCyrus

this started to happen with 0.29.0. probably the issue is here:

https://github.com/belav/csharpier/blob/5d73fe3f9aea7ea9e47858177d44b9ff76683b0a/Src/CSharpier.MsBuild/build/CSharpier.MsBuild.targets#L26

OneCyrus avatar Oct 09 '24 12:10 OneCyrus

@belav as this is still blocking us, are you aware of a workaround to fail the build when csharpier is reporting an error?

OneCyrus avatar Dec 06 '24 07:12 OneCyrus

@OneCyrus hey sorry I lost track of this one, I'll see if it's something I can resolve today for you

belav avatar Dec 06 '24 17:12 belav

@OneCyrus IgnoreExitCode is behaving inconsistently between linux and windows. That was added to cleanup the output.

With the new PR, windows outputs this when it runs into unformatted files

    Project -> D:\a\csharpier\csharpier\Tests\MsBuild\TestCases\UnformattedFileCausesError\bin\Release\net8.0\Project.dll
  
  Build FAILED.
  
  EXEC : error D: /a/csharpier/csharpier/Tests/MsBuild/TestCases/UnformattedFileCausesError/UnformattedFile.cs - Was not formatted. [D:\a\csharpier\csharpier\Tests\MsBuild\TestCases\UnformattedFileCausesError\Project.csproj]
      0 Warning(s)
      1 Error(s)
  

vs linux, which duplicates the error.

  /home/runner/.nuget/packages/csharpier.msbuild/0.0.1/build/CSharpier.MsBuild.targets(27,5): error MSB3073: The command ""/usr/share/dotnet/dotnet" exec "/home/runner/.nuget/packages/csharpier.msbuild/0.0.1/build/../tools/csharpier/net9.0/dotnet-csharpier.dll"  --check --no-msbuild-check --compilation-errors-as-warnings "/home/runner/work/csharpier/csharpier/Tests/MsBuild/TestCases/UnformattedFileCausesError" > /dev/null " exited with code 1. [/home/runner/work/csharpier/csharpier/Tests/MsBuild/TestCases/UnformattedFileCausesError/Project.csproj]
  
  Build FAILED.
  
  /home/runner/.nuget/packages/csharpier.msbuild/0.0.1/build/CSharpier.MsBuild.targets(27,5): error MSB3073: The command ""/usr/share/dotnet/dotnet" exec "/home/runner/.nuget/packages/csharpier.msbuild/0.0.1/build/../tools/csharpier/net9.0/dotnet-csharpier.dll"  --check --no-msbuild-check --compilation-errors-as-warnings "/home/runner/work/csharpier/csharpier/Tests/MsBuild/TestCases/UnformattedFileCausesError" > /dev/null " exited with code 1. [/home/runner/work/csharpier/csharpier/Tests/MsBuild/TestCases/UnformattedFileCausesError/Project.csproj]
      0 Warning(s)
      1 Error(s)

I'm thinking release this as is, and come back later and try to clean up the duplication.

belav avatar Dec 06 '24 19:12 belav

I see. While it's not perfect I would still prefer duplicated messages if this would result in reporting issues as errors again.

OneCyrus avatar Dec 06 '24 19:12 OneCyrus

I think inconsistency is in that on Windows error message, due to colon (:) in path, recognized as standard error message (https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-diagnostic-format-for-tasks?view=vs-2022). You should probably add IgnoreStandardErrorWarningFormat to Exec if it is not your intention. Or add command line option to format errors in MSBuild compatible way.

PetSerAl avatar Dec 06 '24 22:12 PetSerAl

I suspect changing CSharpier output from Error path to Error: path should make it to be recognized as MSBuild error both on Windows and Linux, and also take care of extra space added after : on Windows, which make path unclickable in VS Code terminal in some cases.

PetSerAl avatar Dec 06 '24 22:12 PetSerAl