lix-project/lix

Fork 58

`nix build --rebuild` on an unbuilt derivation fails with "some outputs of '...' are not valid, so checking is not possible" #485

New issue

Open

opened 2024-08-23 23:37:38 +00:00 by rbt · 6 comments

rbt commented

2024-08-23 23:37:38 +00:00

Owner

Describe the bug

If you use nix build --rebuild or nix-build --check on a derivation that has not been built locally yet, Nix will fail with an inscrutable error message that does not indicate you should remove the --rebuild flag.

Steps To Reproduce

With unbuilt.nix:

derivation {
  name = "unbuilt";
  builder = "bash";
  system = "aarch64-darwin";
}

$ nix build --rebuild --file unbuilt.nix
error: some outputs of '/nix/store/dljzaa16741fjkb0qs1bfcxjjrhqycx5-unbuilt.drv' are not valid, so checking is not possible

Note that this derivation will fail to build, but for a different reason:

$ nix build --file unbuilt.nix
error: builder for '/nix/store/dljzaa16741fjkb0qs1bfcxjjrhqycx5-unbuilt.drv' failed with exit code 1;
       last 1 log lines:
       > error: executing 'bash': Undefined error: 0
       For full logs, run 'nix-store -l /nix/store/dljzaa16741fjkb0qs1bfcxjjrhqycx5-unbuilt.drv'.

Expected behavior

Really it should just go ahead and build the derivation, but I would settle for it telling me to remove the --rebuild flag.

`nix --version` output

nix (Lix, like Nix) 2.91.0

Additional context

#66 looks very similar, but it's emitted from a different code path.

## Describe the bug If you use `nix build --rebuild` or `nix-build --check` on a derivation that has not been built locally yet, Nix will fail with an inscrutable error message that does not indicate you should remove the `--rebuild` flag. ## Steps To Reproduce With `unbuilt.nix`: ```nix derivation { name = "unbuilt"; builder = "bash"; system = "aarch64-darwin"; } ``` ``` $ nix build --rebuild --file unbuilt.nix error: some outputs of '/nix/store/dljzaa16741fjkb0qs1bfcxjjrhqycx5-unbuilt.drv' are not valid, so checking is not possible ``` Note that this derivation _will_ fail to build, but for a different reason: ``` $ nix build --file unbuilt.nix error: builder for '/nix/store/dljzaa16741fjkb0qs1bfcxjjrhqycx5-unbuilt.drv' failed with exit code 1; last 1 log lines: > error: executing 'bash': Undefined error: 0 For full logs, run 'nix-store -l /nix/store/dljzaa16741fjkb0qs1bfcxjjrhqycx5-unbuilt.drv'. ``` ## Expected behavior Really it should just go ahead and build the derivation, but I would settle for it telling me to remove the `--rebuild` flag. ## `nix --version` output ``` nix (Lix, like Nix) 2.91.0 ``` ## Additional context #66 looks very similar, but it's emitted from a different code path.

rbt added the

bug

label

2024-08-23 23:37:38 +00:00

jade commented

2024-09-26 00:45:00 +00:00

Owner

dupe #281, but your report is better, so im going to close the other one.

dupe https://git.lix.systems/lix-project/lix/issues/281, but your report is better, so im going to close the other one.

jade referenced this issue

2024-09-26 00:45:10 +00:00

make --rebuild work if the store path isn't already built #281

jade added the

E/help wanted

E/reproducible

labels

2024-09-26 00:45:46 +00:00

rbt commented

2024-10-04 17:59:45 +00:00

Author

Owner

💭 i'm going to get a good grade in bug reports, something that's normal to want & possible to achieve

lunaphied self-assigned this

2025-10-20 10:38:12 +00:00

raito added the

release-blocker

label

2025-11-17 22:30:28 +00:00

lunaphied was unassigned by raito

2025-11-30 16:56:57 +00:00

pennae added this to the 2.95 milestone

2025-12-01 14:51:44 +00:00

raito commented

2025-12-05 01:34:43 +00:00

Owner

This is the context where this occurs:

    auto [allValid, validOutputs] = TRY_AWAIT(checkPathValidity());

    // recheck needRestart. more wanted outputs may have been added during the
    // path validity check, and we do not want to treat !allValid as an error.
    if (!allValid && needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) {
        needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed;
        co_return TRY_AWAIT(haveDerivation());
    }

    if (buildMode == bmNormal && allValid) {
        co_return done(BuildResult::Substituted, std::move(validOutputs));
    }
    if (buildMode == bmRepair && allValid) {
        co_return TRY_AWAIT(repairClosure());
    }
    if (buildMode == bmCheck && !allValid)
        throw Error("some outputs of '%s' are not valid, so checking is not possible",
            worker.store.printStorePath(drvPath));

My view is that this check is critical to perform a check over non-locally-rebuildable derivations that exist locally in your store but are only available via substitution.

So it needs to stay. If we want to make a bmNormal build take place because the original path does not exist, we need to determine earlier if we are in bmCheck and the target original path simply is unregistered (and not in an unknown state wrt corruption) then schedule a normal goal to build this and wait until its completion then resume operations including this code.

This could take place in haveDerivation around here:

    /* Check what outputs paths are not already valid. */
    auto [allValid, validOutputs] = TRY_AWAIT(checkPathValidity());

    /* If they are all valid, then we're done. */
    if (allValid && buildMode == bmNormal) {
        co_return done(BuildResult::AlreadyValid, std::move(validOutputs));
    }

    // New proposal code:
    if (!allValid && buildMode == bmCheck) {
        printInfo(
            "derivation did not already exist in the store entirely, realizing for the first time"
        );
        // NOTE: we could optimize and remove outputs we already have based on validOutputs.
        // i.e. compute wantedOutputs - validOutputs.
        kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
        dependencies.add(worker.goalFactory().makeDerivationGoal(drvPath, wantedOutputs, bmNormal));
        TRY_AWAIT(
            waitForGoals(dependencies.releaseAsArray())
        );
    }

Alas, this naive implementation deadlocks and I surmise this is because we are ALREADY realizing that derivation in bmCheck mode and the goal loop is like "nah, wait for the other".

This is the context where this occurs: ```cpp auto [allValid, validOutputs] = TRY_AWAIT(checkPathValidity()); // recheck needRestart. more wanted outputs may have been added during the // path validity check, and we do not want to treat !allValid as an error. if (!allValid && needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) { needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; co_return TRY_AWAIT(haveDerivation()); } if (buildMode == bmNormal && allValid) { co_return done(BuildResult::Substituted, std::move(validOutputs)); } if (buildMode == bmRepair && allValid) { co_return TRY_AWAIT(repairClosure()); } if (buildMode == bmCheck && !allValid) throw Error("some outputs of '%s' are not valid, so checking is not possible", worker.store.printStorePath(drvPath)); ``` My view is that this check is critical to perform a check over non-locally-rebuildable derivations that exist locally in your store but are only available via substitution. So it needs to stay. If we want to make a `bmNormal` build take place because the original path does not exist, we need to determine earlier if we are in `bmCheck` and the target original path simply is unregistered (and not in an unknown state wrt corruption) then schedule a normal goal to build this and wait until its completion then resume operations including this code. This could take place in `haveDerivation` around here: ```cpp /* Check what outputs paths are not already valid. */ auto [allValid, validOutputs] = TRY_AWAIT(checkPathValidity()); /* If they are all valid, then we're done. */ if (allValid && buildMode == bmNormal) { co_return done(BuildResult::AlreadyValid, std::move(validOutputs)); } // New proposal code: if (!allValid && buildMode == bmCheck) { printInfo( "derivation did not already exist in the store entirely, realizing for the first time" ); // NOTE: we could optimize and remove outputs we already have based on validOutputs. // i.e. compute wantedOutputs - validOutputs. kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies; dependencies.add(worker.goalFactory().makeDerivationGoal(drvPath, wantedOutputs, bmNormal)); TRY_AWAIT( waitForGoals(dependencies.releaseAsArray()) ); } ``` Alas, this naive implementation deadlocks and I surmise this is because we are *ALREADY* realizing that derivation in `bmCheck` mode and the goal loop is like "nah, wait for the other".

raito added the

E/hard

label

2025-12-05 01:34:51 +00:00

pennae commented

2025-12-06 14:14:36 +00:00

Owner

Alas, this naive implementation deadlocks and I surmise this is because we are ALREADY realizing that derivation in bmCheck mode and the goal loop is like "nah, wait for the other".

correct, your makeDerivationGoal will return the goal you're calling this from because goals are cached with the derivation path as the cache key. building an output we don't have only to build it again directly after is arguably not even what --rebuild and --check express as intent, so maybe we should just change the error message to something more meaningful?

> Alas, this naive implementation deadlocks and I surmise this is because we are ALREADY realizing that derivation in bmCheck mode and the goal loop is like "nah, wait for the other". correct, your `makeDerivationGoal` will return the goal you're calling this from because goals are cached with the derivation path as the cache key. building an output we don't have only to build it *again* directly after is arguably not even what `--rebuild` and `--check` express as intent, so maybe we should just change the error message to something more meaningful?

raito commented

2025-12-06 17:36:14 +00:00

Owner

so maybe we should just change the error message to something more meaningful?

we could do that as a first thing to close this issue here then we can open a new bug for making the UX of --rebuild/--check nicer I'd say because having to build yourself once then run it again looks needlessly painful to me.

> so maybe we should just change the error message to something more meaningful? we could do that as a first thing to close this issue here then we can open a new bug for making the UX of --rebuild/--check nicer I'd say because having to build yourself once then run it again looks needlessly painful to me.

lix-bot commented

2025-12-06 19:24:20 +00:00

Member

This issue was mentioned on Gerrit on the following CLs:

commit message in cl/4720 ("libstore/build: report better error messages for --check")

This issue was mentioned on Gerrit on the following CLs: * commit message in [cl/4720](https://gerrit.lix.systems/c/lix/+/4720) ("libstore/build: report better error messages for --check")

raito referenced this issue

2025-12-06 19:26:47 +00:00

--check or --rebuild should automate recovery when feasible #1067

raito self-assigned this

2025-12-07 00:26:50 +00:00