Detect "shadowed" pkg_{setup,pretend} phases
This is mostly an issue with pkg_{setup,pretend,postinst} but it affects other phase functions too.
For example, xdg.eclass exports a xdg_pkg_postinst, but if one has...
EAPI=8
inherit optfeature xdg
pkg_postinst() {
optfeature "Foo support" dev-libs/foo
}
... then xdg_pkg_postinst is never called, which may be because the developer didn't realise xdg even exported a pkg_postinst.
This is arguably more of a problem with things like fortran-2.eclass where we might often have to define pkg_setup to check for OpenMP support, but then forget to call fortran-2_pkg_setup (example modified from sci-libs/lis/lis-1.6.5.ebuild:
EAPI=8
inherit toolchain-funcs fortran-2
pkg_pretend() {
[[ ${MERGE_TYPE} != binary ]] && use openmp && tc-check-openmp
}
pkg_setup() {[
# (Dropped fortran-2_pkg_setup from original ebuild for purposes of example)
[[ ${MERGE_TYPE} != binary ]] && use openmp && tc-check-openmp && FORTRAN_NEED_OPENMP=1
}
Well, after trying to implement it (looked not too hard at first sight), I noticed some questions and ideas:
- Sometimes we do want to skip a phase function from an eclass, for some weird cases - we need a way for the author to specify "I know what I'm doing, skip me". I was thinking on him just adding a simple comment inside phase function with skipped phase name (like
# fortran-2_pkg_setup). - How do we define "was a phase function shadowed" in more complex phase function that call other once. Let's take the hardest combination:
distutils-r1andmultibuildin same ebuild. You need to call one of them inside "sub-phase" of the other. If we want to catch such cases, we need to somehow calculate who calls whom, so we can state "that function isn't called at all". I was thinking of creating a Directed Acyclic Graph of caller->callee, but then I remembered that like in the previous example, if we talk about merging sub-phases across eclasses, this becomes too hard and slow for a check.
I could simplify a lot the check, by collecting all exported phase functions and checking if they are called somewhere in ebuild. But in that case what do I do with functions like "multibuild_foreach_variant", which might get passed with distutils-r1_src_test.
- Works for me. Do wonder about maybe a more standard way of skipping checks in ebuilds but not something we necessarily have to handle now.
- While it's possible a situation like that is quite serious, we can hope that in most cases, such a complex interaction would have obvious failure, and it's not worth pursuing all possible cases if we can solve the common/easier ones. But I do admit that this bug was partly inspired by perl-module exporting src_test and clobbering CMake's!
We could consider having "enhanced" checks which are too expensive to enable by default. We already have network checks in a similar category and go full DAG. It's up to you if you feel it's worth the effort though, I'd feel bad asking you to do that.
For the last point, do wonder if we could just have tags or something to indicate a multi build situation but that feels a bit gross.