core: Strengthen how we enforce lockfiles #1849
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
It's nicer to read/less verbose.
We no longer tell libdnf to add the pkg to the goal until later (because of rpm-software-management/libdnf#700). Just tweak those comments to be more explicit about that.
One problem with how we use lockfiles right now is that we don't enforce them for dependencies. That is, if `foo` requires `bar`, but only `foo` is in the manifest, then while `foo` will be locked, `bar` will never be checked against the lockfile because it was never explicitly requested. Higher-level though, I don't like how indirect the locking here feels. See some comments about that in: coreos#1745 (comment) coreos#1745 (comment) Essentially, the manifest is an input file of patterns, and all we really know from the lockfile output is that the set of packages in there satisfies this input in some way. But: 1. there are multiple ways to satisfy the same input (hence why hints like `SOLVER_FAVOR` exist) 2. the solution is dependent on how the solver is implemented (i.e. different libsolv versions might yield different solutions) 3. the solution is dependent on flags fed to the solver (i.e. different libdnf versions might yield different solutions) So any attempt at cross-checking between the input file and the lockfile is going to be very hard. Using a stricter mode as I suggested in coreos#1745 of only allowing pure pkgnames or NEVRAs would help, but it wouldn't address the dependency issue. (Though I'm still thinking about possibly doing this anyway.) The solution I propose here is instead to take the nuclear approach: we completely exclude from the sack all packages of the same name as packages in our lockfiles, but which do not match the NEVRA. Therefore, any possible solution has to also satisfy our lockfile (or error out).
cgwalters
reviewed
Jun 5, 2019
|
Bugs fixed; maintain course! 🚀 |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 6, 2019
It's nicer to read/less verbose. Closes: #1849 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 6, 2019
We no longer tell libdnf to add the pkg to the goal until later (because of rpm-software-management/libdnf#700). Just tweak those comments to be more explicit about that. Closes: #1849 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 6, 2019
One problem with how we use lockfiles right now is that we don't enforce them for dependencies. That is, if `foo` requires `bar`, but only `foo` is in the manifest, then while `foo` will be locked, `bar` will never be checked against the lockfile because it was never explicitly requested. Higher-level though, I don't like how indirect the locking here feels. See some comments about that in: #1745 (comment) #1745 (comment) Essentially, the manifest is an input file of patterns, and all we really know from the lockfile output is that the set of packages in there satisfies this input in some way. But: 1. there are multiple ways to satisfy the same input (hence why hints like `SOLVER_FAVOR` exist) 2. the solution is dependent on how the solver is implemented (i.e. different libsolv versions might yield different solutions) 3. the solution is dependent on flags fed to the solver (i.e. different libdnf versions might yield different solutions) So any attempt at cross-checking between the input file and the lockfile is going to be very hard. Using a stricter mode as I suggested in #1745 of only allowing pure pkgnames or NEVRAs would help, but it wouldn't address the dependency issue. (Though I'm still thinking about possibly doing this anyway.) The solution I propose here is instead to take the nuclear approach: we completely exclude from the sack all packages of the same name as packages in our lockfiles, but which do not match the NEVRA. Therefore, any possible solution has to also satisfy our lockfile (or error out). Closes: #1849 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 6, 2019
It's nicer to read/less verbose. Closes: #1849 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 6, 2019
We no longer tell libdnf to add the pkg to the goal until later (because of rpm-software-management/libdnf#700). Just tweak those comments to be more explicit about that. Closes: #1849 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 6, 2019
One problem with how we use lockfiles right now is that we don't enforce them for dependencies. That is, if `foo` requires `bar`, but only `foo` is in the manifest, then while `foo` will be locked, `bar` will never be checked against the lockfile because it was never explicitly requested. Higher-level though, I don't like how indirect the locking here feels. See some comments about that in: #1745 (comment) #1745 (comment) Essentially, the manifest is an input file of patterns, and all we really know from the lockfile output is that the set of packages in there satisfies this input in some way. But: 1. there are multiple ways to satisfy the same input (hence why hints like `SOLVER_FAVOR` exist) 2. the solution is dependent on how the solver is implemented (i.e. different libsolv versions might yield different solutions) 3. the solution is dependent on flags fed to the solver (i.e. different libdnf versions might yield different solutions) So any attempt at cross-checking between the input file and the lockfile is going to be very hard. Using a stricter mode as I suggested in #1745 of only allowing pure pkgnames or NEVRAs would help, but it wouldn't address the dependency issue. (Though I'm still thinking about possibly doing this anyway.) The solution I propose here is instead to take the nuclear approach: we completely exclude from the sack all packages of the same name as packages in our lockfiles, but which do not match the NEVRA. Therefore, any possible solution has to also satisfy our lockfile (or error out). Closes: #1849 Approved by: cgwalters
|
💔 Test failed - status-atomicjenkins |
|
@rh-atomic-bot retry |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 6, 2019
We no longer tell libdnf to add the pkg to the goal until later (because of rpm-software-management/libdnf#700). Just tweak those comments to be more explicit about that. Closes: #1849 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 6, 2019
One problem with how we use lockfiles right now is that we don't enforce them for dependencies. That is, if `foo` requires `bar`, but only `foo` is in the manifest, then while `foo` will be locked, `bar` will never be checked against the lockfile because it was never explicitly requested. Higher-level though, I don't like how indirect the locking here feels. See some comments about that in: #1745 (comment) #1745 (comment) Essentially, the manifest is an input file of patterns, and all we really know from the lockfile output is that the set of packages in there satisfies this input in some way. But: 1. there are multiple ways to satisfy the same input (hence why hints like `SOLVER_FAVOR` exist) 2. the solution is dependent on how the solver is implemented (i.e. different libsolv versions might yield different solutions) 3. the solution is dependent on flags fed to the solver (i.e. different libdnf versions might yield different solutions) So any attempt at cross-checking between the input file and the lockfile is going to be very hard. Using a stricter mode as I suggested in #1745 of only allowing pure pkgnames or NEVRAs would help, but it wouldn't address the dependency issue. (Though I'm still thinking about possibly doing this anyway.) The solution I propose here is instead to take the nuclear approach: we completely exclude from the sack all packages of the same name as packages in our lockfiles, but which do not match the NEVRA. Therefore, any possible solution has to also satisfy our lockfile (or error out). Closes: #1849 Approved by: cgwalters
|
☀️ Test successful - status-atomicjenkins |
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.
One problem with how we use lockfiles right now is that we don't enforce
them for dependencies. That is, if
foorequiresbar, but onlyfoois in the manifest, then while
foowill be locked,barwill neverbe checked against the lockfile because it was never explicitly
requested.
Higher-level though, I don't like how indirect the locking here feels.
See some comments about that in:
#1745 (comment)
#1745 (comment)
Essentially, the manifest is an input file of patterns, and all we
really know from the lockfile output is that the set of packages in
there satisfies this input in some way. But:
like
SOLVER_FAVORexist)different libsolv versions might yield different solutions)
libdnf versions might yield different solutions)
So any attempt at cross-checking between the input file and the lockfile
is going to be very hard. Using a stricter mode as I suggested in #1745
of only allowing pure pkgnames or NEVRAs would help, but it wouldn't
address the dependency issue. (Though I'm still thinking about possibly
doing this anyway.)
The solution I propose here is instead to take the nuclear approach: we
completely exclude from the sack all packages of the same name as
packages in our lockfiles, but which do not match the NEVRA. Therefore,
any possible solution has to also satisfy our lockfile (or error out).