Skip to content

Conversation

@diizzyy
Copy link
Contributor

@diizzyy diizzyy commented Mar 31, 2024

  • Use USE_CCACHE switch, this seems to be a more common option having a quick look using Google
  • Make use of find_program() functionality introduced in CMake 3.18 to simplify code

Copy link
Member

@sjaeckel sjaeckel left a comment

Choose a reason for hiding this comment

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

DDG doesn't turn up a single hit for USE_CCACHE but G apparently finds some.

This rename falls under API breakage IMO, i.e. it must be backported to 1.3 and a warning must be added that it will change in the future.

This PR doesn't only simplify code, but also changes behavior.

I don't like the functionality change where the old solution defaults to enable its usage if ccache is found and with your change it's disabled by default. As there was no comment describing why this was done, I suspect that this was accidental!?

Also: please fix CI :)

@diizzyy
Copy link
Contributor Author

diizzyy commented Apr 3, 2024

I think the "most" correct way would be to rip it out completely and leave it up to the user compiling it or the framework to specify rather than blindly looking for a binary. https://stackoverflow.com/questions/1815688/how-to-use-ccache-with-cmake
Edit: Engrish ;-)

I can go either way :)

@sjaeckel
Copy link
Member

sjaeckel commented Apr 4, 2024

I think the "most" correct way would be to rip out of completely and leave it up to the user compiling it or the framework to specify it

Thanks, that's exactly what we should do.

Do you wanna modify this PR to simply drop it or should I?

@diizzyy
Copy link
Contributor Author

diizzyy commented Apr 4, 2024

I'll update the PR, thanks! :)

Rely on CMAKE_C_COMPILER_LAUNCHER instead of homegrown logic

See CMake's documentation for more information
https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_COMPILER_LAUNCHER.html
@diizzyy
Copy link
Contributor Author

diizzyy commented Apr 5, 2024

Updated

freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this pull request Apr 6, 2024
- remove built-in ccache detection, see
  libtom/libtommath#577
- put OPTIMIZED_CFLAGS and LTO in OPTIONS, on by default

PR:		278155
Reported by:	diizzy
@sjaeckel sjaeckel closed this Apr 7, 2024
@sjaeckel sjaeckel reopened this Apr 7, 2024
@sjaeckel sjaeckel merged commit 42b3fb0 into libtom:develop Apr 7, 2024
@sjaeckel
Copy link
Member

"Backported" via 5410d0b

sjaeckel added a commit to libtom/libtomcrypt that referenced this pull request Oct 16, 2025
This reverts commit 93f5348.

You should use the CMake flag `-DCMAKE_CXX_COMPILER_LAUNCHER=ccache`
instead.

Sibling-to: libtom/libtommath#577
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
sjaeckel added a commit to libtom/libtomcrypt that referenced this pull request Oct 16, 2025
This reverts commit 93f5348.

You should use the CMake flag `-DCMAKE_CXX_COMPILER_LAUNCHER=ccache`
instead.

Sibling-to: libtom/libtommath#577
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
sjaeckel added a commit to libtom/libtomcrypt that referenced this pull request Oct 16, 2025
This reverts commit 93f5348.

You should use the CMake flag `-DCMAKE_CXX_COMPILER_LAUNCHER=ccache`
instead.

Sibling-to: libtom/libtommath#577
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
sjaeckel added a commit to libtom/libtomcrypt that referenced this pull request Oct 17, 2025
This reverts commit 93f5348.

You should use the CMake flag `-DCMAKE_CXX_COMPILER_LAUNCHER=ccache`
instead.

Sibling-to: libtom/libtommath#577
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
sjaeckel added a commit to libtom/libtomcrypt that referenced this pull request Nov 2, 2025
This reverts commit 93f5348.

You should use the CMake flag `-DCMAKE_CXX_COMPILER_LAUNCHER=ccache`
instead.

Sibling-to: libtom/libtommath#577
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
sjaeckel added a commit to libtom/libtomcrypt that referenced this pull request Nov 3, 2025
This reverts commit 93f5348.

You should use the CMake flag `-DCMAKE_CXX_COMPILER_LAUNCHER=ccache`
instead.

Sibling-to: libtom/libtommath#577
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
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 this pull request may close these issues.

2 participants