Back up on scrub

mock/py/mockbuild/backend.py

@@ -56,33 +56,15 @@ def __init__(self, config, uid_manager, plugins, state, buildroot, bootstrap_bui
         self.more_buildreqs = config['more_buildreqs']
         self.cache_alterations = config['cache_alterations']
 
-        self.backup = config['backup_on_clean']
-        self.backup_base_dir = config['backup_base_dir']
-
         # do we allow interactive root shells?
         self.no_root_shells = config['no_root_shells']
 
         self.private_network = not config['rpmbuild_networking']
         self.rpmbuild_noclean_option = None
 
-    @traceLog()
-    def backup_results(self):
-        srcdir = os.path.join(self.buildroot.basedir, "result")
-        if not os.path.exists(srcdir):
-            return
-        dstdir = os.path.join(self.backup_base_dir, self.config['root'])
-        file_util.mkdirIfAbsent(dstdir)
-        rpms = glob.glob(os.path.join(srcdir, "*rpm"))
-        if len(rpms) == 0:
-            return
-        self.state.state_log.info("backup_results: saving with cp %s %s", " ".join(rpms), dstdir)
-        util.run(cmd="cp %s %s" % (" ".join(rpms), dstdir))
-
     @traceLog()
     def clean(self):
         """clean out chroot with extreme prejudice :)"""
-        if self.backup:
-            self.backup_results()
         self.state.start("clean chroot")
         self.buildroot.delete()
         self.state.finish("clean chroot")

mock/py/mockbuild/buildroot.py

@@ -1046,12 +1046,37 @@ def wrap_host_file(self, filename):
 
         return util.BindMountedFile(chroot_filename, host_filename)
 
+    @traceLog()
+    def backup_build_results(self):
+        """
+        Back up built RPMs if `backup_on_clean` is enabled, before cleaning the chroot and results.
+        """
+        if not self.config['backup_on_clean']:
+            return
+        srcdir = os.path.join(self.basedir, "result")
+        if not os.path.exists(srcdir):
+            return
+        rpms = glob.glob(os.path.join(srcdir, "*rpm"))
+        if len(rpms) == 0:
+            return
+        dstdir = os.path.join(self.config['backup_base_dir'], self.config['root'])
+        file_util.mkdirIfAbsent(dstdir)
+        self.state.state_log.info("backup_on_clean (completed builds): saving with mv %s %s", " ".join(rpms), dstdir)
+        for rpm in rpms:
+            dest_path = os.path.join(dstdir, os.path.basename(rpm))
+            try:
+                os.replace(rpm, dest_path)
+            except OSError as e:
+                self.state.state_log.error("backup_on_clean (completed builds): error moving %s to %s: %s",
+                        rpm, dest_path, e)
+
     @traceLog()
     def delete(self):
         """
         Deletes the buildroot contents.
         """
         if os.path.exists(self.basedir):
+            self.backup_build_results()
             p = self.make_chroot_path()
             self._lock_buildroot(exclusive=True)
             util.orphansKill(p)

releng/release-notes-next/back_up_on_scrub.bugfix

@@ -0,0 +1,8 @@
+`mock --scrub=all` now correctly backs up successful builds from the buildroot. Fixes issue #1639.
+
+The backup process now uses `mv` semantics instead of `cp`, avoiding file duplication,
+preserving timestamps, and improving performance. Gemini Code Assist flagged the
+use of `util.run` as unsafe, so it was replaced with `os.replace` to safely overwrite
+existing files.
+
+Improve logs and comments in backup_build_results for clarity.