Skip to content

Commit

Permalink
[trivial] Fix clang-tidy check.
Browse files Browse the repository at this point in the history
The old testing code would simply ignore all errors.
  • Loading branch information
jendrikseipp authored Jan 7, 2024
1 parent 9c5107f commit aefe96a
Showing 1 changed file with 20 additions and 10 deletions.
30 changes: 20 additions & 10 deletions misc/style/run-clang-tidy.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
import utils


IGNORES = [
"'cplex.h' file not found [clang-diagnostic-error]",
"'soplex.h' file not found [clang-diagnostic-error]",
]


def check_search_code_with_clang_tidy():
# clang-tidy needs the CMake files.
build_dir = os.path.join(REPO, "builds", "clang-tidy")
Expand Down Expand Up @@ -85,22 +91,26 @@ def check_search_code_with_clang_tidy():
"-checks=-*," + ",".join(checks)]
print("Running clang-tidy: " + " ".join(pipes.quote(x) for x in cmd))
print()
# Don't check returncode here because clang-tidy exits with 1 if it finds any issues.
try:
output = subprocess.check_output(cmd, cwd=DIR, stderr=subprocess.STDOUT).decode("utf-8")
except subprocess.CalledProcessError as err:
print("Failed to run clang-tidy-12. Is it on the PATH?")
print("Output:", err.stdout)
return False
p = subprocess.run(cmd, cwd=DIR, text=True, capture_output=True, check=False)
except FileNotFoundError:
sys.exit(f"run-clang-tidy-12 not found. Is it on the PATH?")
output = f"{p.stdout}\n{p.stderr}"
errors = re.findall(r"^(.*:\d+:\d+: .*(?:warning|error): .*)$", output, flags=re.M)
for error in errors:
print(error)
if errors:
filtered_errors = [error for error in errors if not any(ignore in error for ignore in IGNORES)]

if filtered_errors:
print("Errors and warnings:")
for error in filtered_errors:
print(error)
fix_cmd = cmd + [
"-clang-apply-replacements-binary=clang-apply-replacements-12", "-fix"]
print()
print("You may be able to fix these issues with the following command: " +
print("\nYou may be able to fix some of these issues with the following command:\n" +
" ".join(pipes.quote(x) for x in fix_cmd))
sys.exit(1)
elif not errors and p.returncode != 0:
sys.exit(p.stderr)


check_search_code_with_clang_tidy()

4 comments on commit aefe96a

@silvansievers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After pushing the this commit to my fork, the Github actions fail: https://github.com/silvansievers/downward/actions/runs/7446417101/job/20256623897 I assume this shouldn't be the case?

@silvansievers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, this seems to be the persisting problem we have with clang-tidy since a long time.

@FlorianPommerening
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still would be good to see the action pass before we merge this change. Maybe we should try one of the workarounds. The issue is with the github ubuntu image (actions/runner-images#8659) it has the wrong version of some libc or stdlib (I think). It only affects the combination of clang (14?) with c++ 20.

There are workarounds that essentially downgrade these versions of the standard libraries. For example here: https://github.com/mjp41/workaround8649/blob/main/action.yml (lines 16-18). I think the main thing to do to adapt this to our setup would be to ensure that the downgrade only happens in the cases where we need it (i.e., when compiling with clang).

@jendrikseipp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but I'll leave that to someone else since I'm on holiday :-)

Please sign in to comment.