Clang-tidy/clang-format being installed clashed with system binaries

[burgholzer]

Hey 👋

First of all, thanks for this project and all the work you put in over the last couple of weeks to improve it.

We recently adopted the action for our project https://github.com/cda-tum/qcec.
Since we initially had problems with v1 and the compilation database, we switched to using the Python package instead.

After the v2 release, I tried to switch to the action again (cda-tum/mqt-qcec#132), which seemed to work great. Unfortunately (as I later found out in cda-tum/mqt-qcec#136), under the hood, this silently failed due a segmentation fault in clang-tidy-14.
In that PR, I tried all kinds of twists and turns to make things work, but neither downgrading the GitHub runner image nor the clang-tidy version produced proper results.
In the end, I switched back to using pipx and the core cpp-linter package, which works effortlessly for now 🎉.

My best guess after a long night of debugging why all my efforts have been failing in that PR is that GitHub runners come with certain versions of the LLVM toolchain by default (typically the three latest versions). For Ubuntu-22.04 that's clang/clang-tidy/clang-format versions 12, 13, and 14.
When running the cpp-linter-action, it tries to automatically download and install the respective clang-format and clang-tidy versions using clang-tools. It's only a guess, but somehow this messes up the subsequent cpp-linter run (see, e.g., https://github.com/cda-tum/qcec/runs/8217659640?check_suite_focus=true#step:4:114 where it can't find the <stddef.h> system header all of a sudden).

One possible fix could be to check whether the version requested from the cpp-linter-action is available on the current system. If yes, then use the existing binaries. If no, proceed as before.
This also allows to use this action under Ubuntu-22.04 with clang-tidy-14 (it would probably also not hurt to document the segfault issue of the clang-tools binary for clang-tidy-14 under Linux here).

Let me know, what you think. It would be great to switch back to using the GitHub action after all (let alone that it contributes to the Used by ... statistic 😃 )

[2bndy5]

this silently failed due a segmentation fault in clang-tidy-14.

Yes this is known problem in the repo we get our static binaries from. It's due to a difference in glibc used in Ubuntu 20.04 vs Ubuntu 22.04. It was caused by a change in clang's use of debugging symbols... We are working on a fix for this, but clang-format-14 for macOS is still resulting in 1.5 GB static binary.

For Ubuntu-22.04 that's clang/clang-tidy/clang-format versions 12, 13, and 14.

Use of Ubuntu 22.04 in github runners is still in beta, so we can't confidently suggest this approach. However, the rollout progress for that image has greatly increased since I last checked. See the progress here.

it can't find the <stddef.h> system header all of a sudden

I only noticed this in our test on a Windows runner in which the clang version expected by the compiler was greater than the clang-tidy version used by cpp-linter. I'm still not exactly sure what the problem is there. I didn't see it in our Ubuntu tests though - that's new.

One possible fix could be to check whether the version requested from the cpp-linter-action is available on the current system

This was our original approach in development, but the way clang tools are invoked is rather system dependent. For example, clang-tidy v12 can be invoked on Linux as clang-tidy-12 or clang-tidy if it is the only version installed. On Windows, there can only be 1 version installed (& added to PATH), and despite the version, it is always invoked with clang-tidy (or path/to/LLVM/bin/clang-tidy.exe if installed to a custom location).

Clearly there are ways around this, but ultimately, we didn't want to mess with binaries that we didn't install ourselves because symlinking to a binary found in PATH is problematic for a number of reasons. We need the symlink for cross-platform friendly invocation.

We really should be using a venv in the composite action's steps, but I'm not sure if that would also require we have a certain version of python at our disposal. Making sure there is a specific version of python available will lead to increased runtime.

If we switch back to using KyleMayes/install-llvm-action to install a certain version of LLVM that isn't available, there would be a 1+ minute spike in runtime without using runner cache. Plus, it installs way more than we really need for this action.

---

Ultimately, I think you've done the best thing there is: Use the core py pkg in a env that you control.

[burgholzer]

this silently failed due a segmentation fault in clang-tidy-14.

Yes this is known problem in the repo we get our static binaries from. It's due to a difference in glibc used in Ubuntu 20.04 vs Ubuntu 22.04. It was caused by a change in clang's use of debugging symbols... We are working on a fix for this, but clang-format-14 for macOS is still resulting in 1.5 GB static binary.

Yeah, I have seen that issue and appreciate the work towards fixing the underlying issue!
For what it is worth, clang-format-14 also segfaults under the Ubuntu-20.04 GitHub runner. See https://github.com/cda-tum/qcec/runs/8218075258?check_suite_focus=true#step:4:5104.
So using version: 14 under Linux seems to be broken in general.

For Ubuntu-22.04 that's clang/clang-tidy/clang-format versions 12, 13, and 14.

Use of Ubuntu 22.04 in github runners is still in beta, so we can't confidently suggest this approach. However, the rollout progress for that image has greatly increased since I last checked. See the progress here.

Ubuntu-20.04 provides versions 10, 11, and 12, which would technically cover the default configuration of the cpp-linter.

it can't find the <stddef.h> system header all of a sudden

I only noticed this in our test on a Windows runner in which the clang version expected by the compiler was greater than the clang-tidy version used by cpp-linter. I'm still not exactly sure what the problem is there. I didn't see it in our Ubuntu tests though - that's new.

For what it's worth, this also happens if clang-12 is used for generating the compilation database and version: 12 is used in the cpp-linter-action on an Ubuntu-20.04 runner. See https://github.com/cda-tum/qcec/runs/8218075258?check_suite_focus=true#step:4:5173

One possible fix could be to check whether the version requested from the cpp-linter-action is available on the current system

This was our original approach in development, but the way clang tools are invoked is rather system dependent. For example, clang-tidy v12 can be invoked on Linux as clang-tidy-12 or clang-tidy if it is the only version installed. On Windows, there can only be 1 version installed (& added to PATH), and despite the version, it is always invoked with clang-tidy (or path/to/LLVM/bin/clang-tidy.exe if installed to a custom location).

That might of course be a problem. However, since this repository is intended as a GitHub action, it will in almost all cases run on GitHub hosted runners. And for those, there are guarantees on the pre-installed versions (see https://github.com/actions/runner-images#support-policy).

Clearly there are ways around this, but ultimately, we didn't want to mess with binaries that we didn't install ourselves because symlinking to a binary found in PATH is problematic for a number of reasons. We need the symlink for cross-platform friendly invocation.

On the cross-platform friendly part. Trying to run the action on a macOS runner also hopelessly failed. See https://github.com/cda-tum/qcec/runs/8218221741?check_suite_focus=true
Is cross-platform support really such a big deal for GitHub actions? I would expect most people to create a separate linter workflow for running this action. And I see almost no reason for that linter workflow to not run under Linux.

We really should be using a venv in the composite action's steps, but I'm not sure if that would also require we have a certain version of python at our disposal. Making sure there is a specific version of python available will lead to increased runtime.

At least for GitHub Actions, there is https://github.com/actions/setup-python, which allows to effortlessly setup Python environments in workflows. Might be worth a look.

If we switch back to using KyleMayes/install-llvm-action to install a certain version of LLVM that isn't available, there would be a 1+ minute spike in runtime without using runner cache. Plus, it installs way more than we really need for this action.

I see why that is undesirable and totally agree on not pursuing that route.

Ultimately, I think you've done the best thing there is: Use the core py pkg in a env that you control.

For our use case that is perfectly fine. However, I can't believe that we are the only ones facing problems with properly setting up this action. Maybe some of the points raised above help in making it easier for future users to adopt the action.

[2bndy5]

And I see almost no reason for that linter workflow to not run under Linux.

We have had multiple users ask about how to the action in a Windows-only workflow. It's not always the case to have a separate workflow just for linting. In C++, it's very common to have a linter check before compiling in the same workflow because the database is generated when the CMake build env is configured.

However, I can't believe that we are the only ones facing problems with properly setting up this action.

We just released v2 less than a week ago. It is likely that others have pinned the action to a specific commit SHA or @v1

I'm thankful for your feedback, and I do intend to look closer at the static binaries that we are using.

[burgholzer]

And I see almost no reason for that linter workflow to not run under Linux.

We have had multiple users ask about how to the action in a Windows-only workflow. It's not always the case to have a separate workflow just for linting. In C++, it's very common to have a linter check before compiling in the same workflow because the database is generated when the CMake build env is configured.

True true. Didn't think that through fully.

However, I can't believe that we are the only ones facing problems with properly setting up this action.

We just released v2 less than a week ago. It is likely that others have pinned the action to a specific commit SHA or @v1

I'm thankful for your feedback, and I do intend to look closer at the static binaries that we are using.

Thanks again for all your efforts and for responding so quickly!
If there is anything, where we could help, let us know.
At the moment, we have a working solution using the core package so it's all good.

Just to summarize: we didn't manage to get the GitHub action running properly in any of the following configurations for our project:

GitHub Runner	cpp-linter version	compilation database compiler	result	run
Ubuntu-22.04	14	clang-14	segfault	https://github.com/cda-tum/qcec/runs/8217114823?check_suite_focus=true
Ubuntu-22.04	13	clang-14	stdlib header not found	https://github.com/cda-tum/qcec/runs/8217659640?check_suite_focus=true
Ubuntu-22.04	13	clang-13	stdlib header not found	https://github.com/cda-tum/qcec/runs/8217934578?check_suite_focus=true
Ubuntu-22.04	12	clang-12	stdlib header not found	https://github.com/cda-tum/qcec/runs/8218038348?check_suite_focus=true
Ubuntu-20.04	14	clang-12	segfault	https://github.com/cda-tum/qcec/runs/8218075258?check_suite_focus=true
macOS-latest	14	clang-13	No such file or directory: 'clang-tidy-14'	https://github.com/cda-tum/qcec/runs/8218221741?check_suite_focus=true

One last guess at the potential source of the problem:
Could it be that the compiler that is listed in the compilation database is (for whatever reason) not compatible with the clang-tidy installed from clang-tools?

[2bndy5]

not compatible with the clang-tidy installed from clang-tools?

This is what I intend to find out. We haven't tested v14 due to the seg fault bug. It may be that statically compiling against a certain dist of libc++ has unintended consequences.

[2bndy5]

@burgholzer We just published a new version of clang-tools-pip pkg that should use the system's dynamically linked binaries when the version exactly matches the user input; version 12 will only use the system's v12.0.0 binaries if they are installed, and version 12.0.1 will only use v12.0.1 if installed... Hopefully, that should resolve most problems with clang-tidy not finding std libs.

[burgholzer]

@burgholzer We just published a new version of clang-tools-pip pkg that should use the system's dynamically linked binaries when the version exactly matches the user input; version 12 will only use the system's v12.0.0 binaries if they are installed, and version 12.0.1 will only use v12.0.1 if installed... Hopefully, that should resolve most problems with clang-tidy not finding std libs.

Thanks! I'll try that out over the next couple of days and get back to you 👍🏼

[2bndy5]

That would much appreciated. User feedback is a great way to contribute!

[shenxianpeng]

I can still see the error in this runs https://github.com/shenxianpeng/gcov-example/actions/runs/3224876381

[/usr/include/stdio.h:33:10 [clang-diagnostic-error]: usr/include/stdio.h#L33]
'stddef.h' file not found

This error is reported by clang-tidy. Currently, we do not support disabling clang-tidy to run only clang-format.

[2bndy5]

This error is reported by clang-tidy. Currently, we do not support disabling clang-tidy to run only clang-format.

Actually, this can be done by setting tidy-checks to -*.

---

To be clear, clang-tidy is having trouble with analyzing the includes in stdio.h

/usr/include/stdio.h:33:10: error: 'stddef.h' file not found [clang-diagnostic-error]
#include <stddef.h>

This is what made me think there was a problem related to the C++ distribution that is installed. Although I'm not entirely clear where clang-tidy is looking for the stddef.h header. Typically, the AST generated by clang uses absolute paths, so it would be nice to get an AST tree dump from a machine that can reproduce this locally.

[shenxianpeng]

Actually, this can be done by setting tidy-checks to -*.

Okay, it works 👍

[2bndy5]

I found this stackoverflow Q/A interesting.

[shenxianpeng]

I found this stackoverflow Q/A interesting.

I tried to install clang before running cpp-linter-action (tested clang, clang-12, and clang-10) and the error is still the same.

[shenxianpeng]

I can reproduce error: 'stddef.h' file not found, the reason should be clang-tools still downloads and use non-native clang-tidy binary.

something like the this described here:

because our pre-built binary isn't able to locate one of the clang include paths that you have installed on your machine.

Workaround

Using native binary can avoid this problem.

Reproduce

Using native clang-tidy works fine

gitpod /workspace/gcov-example (master) $ /usr/bin/clang-tidy-10 -checks="boost-*,bugprone-*,performance-*,readability-*,portability-*,modernize-*,clang-analyzeines-*" main.c
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "main.c"
No compilation database found in /workspace/gcov-example or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
1 warning generated.
Suppressed 1 warnings (1 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
gitpod /workspace/gcov-example (master) $ which clang-tidy-10
/usr/bin/clang-tidy-10

Download clang-tidy with clang-tools. clang-tools still download clang-tidy binary even though native binary exists.

gitpod /workspace/gcov-example (master) $ clang-tools -i 10
Found a installed version of clang-format: 10.0.0 at /usr/lib/llvm-10/bin/clang-format
downloading clang-format (version 10)
    |████████████████████| 100% (of 2647984 bytes)
Installing clang-format-10 to /home/gitpod/.local/bin/
symbolic link /home/gitpod/.local/bin/clang-format already exists. Use '-f' to overwrite. Leaving it as is.
Found a installed version of clang-tidy: 10.0.0 at /usr/lib/llvm-10/bin/clang-tidy
downloading clang-tidy (version 10)
    |████████████████████| 100% (of 34110384 bytes)
Installing clang-tidy-10 to /home/gitpod/.local/bin/
symbolic link /home/gitpod/.local/bin/clang-tidy already exists. Use '-f' to overwrite. Leaving it as is.

Using the downlaod binary run it again, and the error occurs.

gitpod /workspace/gcov-example (master) $ which clang-tidy-10
/home/gitpod/.local/bin/clang-tidy-10
gitpod /workspace/gcov-example (master) $ clang-tidy-10 -checks="boost-*,bugprone-*,performance-*,readability-*,portability-*,modernize-*,clang-analyzeines-*" main.c
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "main.c"
No compilation database found in /workspace/gcov-example or any parent directory
json-compilation-database: Error while opening JSON database: No such file or directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
Running without flags.
1 error generated.
Error while processing /workspace/gcov-example/main.c.
/usr/include/stdio.h:33:10: error: 'stddef.h' file not found [clang-diagnostic-error]
#include <stddef.h>
         ^
Found compiler error(s).
gitpod /workspace/gcov-example (master) $ 

[2bndy5]

Download clang-tidy with clang-tools. clang-tools still download clang-tidy binary even though native binary exists.

This might have something to do with comparing a tuple of strings instead of a tuple of integers. See https://github.com/cpp-linter/clang-tools-pip/blob/ce41d4f571abeffc4bfa94b65682804f2cda1613/clang_tools/install.py#L59

We should have verified the CI more thoroughly... We missed this in the CI logs (evidently).

[shenxianpeng]

@burgholzer We just published a new version of clang-tools-pip pkg that should use the system's dynamically linked binaries when the version exactly matches the user input; version 12 will only use the system's v12.0.0 binaries if they are installed, and version 12.0.1 will only use v12.0.1 if installed... Hopefully, that should resolve most problems with clang-tidy not finding std libs.

We just published another new version v2.2.0 of cpp-linter-action to use system binaries, it supports version '9','10', '11', '12' but not '13' and '14', because they are not included in ubuntu-latest (ubuntu 20.04), but in ubuntu 22.04.. they will be soon though... actions/runner-images#6399

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.