StableKeywordsCheck: detect packages using stable keywords#769
StableKeywordsCheck: detect packages using stable keywords#769falbrechtskirchinger wants to merge 1 commit intopkgcore:masterfrom
Conversation
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. Signed-off-by: Florian Albrechtskirchinger <falbrechtskirchinger@gmail.com>
falbrechtskirchinger
force-pushed
the
stablekeywords
branch
from
7342501 to
41bd9a7
Compare
thesamesam
left a comment
thesamesam
left a comment
There was a problem hiding this comment.
It looks reasonable to me.
arthurzam
left a comment
arthurzam
left a comment
There was a problem hiding this comment.
Please handle those comments. Sorry for long delay
|
|
||
| _source = sources.PackageRepoSource | ||
| required_addons = (addons.StableArchesAddon,) | ||
| known_results = frozenset([StableKeywords]) |
There was a problem hiding this comment.
| known_results = frozenset([StableKeywords]) | |
| known_results = frozenset({StableKeywords}) |
| class StableKeywords(results.PackageResult, results.Error): | ||
| """Package uses stable keywords.""" |
There was a problem hiding this comment.
not a good result name and description - what does it even mean? A developer that gets such error won't understand it's reason.
|
|
||
| def __init__(self, *args, stable_arches_addon=None): | ||
| super().__init__(*args) | ||
| self.arches = {x.strip().lstrip("~") for x in self.options.stable_arches} |
There was a problem hiding this comment.
| self.arches = {x.strip().lstrip("~") for x in self.options.stable_arches} | |
| self.arches = frozenset({x.strip().lstrip("~") for x in self.options.stable_arches}) |
I like immutability
There was a problem hiding this comment.
The request to make this immutable may sound pedantic, but I fixed an issue the other day where code was incorrectly mutating a Result object, so that pattern we maintain, exists for a valid reason. It's not python standard, but the defensive approach has prevented a surprisingly large amount of fuckups.
TL;DR: please make it immutable. Frankly a tuple is fine rather than a frozenset also, but whatever convention is, go with that.
| class StableKeywordsCheck(OptionalCheck): | ||
| """Scan for packages using stable keywords.""" | ||
|
|
||
| _source = sources.PackageRepoSource |
There was a problem hiding this comment.
Why use here Repo scope and not just Package scope like most other scans? It would be much simpler to handle, group, ...
There was a problem hiding this comment.
This needs to be package scope; the only results it generates are package scope already.
|
|
||
| def __init__(self, *args, stable_arches_addon=None): | ||
| super().__init__(*args) | ||
| self.arches = {x.strip().lstrip("~") for x in self.options.stable_arches} |
There was a problem hiding this comment.
The request to make this immutable may sound pedantic, but I fixed an issue the other day where code was incorrectly mutating a Result object, so that pattern we maintain, exists for a valid reason. It's not python standard, but the defensive approach has prevented a surprisingly large amount of fuckups.
TL;DR: please make it immutable. Frankly a tuple is fine rather than a frozenset also, but whatever convention is, go with that.
| # collapse reports by sets of arches | ||
| for arches, pkgs in arches_pkgs.items(): | ||
| versions = (pkg.fullver for pkg in sorted(pkgs)) | ||
| yield StableKeywords(versions, sort_keywords(arches), pkg=pkgs[0]) |
There was a problem hiding this comment.
Move sort_keywords into the __init__ of StableKeywords. the sorting is an aspect of that result's presentation- somethiing it's responsible for.
Encapsulation wise, it shouldn't expect that creators of the instance know to sort that value. Your code does, other code absolutely won't. :)
| class StableKeywordsCheck(OptionalCheck): | ||
| """Scan for packages using stable keywords.""" | ||
|
|
||
| _source = sources.PackageRepoSource |
There was a problem hiding this comment.
This needs to be package scope; the only results it generates are package scope already.
4 participants
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 inO(N * M), where a simple set intersection could beO(N).The severity was chosen based on the language in the regulations (rule 4): "Stable keywords must not be used." (emphasis mine)
Tests
expected.jsonandfix.patch.