Repository navigation
[New Check]: Warn on dubious src_prepare #730
Description
Is there an existing such new check request for this?
- I have searched the existing issues
Explain
Today, I've fixed a bunch of src_prepare and eapply_user issues, see git log a6f09dafb524e7b6bdbc0585713c58fa324dfdc8~1..e0e58730761f9cb48aeb7e8f943fd26e2700c662.
We should try to detect the following:
src_preparewhere it matches the default implementation (maybe we should do that for all phases);src_preparecallingdefaultand theneapply_user(maybe if it's literally that to avoid false positives with conditionals), meaning user patches would be applied twicesrc_preparecallingeapply_userbut notdefault(oreapply ...) whilePATCHESis defined in the ebuild
Examples
There were several problematic cases. Inspired by https://bugs.gentoo.org/949031 (and git log above).
Case 1:
src_prepare() {
default
eapply_user # applies user patches twice if they exist!
}
Case 2:
src_prepare() {
eapply "${PATCHES[@]}"
eapply_user # these two lines are identical to default_src_prepare
}
Case 3:
PATCHES=(
"${FILESDIR}"/important.patch
)
src_prepare() {
eapply_user # never applies PATCHES
}
Output message
BuggySrcPrepare
Documentation
src_prepare is written in a suspicious manner that may either fail to apply some patches or double-apply user patches.
Result level
warning
Activity
thesamesam commentedon Feb 24, 2025
We may want to consider warning on the following as error-prone:
radhermit commentedon Feb 25, 2025
Applying patches twice feels like an implementation bug as the spec for
eapply_userstates the following: "In EAPIs where it is supported, eapply_user must be called once in the src_prepare phase. For any subsequent calls, the command will do nothing and return 0."Meaning multiple
eapply_usercalls are ignored (even when called viadefault) which is how I implemented it for both pkgcore and pkgcraft.thesamesam commentedon Feb 25, 2025
I'll double check if Portage really does that. I'm fairly sure it does (apply twice) but not checked recently.
radhermit commentedon Feb 25, 2025
On a quick glance it appears to handle it correctly using a file existence check -- https://github.com/gentoo/portage/blob/master/bin/phase-helpers.sh#L1088.
thesamesam commentedon Feb 25, 2025
I might've been thinking of double default where an eclass calls it instead (=> double
PATCHESapplication).