mirror of
https://github.com/getsolus/packages.git
synced 2025-04-26 12:50:29 +03:00
ci/package_checks: Improve linter feedback and bump check (#2038)
**Summary**
This PR:
- Improves the output format when not running in CI.
- Shows this output when being called via a hook.
- Extends the package bump check so that pspec files must always be
bumped.
Resolves #1888.
**Test Plan**
Run CI checks locally using `go-task check` in a fairly dirty working
directory:
Try to commit a changed pspec file:
```
$ gotopkg nano
$ go-task build
$ git add pspec_x86_64.xml
$ git commit
ERR packages/n/nano/pspec_x86_64.xml:1: Package release is not incremented by 1
Package checks failed: <snip>/packages/common/CI/package_checks.py packages/n/nano/pspec_x86_64.xml
```
**Checklist**
- [x] ~~Package was built and tested against unstable~~
This commit is contained in:
Rune Morling
GitHub
commit
efc83c3119
2 changed files with 160 additions and 43 deletions
|
|
@ -2,18 +2,77 @@
|
|||
import argparse
|
||||
import glob
|
||||
import json
|
||||
import logging
|
||||
import os.path
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime, timezone
|
||||
from enum import Enum
|
||||
from typing import Any, Dict, List, Optional, TextIO, Tuple, Union
|
||||
from typing import Any, Callable, Dict, List, Optional, TextIO, Tuple, Union
|
||||
from urllib import request
|
||||
from xml.etree import ElementTree
|
||||
|
||||
import yaml
|
||||
|
||||
"""Package is either a Package YML file or Pspec XML file."""
|
||||
Package = Union['PackageYML', 'PspecXML']
|
||||
|
||||
|
||||
def in_ci() -> bool:
|
||||
"""Returns true if running in GitHub Actions or GitLab CI."""
|
||||
return os.environ.get('CI') == 'true'
|
||||
|
||||
|
||||
class PackageYML:
|
||||
"""Represents a Package YML file."""
|
||||
|
||||
def __init__(self, stream: Any):
|
||||
self._data = dict(yaml.safe_load(stream))
|
||||
|
||||
@property
|
||||
def name(self) -> str:
|
||||
return str(self._data['name'])
|
||||
|
||||
@property
|
||||
def version(self) -> str:
|
||||
return str(self._data['version'])
|
||||
|
||||
@property
|
||||
def release(self) -> int:
|
||||
return int(self._data['release'])
|
||||
|
||||
@property
|
||||
def homepage(self) -> Optional[str]:
|
||||
return self._data.get('homepage')
|
||||
|
||||
def get(self, key: str, default: Any = None) -> Any:
|
||||
return self._data.get(key, default)
|
||||
|
||||
|
||||
class PspecXML:
|
||||
"""Represents a Pspec XML file."""
|
||||
|
||||
def __init__(self, data: str):
|
||||
self._xml = ElementTree.fromstring(data)
|
||||
|
||||
@property
|
||||
def release(self) -> int:
|
||||
return int(self._xml.findall('.//Update')[0].attrib['release'])
|
||||
|
||||
@property
|
||||
def homepage(self) -> Optional[str]:
|
||||
xml_homepage_element = self._xml.find('.//Homepage')
|
||||
if xml_homepage_element is not None:
|
||||
return xml_homepage_element.text
|
||||
|
||||
return None
|
||||
|
||||
@property
|
||||
def files(self) -> List[str]:
|
||||
return [str(element.text) for element in self._xml.findall('.//Path')]
|
||||
|
||||
|
||||
@dataclass
|
||||
class FreezeConfig:
|
||||
|
|
@ -89,6 +148,28 @@ class Git:
|
|||
return self.run_lines('diff', '--name-only', '--diff-filter=AM')
|
||||
|
||||
|
||||
class LogFormatter(logging.Formatter):
|
||||
fmt = '\033[0m \033[94m%(pathname)s:%(lineno)d:\033[0m %(message)s'
|
||||
formatters = {
|
||||
logging.DEBUG: logging.Formatter('\033[1;30mDBG' + fmt),
|
||||
logging.INFO: logging.Formatter('\033[34mINF' + fmt),
|
||||
logging.WARNING: logging.Formatter('\033[33mWRN' + fmt),
|
||||
logging.ERROR: logging.Formatter('\033[31mERR' + fmt),
|
||||
logging.CRITICAL: logging.Formatter('\033[31mCRI' + fmt),
|
||||
}
|
||||
|
||||
def format(self, record: logging.LogRecord) -> str:
|
||||
return self.formatters[record.levelno].format(record)
|
||||
|
||||
@staticmethod
|
||||
def handler() -> logging.Handler:
|
||||
handler = logging.StreamHandler(sys.stderr)
|
||||
handler.setLevel(logging.DEBUG)
|
||||
handler.setFormatter(LogFormatter())
|
||||
|
||||
return handler
|
||||
|
||||
|
||||
class Level(str, Enum):
|
||||
__str__ = Enum.__str__
|
||||
DEBUG = 'debug'
|
||||
|
|
@ -96,6 +177,20 @@ class Level(str, Enum):
|
|||
ERROR = 'error'
|
||||
WARNING = 'warning'
|
||||
|
||||
@property
|
||||
def log_level(self) -> int:
|
||||
match self:
|
||||
case Level.DEBUG:
|
||||
return logging.DEBUG
|
||||
case Level.NOTICE:
|
||||
return logging.INFO
|
||||
case Level.WARNING:
|
||||
return logging.WARNING
|
||||
case Level.ERROR:
|
||||
return logging.ERROR
|
||||
case _:
|
||||
return 0
|
||||
|
||||
|
||||
@dataclass
|
||||
class Result:
|
||||
|
|
@ -109,7 +204,14 @@ class Result:
|
|||
endLine: Optional[int] = None
|
||||
|
||||
def __str__(self) -> str:
|
||||
return f'::{self.level}{self._meta}::{self._message}'
|
||||
return f'::{self.level.value}{self._meta}::{self._message}'
|
||||
|
||||
def log(self) -> None:
|
||||
if in_ci():
|
||||
print(str(self))
|
||||
return
|
||||
|
||||
logging.getLogger(__name__).handle(self._record)
|
||||
|
||||
@property
|
||||
def _message(self) -> str:
|
||||
|
|
@ -132,6 +234,15 @@ class Result:
|
|||
|
||||
return f'{key}={value}'
|
||||
|
||||
@property
|
||||
def _record(self) -> logging.LogRecord:
|
||||
return logging.LogRecord('',
|
||||
self.level.log_level,
|
||||
self.file or '',
|
||||
self.line or 1,
|
||||
self.message,
|
||||
None, None)
|
||||
|
||||
|
||||
class PullRequestCheck:
|
||||
_package_files = ['package.yml']
|
||||
|
|
@ -169,12 +280,22 @@ class PullRequestCheck:
|
|||
def _open(self, path: str) -> TextIO:
|
||||
return open(self._path(path), 'r')
|
||||
|
||||
def load_package_yml(self, file: str) -> Dict[str, Any]:
|
||||
with self._open(file) as f:
|
||||
return dict(yaml.safe_load(f))
|
||||
def _read(self, path: str) -> str:
|
||||
with self._open(path) as f:
|
||||
return str(f.read())
|
||||
|
||||
def load_package_yml_from_commit(self, ref: str, file: str) -> Dict[str, Any]:
|
||||
return dict(yaml.safe_load(self.git.file_from_commit(ref, file)))
|
||||
def load_package_yml(self, file: str) -> PackageYML:
|
||||
with self._open(file) as f:
|
||||
return PackageYML(f)
|
||||
|
||||
def load_package_yml_from_commit(self, ref: str, file: str) -> PackageYML:
|
||||
return PackageYML(self.git.file_from_commit(ref, file))
|
||||
|
||||
def load_pspec_xml(self, file: str) -> PspecXML:
|
||||
return PspecXML(self._read(file))
|
||||
|
||||
def load_pspec_xml_from_commit(self, ref: str, file: str) -> PspecXML:
|
||||
return PspecXML(self.git.file_from_commit(ref, file))
|
||||
|
||||
def file_line(self, file: str, expr: str) -> Optional[int]:
|
||||
with self._open(file) as f:
|
||||
|
|
@ -280,27 +401,35 @@ class Homepage(PullRequestCheck):
|
|||
class PackageBumped(PullRequestCheck):
|
||||
_msg = 'Package release is not incremented by 1'
|
||||
_msg_new = 'Package release is not 1'
|
||||
_level = Level.WARNING
|
||||
|
||||
def run(self) -> List[Result]:
|
||||
results = [self._check_commit(self.base or 'HEAD', file)
|
||||
for file in self.package_files]
|
||||
for file in self.files]
|
||||
|
||||
return [result for result in results if result is not None]
|
||||
|
||||
def _check_commit(self, base: str, file: str) -> Optional[Result]:
|
||||
new = self.load_package_yml(file)
|
||||
match os.path.basename(file):
|
||||
case 'package.yml':
|
||||
return self._check(base, file, PackageYML, Level.WARNING)
|
||||
case 'pspec_x86_64.xml':
|
||||
return self._check(base, file, PspecXML, Level.ERROR)
|
||||
case _:
|
||||
return None
|
||||
|
||||
def _check(self, base: str, file: str, loader: Callable[[str], Package], level: Level) -> Optional[Result]:
|
||||
new = loader(self._read(file))
|
||||
|
||||
try:
|
||||
old = self.load_package_yml_from_commit(base, file)
|
||||
if int(old['release']) + 1 != int(new['release']):
|
||||
return Result(level=self._level, file=file, message=self._msg)
|
||||
old = loader(self.git.file_from_commit(base, file))
|
||||
if old.release + 1 != new.release:
|
||||
return Result(level=level, file=file, message=self._msg)
|
||||
|
||||
return None
|
||||
except Exception as e:
|
||||
if 'exists on disk, but not in' in str(e):
|
||||
if int(new['release']) != 1:
|
||||
return Result(level=self._level, file=file, message=self._msg_new)
|
||||
if new.release != 1:
|
||||
return Result(level=level, file=file, message=self._msg_new)
|
||||
|
||||
return None
|
||||
|
||||
|
|
@ -383,9 +512,7 @@ class PackageVersion(PullRequestCheck):
|
|||
if not self._check_version(path)]
|
||||
|
||||
def _check_version(self, path: str) -> bool:
|
||||
version = self.load_package_yml(path)['version']
|
||||
|
||||
return isinstance(version, str)
|
||||
return isinstance(self.load_package_yml(path).version, str)
|
||||
|
||||
|
||||
class Patch(PullRequestCheck):
|
||||
|
|
@ -422,7 +549,7 @@ class SPDXLicense(PullRequestCheck):
|
|||
for r in self._validate_spdx(f) if r]
|
||||
|
||||
def _validate_spdx(self, file: str) -> List[Optional[Result]]:
|
||||
license = self.load_package_yml(file)['license']
|
||||
license = self.load_package_yml(file).get('license')
|
||||
if isinstance(license, list):
|
||||
return [self._validate_license(file, id) for id in license]
|
||||
|
||||
|
|
@ -492,26 +619,16 @@ class Pspec(PullRequestCheck):
|
|||
if not self._check_consistent(path)]
|
||||
|
||||
def _check_consistent(self, package_dir: str) -> bool:
|
||||
xml = ElementTree.parse(self._xml_file(package_dir))
|
||||
xml_release = int(xml.findall('.//Update')[0].attrib['release'])
|
||||
xml_homepage: str = ''
|
||||
xml_homepage_element = xml.find('.//Homepage')
|
||||
xml = self._xml_file(package_dir)
|
||||
yml = self._yml_file(package_dir)
|
||||
|
||||
if xml_homepage_element is not None:
|
||||
xml_homepage = xml_homepage_element.text or ''
|
||||
return bool(yml.release == xml.release and yml.homepage == xml.homepage)
|
||||
|
||||
with open(self._yml_file(package_dir), 'r') as f:
|
||||
yml = yaml.safe_load(f)
|
||||
yml_release = yml.get('release', '')
|
||||
yml_homepage = yml.get('homepage', '')
|
||||
def _yml_file(self, package_dir: str) -> PackageYML:
|
||||
return self.load_package_yml(os.path.join(package_dir, 'package.yml'))
|
||||
|
||||
return bool(yml_release == xml_release and yml_homepage == xml_homepage)
|
||||
|
||||
def _yml_file(self, package_dir: str) -> str:
|
||||
return self._path(os.path.join(package_dir, 'package.yml'))
|
||||
|
||||
def _xml_file(self, package_dir: str) -> str:
|
||||
return self._path(os.path.join(package_dir, 'pspec_x86_64.xml'))
|
||||
def _xml_file(self, package_dir: str) -> PspecXML:
|
||||
return self.load_pspec_xml(os.path.join(package_dir, 'pspec_x86_64.xml'))
|
||||
|
||||
|
||||
class SystemDependencies(PullRequestCheck):
|
||||
|
|
@ -529,7 +646,7 @@ class SystemDependencies(PullRequestCheck):
|
|||
return self._validate_deps(pkg) if self._components_match(pkg_components) else []
|
||||
|
||||
def _package_components(self, pkg: str) -> List[str]:
|
||||
return self._unwrap_component(self.load_package_yml(self.package_yml_path(pkg))['component'])
|
||||
return self._unwrap_component(self.load_package_yml(self.package_yml_path(pkg)).get('component'))
|
||||
|
||||
def _unwrap_component(self, component: Any) -> List[str]:
|
||||
if isinstance(component, dict):
|
||||
|
|
@ -587,9 +704,9 @@ class SummaryGenerator(PullRequestCheck):
|
|||
if package is None:
|
||||
return None
|
||||
|
||||
return f"{package['name']}-{package['version']}-{package['release']}"
|
||||
return f"{package.name}-{package.version}-{package.release}"
|
||||
|
||||
def _commit_package_yaml(self, ref: str) -> Optional[Dict[str, Any]]:
|
||||
def _commit_package_yaml(self, ref: str) -> Optional[PackageYML]:
|
||||
files = [f for f in self.git.files_in_commit(ref) if os.path.basename(f) == 'package.yml']
|
||||
if len(files) == 0:
|
||||
return None
|
||||
|
|
@ -644,7 +761,7 @@ class Checker:
|
|||
print(f"Found {len(results)} result(s), {len(errors)} error(s)")
|
||||
|
||||
for result in results:
|
||||
print(result)
|
||||
result.log()
|
||||
|
||||
self.write_summary()
|
||||
|
||||
|
|
@ -663,6 +780,8 @@ class Checker:
|
|||
|
||||
|
||||
if __name__ == "__main__":
|
||||
logging.basicConfig(level=logging.INFO, handlers=[LogFormatter.handler()])
|
||||
|
||||
parser = argparse.ArgumentParser()
|
||||
parser.add_argument('--base', type=str,
|
||||
help='Optional reference to the base branch')
|
||||
|
|
|
|||
|
|
@ -8,11 +8,9 @@ package_checks = os.path.join(common_dir, 'common', 'CI', 'package_checks.py')
|
|||
|
||||
|
||||
def _run(name: str, *args: str) -> bool:
|
||||
res = subprocess.run(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True)
|
||||
res = subprocess.run(args, stdout=subprocess.PIPE, text=True)
|
||||
if res.returncode != 0:
|
||||
print(f'{name} failed: {" ".join(args)}')
|
||||
print('Output:')
|
||||
print("\n".join([' ' + line for line in res.stdout.split("\n")]))
|
||||
|
||||
return res.returncode == 0
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue