search

ros-industrial / industrial_ci

Easy continuous integration repository for ROS repositories
Apache License 2.0
241 stars 128 forks source link

Possible issue using ccache with Clang-Tidy? #491

Open Levi-Armstrong opened 4 years ago

Levi-Armstrong commented 4 years ago

In version cmake 3.12 and newer ccache will work when using different launchers outlined here. If I remember correctly industrial_ci symbolically links ccache to g++, gcc, etc. instead of using the <LANG>_COMPILER_LAUNCHER. I believe using symbolic links instead of the cmake property prevents cland-tidy builds from leveraging ccache.

gavanderhoorn commented 4 years ago

Isn't it just the regular ccache package on Debian/Ubuntu which comes with those symlinks?

I don't remember industrial_ci doing this itself.

That's probably also the reason why you have to install special distcc support for ccache.

Edit: found it:

https://github.com/ros-industrial/industrial_ci/blob/d8c5c6ea76aae945d48a86f94bad992862f37580/industrial_ci/src/tests/source_tests.sh#L109-L112

it just installs the ccache package and then adds the symlinks provided directory containing the symlinks installed by that package to the PATH.

Levi-Armstrong commented 4 years ago

I have been reading the original PR. It is has a comment that the Ubuntu Debian sets up the symbolic links if you call update-ccache-symlinks.

gavanderhoorn commented 4 years ago

You mean this comment:

On debian/ubuntu these symlinks get set-up automatically in /usr/lib/ccache by update-ccache-symlinks

?

update-ccache-symlinks is run by default after package installation. Users don't need to run it themselves (from here):

It is normally not necessary to run update-ccache-symlinks by hand as this is done automatically when compiler packages are removed or installed.

gavanderhoorn commented 4 years ago

But your question basically is whether the symlinks could not be added to PATH, right?

Btw: CMake seems to be rapidly moving towards the point where it will include an email client.

Levi-Armstrong commented 4 years ago

Well I don't think you want to use the symlinks. I think the CMake preferred approach is to use the <LANG>_COMPILER_LAUNCHER so cmake is aware.

gavanderhoorn commented 4 years ago

I don't know about modern CMake, but I've used ccache for the past 10 years with CMake, and have always used the symlink approach. That's anecdotal and n == 1, but it seems at least for straightforward use of ccache, which I believe is what the Debian/Ubuntu package caters to, it works OK.

It could be that for more advanced usage (such as with distcc and apparently Clang-Tidy et al.) it would be advantageous to use a different approach.

That would have to be added to industrial_ci I believe, as right now, the average use-case is covered.

Levi-Armstrong commented 4 years ago

This came up because when I build my workspace without clang-tidy the ccache is around 400MB, but if I build with clang-tidy enabled it is around 40MB.

gavanderhoorn commented 4 years ago

Good chance Clang-Tidy is causing some/most invocations not getting to ccache, or some other reason it's not seeding/hitting the cache.

I'm not against changing how this is all used -- I'm not even a maintainer here -- I just thought to clarify why things are the way they are.

Levi-Armstrong commented 4 years ago

Sure, Yea I think this specific to using external launchers like Clang-Tidy. I am going to test locally and see what needs to change.

Levi-Armstrong commented 4 years ago

I cannot find how industrial_ci sets up ccache. I looked into ccache and when it installs it create symlinks to gcc and g++ and when you are suppose to update PATH="/usr/lib/ccache:$PATH" so it finds the ones located in ccache but I cannot find where this happens in industrial_ci. Does anyone know how industrial_ci gets setup here?

gavanderhoorn commented 4 years ago

I'm confused: there is nothing to setup.

If you sudo apt install ccache on Debian/Ubuntu, it installs symlinks into /usr/lib/ccache (or apparently: update-ccache-symlinks creates them).

All you need to do is then to add /usr/lib/ccache to the PATH, making sure /usr/lib/ccache appears before the regular locations of gcc, g++ and friends.

The code which does that I already linked in https://github.com/ros-industrial/industrial_ci/issues/491#issuecomment-606025701. Here it is again:

https://github.com/ros-industrial/industrial_ci/blob/d8c5c6ea76aae945d48a86f94bad992862f37580/industrial_ci/src/tests/source_tests.sh#L109-L112

Levi-Armstrong commented 4 years ago

I cannot find where the path is exported except what you have posted, but is that not just a test?

Levi-Armstrong commented 4 years ago

I believe I was mistaken, I thought source_tests.sh was a unit test of industrial_ci. This is not a unit test correct?

gavanderhoorn commented 4 years ago

It's not a test.

See:

https://github.com/ros-industrial/industrial_ci/blob/d8c5c6ea76aae945d48a86f94bad992862f37580/industrial_ci/src/ci_main.sh#L40-L57

But @ipa-mdl would be able to explain it more clearly.

Levi-Armstrong commented 4 years ago

Now it makes sense. I know what I need to change to see if it improves ccache with clang tidy.

mathias-luedtke commented 4 years ago

As @gavanderhoorn already pointed out, industrial_ci just adds the path for the ccache symlinks. The src/tests folder contains the tests that are available to the users.

The unit test-related files are kept in mockups.

We don't run clang-tidy from CMake. We just run the ROS build with -DCMAKE_EXPORT_COMPILE_COMMANDS=ON and feed the resulting files into clang-tidy.

I don't know how to set this up with ccache. One issue might be that ccache gets installed before clang-tidy. To test this, please try ADDITIONAL_DEBS clang-tidy or AFTER_INSTALL_CLANG_TIDY=update-ccache-symlinks