Skip to content

Commit

Permalink
Only %prep once when running %generate_buildrequires multiple times
Browse files Browse the repository at this point in the history
This saves time and IO and I believe this is what was originally intended.

While doing this I moved some code around. Namely:

 - moved the variables used only in the if dynamic_buildrequires block inside it
 - ensured the --nodeps for the final rpmbuild call is only added once
  • Loading branch information
hroncok committed Oct 19, 2023
1 parent a1639d7 commit 544d5f3
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 10 deletions.
22 changes: 12 additions & 10 deletions mock/py/mockbuild/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -747,17 +747,18 @@ def get_command(mode, checkdeps=False):
return command

bd_out = self.make_chroot_path(self.buildroot.builddir)
max_loops = int(self.config.get('dynamic_buildrequires_max_loops'))
success = False
dynamic_buildrequires = dynamic_buildrequires and self.config.get('dynamic_buildrequires')
if dynamic_buildrequires:
max_loops = int(self.config.get('dynamic_buildrequires_max_loops'))
success = False
br_mode = ['-br']
while not success and max_loops > 0:
# run rpmbuild+installSrpmDeps until
# * it fails
# * installSrpmDeps does nothing
# * or we run out of dynamic_buildrequires_max_loops tries
packages_before = self.buildroot.all_chroot_packages()
command = get_command(['-br'])
command = get_command(br_mode)
(output, returncode) = \
self.buildroot.doChroot(command,
shell=False, logger=self.buildroot.build_log, timeout=timeout,
Expand All @@ -781,13 +782,14 @@ def get_command(mode, checkdeps=False):
success = True
for f_buildreqs in buildreqs:
os.remove(f_buildreqs)
if not sc:
# We want to (re-)write src.rpm with dynamic BuildRequires,
# but with short-circuit it doesn't matter
mode = ['-ba']
# rpmbuild -br already does %prep, so we don't need waste time
# on re-doing it
mode += ['--noprep']
# The first rpmbuild -br already did %prep, so we don't need waste time
if '--noprep' not in br_mode:
br_mode += ['--noprep']
if not sc:
# We want to (re-)write src.rpm with dynamic BuildRequires,
# but with short-circuit it doesn't matter
mode = ['-ba']
mode += ['--noprep']

# When we used dynamic buildrequires, we must let rpmbuild check deps.
# The rpmbuild -ba call executes the %generate_buildrequires once again
Expand Down
22 changes: 22 additions & 0 deletions releng/release-notes-next/generate_buildrequires_prep.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Only run the `%prep` section once when running `%generate_buildrequires`
multiple times.
Previously Mock run `%prep` repeatedly before each `%generate_buildrequires`
round except for the last one.
This was inconsistent and unnecessary slow/wasteful.

When the original support for `%generate_buildrequires` landed into Mock,
the intention was to only call `%prep` once.
However when Mock added support for multiple rounds of
`%generate_buildrequires`, `%prep` ended up only being skipped in the final
`rpmbuild` call. This was an oversight.
`%prep` is now only called once, as originally intended.

Some RPM packages might be affected by the change, especially if a dirty
working directory after running `%generate_buildrequires` affects the results
of subsequent rounds of `%generate_buildrequires`.
However, such behavior was undefined and quite buggy even previously,
due to the lack of the `%prep` section in the final `rpmbuild` call.

Packages that need to run commands before every round of
`%generate_buildrequires` should place those commands in
the `%generate_buildrequires` section itself rather than `%prep`.

0 comments on commit 544d5f3

Please sign in to comment.