Skip to content

Commit 9503dd4

Browse files
committed
Fix for issue #1639: make mock --scrub=all correctly back up successful builds from the chroot
Use `mv` instead of `cp` for backing up RPM builds. Moving saves time and disk space and preserves the original build timestamps. Replaced the `util.run` call with `shutil.move`, following the suggestion from gemini-code-assist, then replaced `shutil.move` with `os.replace` to safely overwrite existing files. Rename function backup_results to backup_build_results and replace self.backup with self.backup_on_clean for improved clarity and readability.
1 parent d30ff4b commit 9503dd4

File tree

3 files changed

+31
-18
lines changed

3 files changed

+31
-18
lines changed

mock/py/mockbuild/backend.py

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,33 +56,15 @@ def __init__(self, config, uid_manager, plugins, state, buildroot, bootstrap_bui
5656
self.more_buildreqs = config['more_buildreqs']
5757
self.cache_alterations = config['cache_alterations']
5858

59-
self.backup = config['backup_on_clean']
60-
self.backup_base_dir = config['backup_base_dir']
61-
6259
# do we allow interactive root shells?
6360
self.no_root_shells = config['no_root_shells']
6461

6562
self.private_network = not config['rpmbuild_networking']
6663
self.rpmbuild_noclean_option = None
6764

68-
@traceLog()
69-
def backup_results(self):
70-
srcdir = os.path.join(self.buildroot.basedir, "result")
71-
if not os.path.exists(srcdir):
72-
return
73-
dstdir = os.path.join(self.backup_base_dir, self.config['root'])
74-
file_util.mkdirIfAbsent(dstdir)
75-
rpms = glob.glob(os.path.join(srcdir, "*rpm"))
76-
if len(rpms) == 0:
77-
return
78-
self.state.state_log.info("backup_results: saving with cp %s %s", " ".join(rpms), dstdir)
79-
util.run(cmd="cp %s %s" % (" ".join(rpms), dstdir))
80-
8165
@traceLog()
8266
def clean(self):
8367
"""clean out chroot with extreme prejudice :)"""
84-
if self.backup:
85-
self.backup_results()
8668
self.state.start("clean chroot")
8769
self.buildroot.delete()
8870
self.state.finish("clean chroot")

mock/py/mockbuild/buildroot.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ def __init__(self, config, uid_manager, state, plugins, bootstrap_buildroot=None
9999
self._lock_file = None
100100
self.selinux = (not self.config['plugin_conf']['selinux_enable']
101101
and util.selinuxEnabled())
102+
self.backup_on_clean = config['backup_on_clean']
103+
self.backup_base_dir = config['backup_base_dir']
102104

103105
self.chrootuid = config['chrootuid']
104106
self.chrootuser = config['chrootuser']
@@ -1046,12 +1048,36 @@ def wrap_host_file(self, filename):
10461048

10471049
return util.BindMountedFile(chroot_filename, host_filename)
10481050

1051+
@traceLog()
1052+
def backup_build_results(self):
1053+
"""
1054+
Back up the built RPMs before deleting the chroot and results, if the `backup_on_clean` option is enabled.
1055+
"""
1056+
if self.backup_on_clean:
1057+
srcdir = os.path.join(self.basedir, "result")
1058+
if not os.path.exists(srcdir):
1059+
return
1060+
dstdir = os.path.join(self.config['backup_base_dir'], self.config['root'])
1061+
file_util.mkdirIfAbsent(dstdir)
1062+
rpms = glob.glob(os.path.join(srcdir, "*rpm"))
1063+
if len(rpms) == 0:
1064+
return
1065+
self.state.state_log.info("backup_build_results: saving with mv %s %s", " ".join(rpms), dstdir)
1066+
for rpm in rpms:
1067+
dest_path = os.path.join(dstdir, os.path.basename(rpm))
1068+
try:
1069+
os.replace(rpm, dest_path)
1070+
except OSError as e:
1071+
self.state.state_log.error("backup_build_results: error moving %s to %s: %s",
1072+
rpm, dest_path, e)
1073+
10491074
@traceLog()
10501075
def delete(self):
10511076
"""
10521077
Deletes the buildroot contents.
10531078
"""
10541079
if os.path.exists(self.basedir):
1080+
self.backup_build_results()
10551081
p = self.make_chroot_path()
10561082
self._lock_buildroot(exclusive=True)
10571083
util.orphansKill(p)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
`mock --scrub=all` now correctly backs up successful builds from the buildroot. Fixes issue #1639.
2+
The backup process now uses `mv` semantics instead of `cp`, avoiding file duplication, preserving timestamps, and improving performance.
3+
Replaced the `util.run` call with `shutil.move`, following the suggestion from gemini-code-assist and providing a more idiomatic and portable Python implementation.
4+
Fix gemini-code-assist suggestion: `shutil.move` may raise "Error: Destination path already exists"; use `os.replace` to safely overwrite.
5+
Rename function backup_results to backup_build_results and replace self.backup with self.backup_on_clean for improved clarity and readability.

0 commit comments

Comments
 (0)