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

debug #6383 #6484

Closed
wants to merge 2 commits into from
Closed

debug #6383 #6484

wants to merge 2 commits into from

Conversation

fogti
Copy link
Contributor

@fogti fogti commented May 4, 2022

This PR inserts the missing debug output to better diagnose what went wrong in #6383; and fixes a potential use-after-free via getOr.

The main problem is that the error might take a long time to appear, but once it appears is reproducible if the build output is really required (and not just because the output got pulled in by a full build, but is not used by any other derivation)

@fogti fogti force-pushed the debug-6383 branch 5 times, most recently from 7a19904 to 0075fa9 Compare May 4, 2022 15:13
@@ -988,13 +988,28 @@ void DerivationGoal::resolvedFinished()
auto resolvedHash = get(resolvedHashes, wantedOutput);
if ((!initialOutput) || (!resolvedHash))
throw Error(
"derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,resolve)",
"derivation '%s' doesn't have expected output '%s'",
worker.store.printStorePath(drvPath), wantedOutput);
auto realisation = get(resolvedResult.builtOutputs, DrvOutput { *resolvedHash, wantedOutput });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

example failure (reported by @trofi):

error: derivation '/nix/store/g7q24qrdzdcvsirxw8aw0alc9s0qsl77-shared-mime-info-unstable-2021-12-03.drv' doesn't have expected output 'dev @ sha256:094p4p14vbmqhhd2g4nr08phln6hr789rzqnjwll9q79kisc7n7g'
        builtOutputs:
                out @ sha256:094p4p14vbmqhhd2g4nr08phln6hr789rzqnjwll9q79kisc7n7g (key) = /nix/store/98f30ch5mldzkw0zznx2s2ll6r940jv3-shared-mime-info-unstable-2021-12-03

=> hash matches, but the wanted output is not part of the built outputs.

@stale stale bot added the stale label Nov 1, 2022
@Ericson2314
Copy link
Member

I am a little confused what is going with all the explicit construction; are you thinking what we did before is more unsafe than we think?

@stale stale bot removed the stale label Jun 14, 2023
@fogti
Copy link
Contributor Author

fogti commented Jun 15, 2023

Most of the explicit construction is to make sure that strings are still valid when used, and that the types used are correct (the only important cases are Path, which forces a copy, and string_view, which forces a proper string reference). But as this patch by now diverged anyways, I'm just going to close it. I haven't encountered the bug this is supposed to debug in the last few months.

@fogti fogti closed this Jun 15, 2023
@fogti fogti deleted the debug-6383 branch June 15, 2023 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants