install-qa-check.d: migrate xdg-utils checks over from preinst/postinst to ${D} #1362
Conversation
…st to ${D}
It's practically criminal to run these at merge time instead of src_install
time. It disproportionately affects binpkg consumers, because it applies
to the entirety of ROOT. We actually don't want to do... basically any
of this. It's not even accurate because we are heavily reliant on mtime
of installed files to check whether the commands were actually run.
What we actually want to do is significantly simpler: every package that
installs specific files to ${D} has to also inherit an eclass and run a
function in pkg_postinst. We can check for inherits quite trivially, and
warn about those. We can also slightly less efficiently check the
contents of pkg_* functions to see if they make certain calls; bash can
print the function contents and we can grep for that.
It doesn't catch cases where a custom eclass runs the xdg functions, but
we manually include those in our matching.
Signed-off-by: Eli Schwartz <[email protected]>
eli-schwartz
force-pushed
the
xdg-faster
branch
from
659dc0e
to
e6965c9
Compare
|
NAK. There's no real gain from this, and it's extremely fragile. |
The gain is not running tonnes of slow code on merging every single binpkg. Checking mtime is also fragile. We even had a user give specific measurements at https://bugs.gentoo.org/937150#c3 of how slow the existing check is. One particularly bad part about the current setup is, if one package is missing the xdg hooks, you get reminded about that by every other package. |
|
How can you handle ebuilds that call these functions via a separate function (e.g. because they don't want to repeat the same thing in |
|
We could fallback to the worse (mtime) check if I think that's kind of a hack, but I also think that needing |
|
I don't know. This sounds like a lot of extra complexity, and extra complexity means even higher risk of false positives. But I suppose it could work. |
|
I wonder if we're better off doing something like:
I'm going to sleep on it and have a think. I'm not really married to this PR, but I do think the status quo is undesirably slow :( |
|
Yeah, I don't think this kind of QA check needs to run for end users at all. |
|
A false positive that you can solve in the ebuild itself is better than a false positive that can only be solved by fixing a different ebuild. Tests that actually test what they claim to test instead of testing the side effects of something totally unrelated are better than really slow tests that don't test what you want to test. And no, the answer is NOT that the checks simply should run for developers and not for end users. I don't want to be heavily penalized in merge speed just because I'm a developer and the check is badly written. Is it better for developers to simply opt out of QA checks? ... Somewhat tangential but also kind of related: https://bugs.gentoo.org/516014 I am skeptical of the value of running all phase functions for all eclasses as a general concept, but it does feel like eclasses such as xdg would work better as an incremental effect, precisely because pkg_* phases tend to be cumulative via hardcoding instead. Maybe a phase-hooks.eclass which runs each function in |
Yeah, we can (and should) really have QA_* exception vars for all QA checks. I think it should work to just have people set those if they insist on the indirection. It's pretty rare anyway? In general, I agree as well that we shouldn't pessimise everybody based on what one ebuild might be doing wrong, and also inflict warnings where: a) it was caused by another ebuild; b) you may not even be able to do anything about it. |
It's practically criminal to run these at merge time instead of src_install time. It disproportionately affects binpkg consumers, because it applies to the entirety of ROOT. We actually don't want to do... basically any of this. It's not even accurate because we are heavily reliant on mtime of installed files to check whether the commands were actually run.
What we actually want to do is significantly simpler: every package that installs specific files to ${D} has to also inherit an eclass and run a function in pkg_postinst. We can check for inherits quite trivially, and warn about those. We can also slightly less efficiently check the contents of pkg_* functions to see if they make certain calls; bash can print the function contents and we can grep for that.
It doesn't catch cases where a custom eclass runs the xdg functions, but we manually include those in our matching.