mock: fix dynamic buildrequires unnecesarry loop #482
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hrnciar
force-pushed
the
dynamic-buildrequires-unnecessary-loop
branch
2 times, most recently
from
afbb79f
to
b735e72
Compare
praiskup
reviewed
Feb 21, 2020
mock/py/mockbuild/backend.py
Outdated
| @@ -699,7 +699,6 @@ def get_command(mode): | |||
| # * it fails | |||
| # * installSrpmDeps does nothing | |||
| # * or we run out of dynamic_buildrequires_max_loops tries | |||
| packages_before = self.buildroot.all_chroot_packages() | |||
There was a problem hiding this comment.
Yes, we can drop this (no need to access the RPM db by rpm -qa), but we
need a replacement here. Most likely you need to maintain a set() of
real build requirements across the iteration... in each step you can get
the list from installSrpmDeps with small enhancements.
hrnciar
force-pushed
the
dynamic-buildrequires-unnecessary-loop
branch
from
b735e72
to
13c095e
Compare
hrnciar
force-pushed
the
dynamic-buildrequires-unnecessary-loop
branch
from
13c095e
to
955fd7b
Compare
praiskup
reviewed
Mar 2, 2020
mock/py/mockbuild/backend.py
Outdated
| self.installSrpmDeps(*buildreqs) | ||
| packages_after = self.buildroot.all_chroot_packages() | ||
| if packages_after == packages_before: | ||
| new_deps = self.installSrpmDeps(*buildreqs, *deps) |
There was a problem hiding this comment.
Shouldn't we use just deps argument here, without star?
There was a problem hiding this comment.
I'd perhaps use ignored_deps=deps to underline the purpose of the argument.
praiskup
reviewed
Mar 2, 2020
mock/py/mockbuild/backend.py
Outdated
| @@ -180,24 +180,27 @@ def remove(self, *rpms): | |||
| self.buildroot.root_log.info(output) | |||
|
|
|||
| @traceLog() | |||
| def installSrpmDeps(self, *srpms): | |||
| def installSrpmDeps(self, *srpms, ignored_deps=None): | |||
| """Figure out deps from srpm. Call package manager to install them""" | |||
There was a problem hiding this comment.
Please state what ignored_deps means in the method documentation.
hrnciar
force-pushed
the
dynamic-buildrequires-unnecessary-loop
branch
3 times, most recently
from
9069255
to
84dc7a7
Compare
|
The commit message doesn't correspond to what is really changing. |
praiskup
reviewed
Mar 5, 2020
| @@ -708,6 +708,7 @@ def get_command(mode): | |||
| except Error as e: | |||
| if e.resultcode != 11: | |||
There was a problem hiding this comment.
Please document the fact that we accept exit_status 0 and exit_status 11, and why.
|
I am sorry, the PR is not yet finished. I forgot to add WIP tag. |
praiskup
added a commit
to praiskup/mock
that referenced
this pull request
Mar 5, 2020
Test that mockchain works, with tmpfs plugin ON (keep_mounted=True), nocache plugin ON, and dynamic buildrequires package. Relates: rpm-software-management#482, rpm-software-management#512, rpm-software-management#479
praiskup
added a commit
to praiskup/mock
that referenced
this pull request
Mar 5, 2020
Test that mockchain works, with tmpfs plugin ON (keep_mounted=True), nocache plugin ON, and dynamic buildrequires package. Relates: rpm-software-management#482, rpm-software-management#512, rpm-software-management#479
per discussion here: rpm-software-management/rpm#963 Mock should accept return code 0 as well as 11 from rpm. Later when rpm is fixed, mock will accept just 0.
hrnciar
force-pushed
the
dynamic-buildrequires-unnecessary-loop
branch
from
84dc7a7
to
30d0230
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
per discussion here: rpm-software-management/rpm#963
Mock should accept return code 0 from rpm.
Relates: #434