Fixes for the chroot_scan plugin

behave/features/chroot-scan.feature

@@ -0,0 +1,19 @@
+Feature: The chroot_scan plugin
+
+@chroot_scan
+Scenario: Check that chroot_scan works and file permissions are correct
+    Given chroot_scan is enabled for dnf5.log
+    And an unique mock namespace
+    When an online source RPM is rebuilt
+    Then the build succeeds
+    And dnf5.log file is in chroot_scan result dir
+    And ownership of all chroot_scan files is correct
+
+@chroot_scan
+Scenario: Check that chroot_scan tarball is created correctly
+    Given an unique mock namespace
+    And chroot_scan is enabled for dnf5.log
+    And chroot_scan is configured to produce tarball
+    When an online source RPM is rebuilt
+    Then the build succeeds
+    And chroot_scan tarball has correct perms and provides dnf5.log

behave/features/environment.py

@@ -3,6 +3,7 @@
 """
 
 import os
+import pwd
 import random
 import shutil
 import string
@@ -57,11 +58,14 @@ def before_all(context):
 def _cleanup_workdir(context):
     shutil.rmtree(context.workdir)
     context.workdir = None
+    context.custom_config = ""
 
 
 def before_scenario(context, _scenario):
     """ execute before - once for each - scenario """
     context.workdir = tempfile.mkdtemp(prefix="mock-behave-tests-")
+    context.custom_config = ""
     context.add_cleanup(_cleanup_workdir, context)
     context.mock = Mock(context)
     context.add_repos = []
+    context.current_user = pwd.getpwuid(os.getuid())[0]

behave/features/steps/other.py

@@ -5,7 +5,9 @@
 import json
 import os
 import shutil
+import tarfile
 import tempfile
+from pathlib import Path
 
 from hamcrest import (
     assert_that,
@@ -244,3 +246,58 @@ def step_impl(context, option):
 def step_impl(_, directory):
     assert_that(os.path.exists(directory), equal_to(True))
     assert_that(not os.listdir(directory), equal_to(True))
+
+
+@given('chroot_scan is enabled for {regex}')
+def step_impl(context, regex):
+    context.custom_config += f"""\
+config_opts['plugin_conf']['chroot_scan_enable'] = True
+config_opts['plugin_conf']['chroot_scan_opts']['regexes'] = ["{regex}"]
+config_opts['plugin_conf']['chroot_scan_opts']['only_failed'] = False
+"""
+
+
+@then('{file} file is in chroot_scan result dir')
+def step_impl(context, file):
+    resultdir = os.path.join(context.mock.resultdir, 'chroot_scan')
+
+    # Find the expected file
+    found = False
+    print("resultdir: ", resultdir)
+    for _, _, files in os.walk(resultdir):
+        for f in files:
+            print(f)
+            if f == file:
+                found = True
+                break
+        if found:
+            break
+    assert_that(found, equal_to(True))
+
+
+@given('chroot_scan is configured to produce tarball')
+def step_impl(context):
+    context.custom_config += """\
+config_opts['plugin_conf']['chroot_scan_opts']['write_tar'] = True
+"""
+
+
+@then('ownership of all chroot_scan files is correct')
+def step_impl(context):
+    resultdir = os.path.join(context.mock.resultdir, 'chroot_scan')
+    for root, dirs, files in os.walk(resultdir):
+        for f in files + dirs:
+            path = Path(root) / f
+            assert_that(path.group(), equal_to("mock"))
+            assert_that(path.owner(), equal_to(context.current_user))
+
+
+@then('chroot_scan tarball has correct perms and provides dnf5.log')
+def step_impl(context):
+    tarball = Path(context.mock.resultdir, 'chroot_scan.tar.gz')
+    with tarfile.open(tarball, 'r:gz') as tarf:
+        for file in tarf.getnames():
+            if file.endswith("dnf5.log"):
+                break
+    assert_that(tarball.group(), equal_to("mock"))
+    assert_that(tarball.owner(), equal_to(context.current_user))

behave/testlib.py

@@ -2,6 +2,7 @@
 
 from contextlib import contextmanager
 import io
+from pathlib import Path
 import shlex
 import os
 import subprocess
@@ -98,7 +99,16 @@ def init(self):
 
     def rebuild(self, srpms):
         """ Rebuild source RPM(s) """
-        out, err = run_check(self.basecmd + ["--rebuild"] + srpms)
+
+        chrootspec = []
+        if self.context.custom_config:
+            config_file = Path(self.context.workdir) / "custom.cfg"
+            with config_file.open("w") as fd:
+                fd.write(f"include('{self.context.chroot}.cfg')\n")
+                fd.write(self.context.custom_config)
+            chrootspec = ["-r", str(config_file)]
+
+        out, err = run_check(self.basecmd + chrootspec + ["--rebuild"] + srpms)
         self.context.mock_runs['rebuild'] += [{
             "status": 0,
             "out": out,

mock/py/mockbuild/plugins/chroot_scan.py

@@ -9,11 +9,10 @@
 import os.path
 import re
 import shutil
-import subprocess
 
 # our imports
 from mockbuild.trace_decorator import getLog, traceLog
-from mockbuild import util
+from mockbuild import util, file_util
 
 requires_api_version = "1.1"
 
@@ -57,6 +56,8 @@ def __scanChroot(self):
         regex = re.compile(regexstr)
         chroot = self.buildroot.make_chroot_path()
         self.buildroot.create_resultdir()
+        # self.resultdir != self.buildroot.resultdir
+        file_util.mkdirIfAbsent(self.resultdir)
         count = 0
         logger = getLog()
         logger.debug("chroot_scan: Starting scan of %s", chroot)
@@ -66,7 +67,10 @@ def __scanChroot(self):
                 m = regex.search(f)
                 if m:
                     srcpath = os.path.join(root, f)
-                    subprocess.call("cp --preserve=mode --parents %s %s" % (srcpath, self.resultdir), shell=True)
+                    # we intentionally ignore errors here:
+                    # https://github.com/rpm-software-management/mock/issues/1455
+                    util.do(["cp", "--preserve=mode", "--parents", srcpath,
+                             self.resultdir], raiseExc=False)
                     count += 1
                     copied.append(srcpath)
         logger.debug("chroot_scan: finished with %d files found", count)
@@ -76,7 +80,7 @@ def __scanChroot(self):
             self.buildroot.uid_manager.changeOwner(self.resultdir, recursive=True)
             # some packages installs 555 perms on dirs,
             # so user can't delete/move chroot_scan's results
-            subprocess.call(['chmod', '-R', 'u+w', self.resultdir])
+            util.do(['chmod', '-R', 'u+w', self.resultdir])
             if self._tarball():
                 tarfile = self.resultdir + ".tar.gz"
                 logger.info("chroot_scan: creating tarball %s", tarfile)
@@ -86,3 +90,4 @@ def __scanChroot(self):
                     shell=False, printOutput=True
                 )
                 shutil.rmtree(self.resultdir)
+                self.buildroot.uid_manager.changeOwner(tarfile)

releng/release-notes-next/chroot-scan-permissions.bugfix

@@ -0,0 +1,2 @@
+The ownership of the `chroot_scan` plugin "output tarball" (when `write_tar =
+True`) has been corrected, ensuring the file is no longer root-owned.

releng/release-notes-next/chroot-scan-util-do.bugfix

@@ -0,0 +1,3 @@
+The `chroot_scan` plugin now consistently uses the `util.do` method instead of
+custom `subprocess.call` calls.  This ensures that the `mock --verbose` output
+properly displays the commands (like `cp`, or `tar`) being executed.

releng/release-notes-next/chroot_scan_resultdir.bugfix

@@ -0,0 +1,3 @@
+Mock v5.7 introduced a regression in the `chroot_scan` plugin that prevented the
+result directory from being created properly. This issue has been
+[fixed][PR#1472].