Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only %prep once when running %generate_buildrequires multiple times #1250

Merged
merged 1 commit into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, the rpmbuild call will
# execute the %generate_buildrequires section 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`.