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

How to abort bash script if warnings from dotnet CLI tool? #1131

Closed
mayka-mack opened this issue Jan 16, 2024 · 5 comments · Fixed by #1311
Closed

How to abort bash script if warnings from dotnet CLI tool? #1131

mayka-mack opened this issue Jan 16, 2024 · 5 comments · Fixed by #1311
Milestone

Comments

@mayka-mack
Copy link

mayka-mack commented Jan 16, 2024

What would be the proper way to abort a bash script if the dotnet CLI tool issues any warnings? I'm going to be formatting files in a CI pipeline during PRs and committing the changes back to the branch, so I do not want the script to move forward if CSharpier returns any warnings during the formatting process.

This is what I came up with initially, but it feels hackish and only works in my regular terminal not my CI pipeline, which seems to still include everything from stdout into the $errors variable, not just warnings and errors.

errors=$(dotnet csharpier . --write-stdout --loglevel Warning 2>&1)

if [ $? -ne 0 ] || [ -n "$errors" ]; then
    echo "$errors"
    echo "CSharpier formatting failed; aborting."
    exit 1
fi

I also tried this to see if any files remained unformatted, but CSharpier still returns exit code 0 with the --check flag when it warns there are files it cannot format.

dotnet csharpier .
dotnet csharpier --check .
if [ $? -ne 0 ]; then
    echo "CSharpier formatting failed; aborting."
    exit 1
fi
@belav
Copy link
Owner

belav commented Jan 17, 2024

You'd probably want to ensure everything can be built before running csharpier. It doesn't really care if things compile or not, it just warns you if it can't produce a syntax tree because that means it won't be able to format a given file.

--write-stdout will not actually change any files, it would just write the result of formatting each file to stdout.

You could maybe do things in two passes, with --skip-write the first pass, and if that returns an exit code of 0 then repeat without the flag.

Otherwise if you abort in the middle of formatting then you'll be in a state where only some files are formatted. That may require reverting any changes if you want to continue doing anything in your build.

In my mind the better approach is to just validate the files are formatted on a PR and depend on people running CSharpier. A pre-commit hook can make it essentially automatic, which sounds like what you are trying to do.

@mayka-mack
Copy link
Author

mayka-mack commented Jan 17, 2024

You'd probably want to ensure everything can be built before running csharpier. It doesn't really care if things compile or not, it just warns you if it can't produce a syntax tree because that means it won't be able to format a given file.

That is the eventual goal. Currently the pipeline is unable to build the software, since it depends on a proprietary interop package being installed on the build server. While I wait for that to be implemented, I'm primarily using the pipeline just for PR checks.

Otherwise if you abort in the middle of formatting then you'll be in a state where only some files are formatted. That may require reverting any changes if you want to continue doing anything in your build.

CSharpier failing to format all files would imply bigger issues in the project, so I would set up the PR pipeline to fail loudly with a warning and not commit any changes to the repo.

In my mind the better approach is to just validate the files are formatted on a PR and depend on people running CSharpier. A pre-commit hook can make it essentially automatic, which sounds like what you are trying to do.

I'm aiming for something validated on the server side to mandate consistency. I did add the CSharpier.MsBuild nuget package to the solution so that when we build it in VS it will get formatted with CSharpier. The PR pipeline would just be a last line of defense for anything that slips through the cracks.

You could maybe do things in two passes, with --skip-write the first pass, and if that returns an exit code of 0 then repeat without the flag.

Interesting. I tried using --skip-write, but I still received an exit code of 0 when warnings were issued, per the below. Is there a different way I need to be running this to have it give a non-zero exit code when there are formatting warnings?

image

@belav
Copy link
Owner

belav commented Jan 17, 2024

I'm aiming for something validated on the server side to mandate consistency. I did add the CSharpier.MsBuild nuget package to the solution so that when we build it in VS it will get formatted with CSharpier. The PR pipeline would just be a last line of defense for anything that slips through the cracks.

By default CSharpier.MsBuild will validate files are formatted when built in release mode. But if you can't actually build the project that is a whole nother problem.

Interesting. I tried using --skip-write, but I still received an exit code of 0 when warnings were issued, per the below. Is there a different way I need to be running this to have it give a non-zero exit code when there are formatting warnings?

CSharpier doesn't consider it an error if a file cannot be formated. It just warns the user that it can't be formatted because it can't compile the file. Actually it parses the file into a syntax tree, maybe I should update that message... anyway. You'd need pipe the output of dotnet csharpier --skip-write to something else and look for the warnings if you want to consider them a failure condition.

@rodion-m
Copy link

I also have the same problem when trying to pass this linter into aider.
How I have to solve it in a tricky way like:

#!/bin/bash

# Run the dotnet csharpier command and capture its output
output=$(dotnet csharpier "$@")

# Print the output
echo "$output"

# Check if the output contains "Warning" or "error"
if echo "$output" | grep -q -E "Warning|error"; then
    # If it does, exit with a non-zero status code
    exit 1
else
    # If not, exit with the original exit code of dotnet csharpier
    exit ${PIPESTATUS[0]}
fi

@belav Could you please return non-zero exit code when a compilation error occurs?

@belav
Copy link
Owner

belav commented Jul 25, 2024

@rodion-m it seems reasonable to return a non-zero exit code for compilation errors. From what I've seen prettier does the same. I thought CSharpier used to do this and was changed on purpose but I can't find any indication of that being true.

@belav belav added this to the 0.29.0 milestone Jul 25, 2024
belav added a commit that referenced this issue Jul 27, 2024
@belav belav closed this as completed in 0df263d Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants