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

Mutiple replaceRuntimeDependencies may reintroduce depenendices #4336

Closed
roconnor opened this issue Sep 30, 2014 · 5 comments · Fixed by #257234
Closed

Mutiple replaceRuntimeDependencies may reintroduce depenendices #4336

roconnor opened this issue Sep 30, 2014 · 5 comments · Fixed by #257234

Comments

@roconnor
Copy link
Contributor

replaceRuntimeDependencies is implemented by doing a fold over the list of runtime dependency replacements. The issues is that after replacing one depencency, say bash, with an updated version, the next replacement might replace say firefox, with an updated version that depends on the old bash. This is probably unexpected from the user's perspective and could cause dangerous surprises if a user is using this as a security update mechanism.

I should say that this is only a theoretical issue; I haven't verified this acutally happens, but I wanted to file an issue before I forget about it.

One possible solution is to do some sort of topological sort of the list of replacements based on the dependency order of the "new" packages on the "old" packages so that outermost dependencies are replaced first. But some since "new" packages may have completely different run-time dependencies some though is needed to make sure this works right.

I think a better solution is to come up with an elegent way of simulateously processing all replacements directly in the replaceDependency code, but I would need to sit down and think about how to do this.

@shlevy
Copy link
Member

shlevy commented Oct 1, 2014

I won't object to a good implementation of this, but IMO replaceRuntimeDependencies is mostly useful as a stop-gap and should be replaced by a proper rebuild once packages are available, so the frequency of having two packages to replace at the same time is low enough not to prioritize this. I'll unassign myself, but please do let me know if you have a good way to do it.

@shlevy shlevy removed their assignment Oct 1, 2014
@roconnor roconnor self-assigned this Oct 1, 2014
@Profpatsch
Copy link
Member

(triage) @roconnor Do you still want to follow up on this?

@Profpatsch
Copy link
Member

No response, so close @zimbatm.

@zimbatm
Copy link
Member

zimbatm commented Mar 20, 2016

This is going to be replaced by a proper solution soon (hopefully). See #14000 and related work.

@zimbatm zimbatm closed this as completed Mar 20, 2016
@nbp
Copy link
Member

nbp commented Mar 20, 2016

This issue would be fixed by #10851 , because the patching mechanism works by constructing the patches out of all the fixed packages, instead of chaining independent replacement of dependencies.

I even have a test case which I used to verify that #10851 is working properly:
https://github.com/nbp/nixpkgs-2/blob/08f53825e305bbaf4610cf9c5b0d2dad102108cc/pkgs/test/security/check-quickfix.nix

@nbp nbp mentioned this issue Mar 27, 2016
6 tasks
alois31 added a commit to alois31/nixpkgs that referenced this issue Sep 25, 2023
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Sep 25, 2023
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Oct 12, 2023
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Nov 11, 2023
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Dec 10, 2023
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Dec 16, 2023
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Jan 27, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
bryango pushed a commit to bryango/nixpkgs that referenced this issue Feb 23, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
bryango pushed a commit to bryango/nixpkgs that referenced this issue Feb 23, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Feb 27, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Jun 12, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
alois31 added a commit to alois31/nixpkgs that referenced this issue Jun 14, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
yu-re-ka pushed a commit that referenced this issue Sep 24, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: #4336
iFreilicht pushed a commit to iFreilicht/nixpkgs that referenced this issue Sep 24, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
theoparis pushed a commit to tinted-software/nixpkgs that referenced this issue Sep 24, 2024
Instead of iterating over all replacements and applying them one by one,
use the newly introduced replaceDependencies function to apply them all
at once for replaceRuntimeDependencies. The advantages are twofold in
case there are multiple replacements:
* Performance is significantly improved, because there is only one pass
  over the closure to be made.
* Correctness is improved, because replaceDependencies also replaces
  dependencies of the replacements themselves if applicable.

Fixes: NixOS#4336
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants