ProhibitedStableKeywordsCheck: detect packages using prohibited stable keywords

[falbrechtskirchinger]

Add an optional check to scan for packages using stable keywords. This is useful for overlays like ::guru, which require that all packages be keyworded unstable.

The implementation is based on UnstableOnlyCheck, though strictly copying the restrictions mechanism results in O(N * M), where a simple set intersection could be O(N).
The severity was chosen based on the language in the regulations (rule 4): "Stable keywords must not be used." (emphasis mine)

Tests

• Added a new repo, as this check conflicts with other test cases using stable keywords.
• Added test ebuilds to exercise both positive and negative paths.
• Added expected.json and fix.patch.

[thesamesam]

It looks reasonable to me.

[arthurzam]

Please handle those comments. Sorry for long delay

[falbrechtskirchinger]

I've renamed the check to DisallowedStableKeywordsCheck and addressed the mutability, encapsulation, and scope issues.

I've also added ignored categories (acct-group, acct-user) because the related eclasses define KEYWORDS, which triggers this check. There hasn't been any progress on resolving the KEYWORDS-in-eclasses issue in 15 years (see https://bugs.gentoo.org/342185).

Pending a final decision on the name, I will rename/regenerate the test data, rebase, and reword the commits.

As I mentioned, the check is based on UnstableOnlyCheck with minimal changes, and I just want to ask if this much simpler version might be preferable:

classDisallowedStableKeywordsCheck(OptionalCheck):
"""Scan for packages using stable keywords in repositories where they are not allowed."""known_results=frozenset({DisallowedStableKeywords})
# acct-group and acct-user eclasses define KEYWORDS# See https://bugs.gentoo.org/342185ignored_categories=frozenset({"acct-group", "acct-user"})
deffeed(self, pkg):
ifpkg.categoryinself.ignored_categories:
returnarches= {kforkinpkg.keywordsifnotk.startswith(('~', '-'))}
ifnotarches:
returnyieldDisallowedStableKeywords(arches, pkg=pkg)

No addons, and no use of package restrictions.

[ferringb]

This PR looks good to go from my standpoint, sans an english nitpick.

I'm guessing the PR got overlooked, but is there any issues from the other maintainers?

[falbrechtskirchinger]

This PR looks good to go from my standpoint, sans an english nitpick.

Not quite done.

@ferringb Could you (or anyone, really) please address my previous comment (specifically the simplified code snippet)? #769 (comment)

Also, I'm still waiting on a final call regarding the name change. DisallowedStableKeywords, ProhibitedStableKeywords, or something else? I haven't renamed all occurrences yet.

[ferringb]

Pardon the lag, I've been buried. If you've got other PR's lagging, comment on them- in the coming week I plan to resume cleaning up the PR queue and ticketing for the pkgcore projects.

Also, I'm still waiting on a final call regarding the name change. DisallowedStableKeywords, ProhibitedStableKeywords, or something else? I haven't renamed all occurrences yet.

Prohibited, definitely. It's a statement of action, and for non-native speakers it avoids the negation.

In the same vein, blacklist -> denylist or prohibited, whitelist -> allowlist or permitted, etc. Both to use modern norms and for linguistic reasons.

@ferringb Could you (or anyone, really) please address my previous comment (specifically the simplified code snippet)? #769 (comment)

That version is better, and the pattern in general is better if you can surface the constant filtration restrictions to the class level. One minor tweak however, to be explicit to the type checker:

- known_results = frozenset({DisallowedStableKeywords})
+ known_results: typing.ClassVar[frozenset] = frozenset({DisallowedStableKeywords})

Moving it to class level permits thus:

1. Check's aren't configurable right now; it is desirable to allow checks to take some basic configuration from the targetted repo for knowing how to configure themselves. Moving things to the class level at least starts surfacing the hardcoded internal constants (so we can surface it in the check help output), and will make it simpler to do the first pass of configurables.

2. The 'feeder' can integrate this sort of knowledge for better batching of work to workers to optimize for them loading package metadta into memory. That may not render much gains, but allowing for it via surfacing the hardcoded restrictions is a decent idea anyways.

[ferringb]

I took a run at tweaking this PR for merging it- basically just doing the rename (not the problem) and also fixing the lint issues blocking it from running.

This actually is failing tests, even if all I'm doing is running ruff corrections: see https://github.com/ferringb/pkgcheck/actions/runs/24746961140/job/72400934662 .

That error is stating- from what I know of it (that codepath is new to me)- that the check isn't actually registered. It's on my todo to refactor that registry into an actual derived registry, but I've not wrote that cleanup. This PR can't be merged since it's directly failing both local and GH tests. And I'm not seeing immediately why it fails tests.

This strikes me as something very simple, but non obvious- something we should fix so it doesn't happen again- but again, I do not see why it's not finding the check in it's registry. To be clear, any caches on disk that may exist don't exist in the GH test run so that shouldn't be implicated.

[falbrechtskirchinger]

This actually is failing tests, even if all I'm doing is running ruff corrections: see ferringb/pkgcheck/actions/runs/24746961140/job/72400934662 .

That is because the test names don't match the check name. Let me do the rename and incorporate your suggestions first. This should get the tests to pass (again), and you can polish it further from there.

I'll try to get it done by the end of the week.

[falbrechtskirchinger]

@ferringb Alright, sorry about the delay, I have:

• reworded the docstrings
• renamed the check, result, and test data
• switched to the simplified implementation
• added type hints to the two class variables
• removed unneeded imports
• reformatted using ruff
• squashed the history and reworded the commit

The tests

tests/scripts/test_pkgcheck_scan.py::TestPkgcheckScan::test_scan_repo_data[prohibited_stable]
tests/scripts/test_pkgcheck_scan.py::TestPkgcheckScan::test_fix[ProhibitedStableKeywordsCheck-ProhibitedStableKeywords]

should once again pass.

I believe I've addressed all comments, and you should be able to take it from here!