Bug 277204 - *: ports misusing WITH_CCACHE_BUILD and NO_CCACHE
Summary: *: ports misusing WITH_CCACHE_BUILD and NO_CCACHE
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Port Management Team
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-20 20:46 UTC by Benjamin Takacs
Modified: 2024-02-29 15:20 UTC (History)
10 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Takacs 2024-02-20 20:46:50 UTC
grepping for WITH_CCACHE_BUILD in the ports tree reveals some ports misusing this user setting. WITH_CCACHE_BUILD should be only set by the user and only read by Mk/bsd.ccache.mk which in turn sets CCACHE_ENABLED.

Both of which is violated by some ports.

Ports setting WITH_CCACHE_BUILD:
emulators/libretro-pcsx2 (beyert@cs.ucr.edu)
graphics/digikam (kde@FreeBSD.org)

Ports reading WITH_CCACHE_BUILD instead of CCACHE_ENABLED:
lang/rust (rust@FreeBSD.org)
lang/rust-bootstrap (rust@FreeBSD.org)
misc/valentina (yuri@FreeBSD.org)
graphics/darktable (dumbbell@FreeBSD.org)
graphics/rawtherapee (mandree@FreeBSD.org)
net-im/telegram-desktop (-)
devel/cmake-core (kde@FreeBSD.org)

Additionally NO_CCACHE should be only set by a port, but there are some reading it anyway:
<most of the ports reading WITH_CCACHE_BUILD>
java/openjdk8 (java@FreeBSD.org)

maintainers put in CC
Comment 1 Matthias Andree freebsd_committer freebsd_triage 2024-02-20 21:02:41 UTC
Benjamin, good idea, but not practical to fix - for graphics/rawtherapee, CCACHE_ENABLED seems to be set too late, even after .include <bsd.port.pre.mk> it's not yet set -- and I need to hack CMAKE_ARGS to actually make rawtherapee use ccache - and I don't want to turn some easily readable .if clause into non-readable variable expansion hacks to delay expansion. CCACHE isn't a PORT_OPTION, so helper variables don't cut the mustard.

If you can lend a hand with graphics/rawtherapee and propose a fix that I do not currently see... feel free to send proposals, else I'll need to keep things as they are and continue emulating bsd.ccache.mk.
Comment 2 Gleb Popov freebsd_committer freebsd_triage 2024-02-21 18:16:15 UTC
You're correct in your analysis, Benjamin. WITH_CCACHE_BUILD uses should be fixed and I even suspect that bsd.ccache.mk should become Features/ccache.mk

Matthias, I took a look on graphics/rawtherapee and wonder why all these knobs are really there. Using GCC, binutils, forcing LTO, smoke-testing in post-install and replaying what bsd.ccache.mk does seems all wrong to me. Comments mention GCC 9, which probably means that all this stuff was put in the port long ago and might be not relevant anymore.

Anyways, there is a new 5.10 release available, can you please look into it?
Comment 3 Matthias Andree freebsd_committer freebsd_triage 2024-02-21 21:19:38 UTC
Gleb, thanks for looking at rawtherapee. I have 5.10 cooking, test compiles on the off-versions got delayed due to the usual rust rebuild madness (because the upstream package for 13.2 wasn't up to date with the port), and I successively tried getting rid of all that you criticize.

The thing about rawtherapee is, we don't have OpenCL, but only OpenMP, and it is computationally VERY expensive (as raw camera image processing is in general, with Bayer/Foveon/TransX demosaicing, denoising and thereabouts), so we need to squeeze every bit out of the CPU that we can.  RT would currently build against GCC 12.3 on amd64, even without too many of the force options, so...

- we should use LTO because upstream recommends it and it seems to increase performance on cross-compilation-unit calls,
- which incurs the forcing of binutils to avoid linker failures,
- we need to work around our cmake not being ccache friendly, because - being written in mostly C++ - rawtherapee is slow enough to compile (and I could argue that our ccache.mk is broken if it sets CCACHE_ENABLE so late that I can't derive CMAKE_ARGS with a simple .if... - it only gets set in the "post" phase, not in port.options.mk, not in port.pre.mk)
- last time I tried, run-time performance with LLVM compiler was inferior to GCC's (might be worth retrying, but that would delay the RT 5.10 upgrade to at least next week or the week after) and I'm not sure what the state of libstdc++ vs. libc++ affairs currently is.

Smoke-testing in post-install is something I found necessary on my end because there used to be builds that would crash on start, and I don't want them to escape my testing.  Without test images in the source folder, these will be skipped. Unfortunately, poudriere has no support to run "make test" and depends properly for "poudriere bulk" and "poudriere testport". 

Arguably I could try to provide such stuff for .mk scripts now that some of the people who were constantly getting in my way or asking for perfection instead of an -exp run are no longer part of portmgr@ and haven't been seen as part of the project in a long time, but that, too, is beyond my available time right now.

Back to the rawtherapee 5.10 topic, I can remove some things, but none of the "lighter" ccache approaches would actually use ccache to build, so the ccache variable abuse that Benjamin pointed out cannot be fixed yet, I've tried that.
Anything but the way it's written currently ends up with a hard-coded /usr/local/bin/g++12 as a compiler (with hardcoded path, so we can't BINARY_ALIAS or play PATH tricks) as a result of the cmake run, and what the port currently does seems to be one of the official cmake ways to get ccache into play.
Comment 4 Benjamin Takacs 2024-02-22 09:09:19 UTC
(In reply to Gleb Popov from comment #2)
As features are still loaded in the post section of bsd.port.mk, I don't see how it would help these ports to detect if ccache support is enabled. By allowing them  to read WITH_CCACHE and making only setting WITH_CCACHE without setting CCACHE_DIR a hard error instead of a warning? And if I read bsd.port.mk right there still could be cases when WITH_CCACHE and WITHOUT_CCACHE are set, when the user sets WITH_CCACHE=yes WITHOUT_CCACHE_PORTS="someport". So ports wanting to detect if ccache is enabled would have to use  .if defined(WITH_CCACHE) && !defined(WITHOUT_CCACHE)  after WITH{,OUT}_<feature> and WITH_<feature>_PORTS is processed in the options section.

Additionally bsd.ccache.mk reads the undocumented variable NOCCACHE (and sets NO_CCACHE=t if it was set); still sets CCACHE_ENABLED=yes if WITH_CCACHE_BUILD and NO_CCACHE are set (but no port that disables ccache via NO_CCACHE should try to detect ccache support, so that's just a minor correctness issue); sets NO_CCACHE in some cases.
Comment 5 Gleb Popov freebsd_committer freebsd_triage 2024-02-22 12:31:17 UTC
(In reply to Benjamin Takacs from comment #4)
There are several concepts interleaved here, let me explain.

* Feature is a sort of OPTION but general enough to be applied to **any** port. DEBUG is an example of a Feature, because almost every port can be build in the "debug" mode.

In Ports Framework Features are implemented as .mk files placed in Mk/Features/ subdir, just like Uses are placed into Mk/Uses/. The Mk/bsd.ccache.mk file probably predates introduction of Mk/Features, but serves the same purpose.

So, moving just bsd.ccache.mk to Features/ would solve all the issues you outlined, but it is still a step in right direction, as it makes the framework more consistent.

* Mk/Features/ also provide a unified interface for both users and port developers.

If a Feature is called FOO an user can put WITH_FOO=yes or WITHOUT_FOO=yes into the make.conf or make command line to enable or disable the Feature globally. WITH_FOO_PORTS and WITHOUT_FOO_PORTS allows for enabling/disabling a Feature for a given set of ports.

With https://reviews.freebsd.org/D43949 landed it will become possible for WITH_FOO_PORTS to cancel WITHOUT_FOO and vice versa.

* Features are not expected to be self-contained. Take a look at Mk/Features/lto.mk - it only sets some variables that are later gets consumed by Uses and the rest of Framework. In the same way, a Uses may be taught to look for some variables that might be set by an enabled feature.

So to fix the "order" problem we first should clearlt understand how enable ccache machinery for Autotools, CMake, Meson, etc. and finally if it is possible to do in a general way. Then just extend corresponding Uses to support a new feature.