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

nix store repair --keep-going #11877

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

roberth
Copy link
Member

@roberth roberth commented Nov 13, 2024

Motivation

Fix what we can.
Tested in anger before adding RepairFailure.

TODO

  • test
    • integration test, or better: perform the refactor below, and unit test this
  • make this the default behavior
  • open issue to factor out the keeping going logic
    • add concurrency (tends to go hand in hand with freedom to pick strictness behavior!)
  • create/use a CLI mixin to operate on sets, not lists of paths

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

So that we can recognize it in an upcoming commit.
@roberth roberth added new-cli Relating to the "nix" command store Issues and pull requests concerning the Nix store labels Nov 13, 2024
Copy link
Member Author

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 

Comment on lines +24 to +29
// Remove duplicates.
StorePathSet storePathsSet;
for (auto & path : storePaths)
store->repairPath(path);
storePathsSet.insert(path);

for (auto & path : storePathsSet) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be bottom-up, ie dependencies first. That way when we do a "substitution fallback" build, we don't use broken inputs.

Copy link
Member

@bryanhonof bryanhonof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance, this looks okay to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-cli Relating to the "nix" command store Issues and pull requests concerning the Nix store
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants