scripts/koji: add more informative error messages for koji_build (#738)

Also fixes fix ruff errors in the directory

Author: psafont

scripts/koji/create_user_target.py

@@ -1,9 +1,10 @@
 #!/usr/bin/env python3
 import argparse
-import subprocess
 import logging
+import subprocess
 import sys
 
+
 def setup_logger():
     logging.basicConfig(
         format="%(asctime)s - %(levelname)s - %(message)s",

scripts/koji/koji_build.py

@@ -155,6 +155,26 @@ def is_remote_branch_commit(git_repo, sha, branch):
         )
     return sha == remote_sha
 
+def build_id_of(name, candidate):
+    if candidate is None:
+        return None
+
+    length = len(candidate)
+    if length > 16:
+        logging.error(f"The {name} build id must be at most 16 characters long, it's {length} characters long")
+        exit(1)
+
+    invalid_chars = any(re.match(r'[a-zA-Z0-9]', char) is None for char in candidate)
+
+    if invalid_chars:
+        pp_invalid = ''.join("^" if re.match(r'[a-zA-Z0-9]', char) is None else " " for char in candidate)
+        logging.error(f"The {name} build id must only contain letters and digits:")
+        logging.error(f"    {candidate}")
+        logging.error(f"    {pp_invalid}")
+        exit(1)
+
+    return candidate
+
 def main():
     parser = argparse.ArgumentParser(
         description='Build a package or chain-build several from local git repos for RPM sources'
@@ -182,17 +202,13 @@ def main():
     git_repos = [os.path.abspath(check_dir(d)) for d in args.git_repos]
     is_scratch = args.scratch
     is_nowait = args.nowait
-    test_build = args.test_build
-    pre_build = args.pre_build
+
+    test_build = build_id_of("test", args.test_build)
+    pre_build = build_id_of("pre", args.pre_build)
+
     if test_build and pre_build:
         logging.error("--pre-build and --test-build can't be used together")
         exit(1)
-    if test_build is not None and re.match('^[a-zA-Z0-9]{1,16}$', test_build) is None:
-        logging.error("The test build id must be 16 characters long maximum and only contain letters and digits")
-        exit(1)
-    if pre_build is not None and re.match('^[a-zA-Z0-9]{1,16}$', pre_build) is None:
-        logging.error("The pre build id must be 16 characters long maximum and only contain letters and digits")
-        exit(1)
 
     if len(git_repos) > 1 and is_scratch:
         parser.error("--scratch is not compatible with chained builds.")

scripts/koji/koji_import_rpms.py

@@ -1,8 +1,10 @@
 #!/usr/bin/env python3
+
 import argparse
+import glob
 import os
 import subprocess
-import glob
+
 
 def get_srpm_info(srpmpath):
     return subprocess.check_output(['rpm', '-qp', srpmpath, '--qf', '%{name};;%{nvr}']).split(';;')
@@ -22,8 +24,6 @@ def main():
     parser.add_argument('--create-build', help='create the build even if there\'s no SRPM', action='store_true', default=False)
     args = parser.parse_args()
 
-    DEVNULL = open(os.devnull, 'w')
-
     srpm_directory = os.path.abspath(check_dir(args.srpm_directory))
     rpm_directory = os.path.abspath(check_dir(args.rpm_directory))
 

scripts/koji/sync_repo_from_koji.py

@@ -1,14 +1,14 @@
 #!/usr/bin/env python3
+
 import argparse
-import re
-import os
-import sys
-import subprocess
+import atexit
 import glob
+import os
+import re
 import shutil
+import subprocess
+import sys
 import tempfile
-import atexit
-
 from datetime import datetime
 
 USER_REPO_HTTPS = "https://koji.xcp-ng.org/repos/user/"
@@ -85,8 +85,6 @@
 
 KEY_ID = "3fd3ac9e"
 
-DEVNULL = open(os.devnull, 'w')
-
 def version_from_tag(tag):
     matches = re.match(r'v(\d+\.\d+)', tag)
     return matches.group(1)
@@ -118,7 +116,7 @@ def sign_rpm(rpm):
         subprocess.check_call(['koji', 'download-build', '--debuginfo', '--noprogress', '--rpm', rpm])
 
         # sign: requires a sign-rpm executable or alias in the PATH
-        subprocess.check_call(['sign-rpm', rpm], stdout=DEVNULL)
+        subprocess.check_call(['sign-rpm', rpm], stdout=subprocess.DEVNULL)
 
         # import signature
         subprocess.check_call(['koji', 'import-sig', rpm])
@@ -184,8 +182,8 @@ def write_repo(tag, dest_dir, tmp_root_dir, offline=False):
         paths.append(os.path.join(path_to_tmp_repo, 'Source'))
     for path in paths:
         print("\n-- Generate repodata for %s" % path)
-        subprocess.check_call(['createrepo_c', path], stdout=DEVNULL)
-        subprocess.check_call(['sign-file', os.path.join(path, 'repodata', 'repomd.xml')], stdout=DEVNULL)
+        subprocess.check_call(['createrepo_c', path], stdout=subprocess.DEVNULL)
+        subprocess.check_call(['sign-file', os.path.join(path, 'repodata', 'repomd.xml')], stdout=subprocess.DEVNULL)
 
     # Synchronize to our final repository:
     # - add new RPMs
@@ -379,7 +377,7 @@ def offline_repo_dir():
                     subprocess.check_call(
                         ['sign-file', 'SHA256SUMS'],
                         cwd=offline_repo_path_parent,
-                        stdout=DEVNULL
+                        stdout=subprocess.DEVNULL
                     )
 
                 # update data

scripts/koji/update_vendor_tags.py

@@ -1,11 +1,10 @@
 #!/usr/bin/env python3
+
 import argparse
-import subprocess
 import json
 import os
 import re
-
-DEVNULL = open(os.devnull, 'w')
+import subprocess
 
 XS_buildhosts = [
     '1b68968c4e4e',
@@ -59,7 +58,7 @@ def update_vendor_tag_for_build(build, is_bootstrap=False):
 
     # get vendor information
     output = subprocess.check_output(
-        ['rpm', '-qp', rpm_path, '--qf', '%{vendor};;%{buildhost}'], stderr=DEVNULL
+        ['rpm', '-qp', rpm_path, '--qf', '%{vendor};;%{buildhost}'], stderr=devprocess.DEVNULL
     ).decode()
     vendor, buildhost = output.split(';;')
     package = re.search('/packages/([^/]+)/', rpm_path).group(1)