[New Check]: Warn on dubious src_prepare

[thesamesam]

Is there an existing such new check request for this?

• [x] 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_prepare where it matches the default implementation (maybe we should do that for all phases);
• src_prepare calling default and then eapply_user (maybe if it's literally that to avoid false positives with conditionals), meaning user patches would be applied twice
• src_prepare calling eapply_user but not default (or eapply ...) while PATCHES is 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

[thesamesam]

We may want to consider warning on the following as error-prone:

src_prepare() {
    #eapply ... # (may or may not be there, but harder to justify warning on that case if it is)
    eapply_user # valid, but error-prone if PATCHES is ever added in the ebuild
}

[radhermit]

Case 1:

src_prepare() {
    default
    eapply_user # applies user patches twice if they exist!
}

Applying patches twice feels like an implementation bug as the spec for eapply_user states 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_user calls are ignored (even when called via default) which is how I implemented it for both pkgcore and pkgcraft.

[thesamesam]

I'll double check if Portage really does that. I'm fairly sure it does (apply twice) but not checked recently.

[radhermit]

I'll double check if Portage really does that. I'm fairly sure it does (apply twice) but not checked recently.

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]

I might've been thinking of double default where an eclass calls it instead (=> double PATCHES application).