From eebeb892b7ca3cabc92b09b490b189e8bdf11db4 Mon Sep 17 00:00:00 2001 From: Jean-Louis Fuchs Date: Wed, 13 Mar 2024 19:47:03 +0100 Subject: [PATCH 1/5] feat: run_command log error before raising --- pyaptly/util.py | 58 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/pyaptly/util.py b/pyaptly/util.py index 696bd7c..344ee4c 100644 --- a/pyaptly/util.py +++ b/pyaptly/util.py @@ -3,12 +3,15 @@ import logging import os import subprocess +import sys +import traceback from pathlib import Path - -from colorama import Fore, init from subprocess import PIPE, CalledProcessError # noqa: F401 +from tempfile import NamedTemporaryFile from typing import Optional, Sequence +from colorama import Fore, init + _DEFAULT_KEYSERVER: str = "hkps://keys.openpgp.org" _PYTEST_KEYSERVER: Optional[str] = None @@ -19,7 +22,7 @@ Command call cmd: {cmd} {color_begin}-> {returncode}{color_end} """.strip() -OUTPUT_LOG = " {out_type}: '{output}'" +OUTPUT_LOG = " {out_type}:{white_space}'{output}'" _indent = " " * 15 _isatty_cache: bool | None = None @@ -28,6 +31,13 @@ lg = logging.getLogger(__name__) +def write_traceback(): # pragma: no cover + with NamedTemporaryFile("w", delete=False) as tmp: + tmp.write(traceback.format_exc()) + tmp.close() + return tmp.name + + def isatty(): global _isatty_cache if _isatty_cache is None: @@ -75,27 +85,48 @@ def run_command( """ added_stdout = False added_stderr = False - # TODO assert PIPE or None if "stdout" not in kwargs: kwargs["stdout"] = PIPE added_stdout = True + else: + assert kwargs["stdout"] in (PIPE, None) if "stderr" not in kwargs: kwargs["stderr"] = PIPE added_stderr = True + else: + assert kwargs["stdout"] in (PIPE, None) # pragma: no cover + # If we want to log stdout/err before raising CalledProcessError we have to + # check ourselves + check = False + if "check" in kwargs: + check = kwargs["check"] + del kwargs["check"] result = None + tb = "" if decode and "encoding" not in kwargs: kwargs["encoding"] = "UTF-8" try: result = subprocess.run(cmd_args, **kwargs) + if check and result.returncode: + raise CalledProcessError( + result.returncode, + result.args, + output=result.stdout, + stderr=result.stderr, + ) + except Exception: + if "pytest" not in sys.modules: + tb = write_traceback() # pragma: no cover + raise finally: if result: - log_msg = format_run_result(result, result.returncode) + log_msg = format_run_result(result, result.returncode, tb) if result.returncode == 0: lg.info(log_msg) else: if not hide_error or lg.root.level <= 20: lg.error(log_msg) - # Do not change returned result by debug mode + # This function should not alter the returned result if added_stdout: delattr(result, "stdout") if added_stderr: @@ -132,7 +163,7 @@ def indent_out(output: bytes | str) -> str: return "\n".join(result) -def format_run_result(result: subprocess.CompletedProcess, returncode: int): +def format_run_result(result: subprocess.CompletedProcess, returncode: int, tb: str): """Format a CompletedProcess result log.""" color_begin = "" color_end = "" @@ -153,11 +184,20 @@ def format_run_result(result: subprocess.CompletedProcess, returncode: int): stderr=indent_out(result.stderr), ) ] - for out_type, output in [("stdout", result.stdout), ("stderr", result.stderr)]: + for out_type, output in [ + ("stdout", result.stdout), + ("stderr", result.stderr), + ("traceback", tb), + ]: output = output.strip() if output: output = indent_out(output) - msg.append(OUTPUT_LOG.format(out_type=out_type, output=output)) + white_space = " " * (11 - len(out_type)) + msg.append( + OUTPUT_LOG.format( + out_type=out_type, white_space=white_space, output=output + ) + ) pass return "\n".join(msg) From c1765d975bc673d5c65e3ece643ccd69f071f3bc Mon Sep 17 00:00:00 2001 From: Jean-Louis Fuchs Date: Wed, 13 Mar 2024 20:06:38 +0100 Subject: [PATCH 2/5] fix: stop forcing noarch although it seems technically correct that the package is noarch, I decided against forcing it. It seems like nothing is gained for the risk for corrupt __pycache__ files and similar things. --- tools/venv-rpm | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/venv-rpm b/tools/venv-rpm index 0760a34..5ecd465 100755 --- a/tools/venv-rpm +++ b/tools/venv-rpm @@ -11,7 +11,6 @@ Version: 2.0.0^{revision} Release: 1%{{?dist}} Summary: Automates the creation and managment of aptly mirrors and snapshots based on toml input files License: AGPL-3.0-or-later -BuildArch: noarch Requires: python3.11 %description @@ -41,10 +40,6 @@ rm -rf /opt/pyaptly def main(): os.chdir("/source") - macros = Path("/root/.rpmmacros") - with macros.open("w", encoding="UTF-8") as f: - f.write("%_binaries_in_noarch_packages_terminate_build 0") - f.write("\n") run(["dnf", "install", "-y", "rpm-build", "python3.11", "git"], check=True) date = datetime.now().strftime("%a %b %d %Y") build_id = run( From 7b88932bab511085593bf43cc19c41437c57d103 Mon Sep 17 00:00:00 2001 From: Jean-Louis Fuchs Date: Wed, 13 Mar 2024 20:26:30 +0100 Subject: [PATCH 3/5] chore: cleanup TODO --- TODO | 58 +++++++++------------------------------------------------- 1 file changed, 9 insertions(+), 49 deletions(-) diff --git a/TODO b/TODO index a883d11..78702e6 100644 --- a/TODO +++ b/TODO @@ -1,58 +1,18 @@ -# All code moves somewhere else +# CHANGELOG -- Tests to the tests/ directory -- Everything else away from legacy.py -- Remove version.py completely +- CHANGELOG convert to toml +- CHANGELOG.rst rename to CHANGELOG.md +- CHANELOG.md generate from toml +- RPM_SPEC generate changelog from toml -# Update old files +# Docs -The following files have just been copied without checking if their content is ok: +- Remove sphinx-doc tags from documentation -> currently deferred -- CHANELOG -- CHANELOG.rst -- README.rst +# Release -The reason for this is to ensure the continuity of the git history. - -# Tool to convert yaml config to toml - -- This is probably just a sub-command of pyaptly -- We probably still want to switch to a modern yaml parser - -# Replace all subprocess commands with a modern one (run_command()) - -# All logging should be done via logging (no stdout/stderr) - -# Remove all top-level `# type: ignore` and all `# type: ignore # TODO` - -# Add good documentation to every function and module, but plan to review all of - it once everything is in place - -# Do all output via logging module - -- Idea show INFO and above, but display info without timestamp etc. - -# latest, maybe repeat: - -- replace all prints with logging -- correctly do getLogger -- is this unit_or_list... thing still needed -- remove codecs.open -- use pathlib.Path -- replace state_reader.state with function -- remove six -- remote test.py completely - -- Build package - - sudo dnf install python3-click python3-frozendict python3-pyyaml python3-tomli python3-tomli-w python3-devel python3-setuptools - -- remove pyp2rpm and just put a spec into build-rpm, like in venv-rpm +- remove pyp2rpm and just put a spec into build-rpm, like in venv-rpm (maybe) - venv-rpm hardcodes version 2.0.0 in some places - review rpm build and improve it later - -- This is string is wrong in several places: - - Automates the creation and managment of aptly mirrors and snapshots based on toml input files From 5ccc61099096a6b50c0cd26edb00914d8124b0a5 Mon Sep 17 00:00:00 2001 From: Jean-Louis Fuchs Date: Wed, 13 Mar 2024 20:27:01 +0100 Subject: [PATCH 4/5] chore: rename TODO to TODO.md --- TODO => TODO.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename TODO => TODO.md (100%) diff --git a/TODO b/TODO.md similarity index 100% rename from TODO rename to TODO.md From 18b817fe34c77c5c80118ed3d507671bdcd4dafe Mon Sep 17 00:00:00 2001 From: Jean-Louis Fuchs Date: Thu, 14 Mar 2024 11:30:29 +0100 Subject: [PATCH 5/5] fix: package build --- tools/venv-rpm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/venv-rpm b/tools/venv-rpm index 0760a34..ea92934 100755 --- a/tools/venv-rpm +++ b/tools/venv-rpm @@ -65,7 +65,7 @@ def main(): shutil.rmtree("/root/rpmbuild", ignore_errors=True) run(["rpmbuild", "--bb", spec], check=True) rpms = Path("/root/rpmbuild/RPMS/noarch") - file = Path(list(rpms.glob("python3-pyaptly-*.noarch.rpm"))[0]) + file = Path(list(rpms.glob("python3-pyaptly-*.rpm"))[0]) shutil.copy2(file, dist)