-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Allow building content-addressed derivations with hydra - reprise #1228
Closed
Closed
Changes from 38 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
00c2e2c
Fix with ca derivations
thufschmitt 030d72e
Add some tests for the content-addressed derivations
thufschmitt 111abeb
Properly register the unresolved derivation
thufschmitt 22a638e
Fix a localStore/destStore confusion
thufschmitt ddfece5
Check for previous builds using the drvPath rather than the output paths
thufschmitt fad8839
Don't call queryPartialDerivationOutputMap on the dest store
thufschmitt 410d7a0
getBuildOutput: Don't query the dest store for the outputs
thufschmitt 68d18b2
Correctly handle the non-ca case when building
thufschmitt af8f070
Allow gc-ing the destination store
thufschmitt 70402e5
Actually use the `content-addressed` test
thufschmitt d1e41cf
Update the ca test
thufschmitt c5a092e
Clarify what happens with the out paths in the db
thufschmitt d243424
Move mkDerivation-for-ca to config.nix.in
thufschmitt 4a6a342
eval-jobs: Don't strip the output path for ia derivations
thufschmitt 89a55b9
Add the output paths of CA derivations to the database
thufschmitt 34aaa0b
Add more ca tests
thufschmitt 2a2aa72
Test evaluating a ca drv without the experimental feature
thufschmitt a07b1db
Move the ca tests to a subdirectory
thufschmitt 3d64cfb
Fix a potential queue-runner crash
thufschmitt 9f2b576
Prevent enabling ca-derivations from breaking substitution
thufschmitt a79afe0
Document the state of ca derivations
thufschmitt f676dc8
Also register the ca outputs on the dest store
thufschmitt 79e5e73
Fix the resolving of derivations
thufschmitt 529c28f
Make non-ca-depending-on-ca derivations work
thufschmitt c6005f4
Correctly handle “real” input-addressed derivations
thufschmitt 1036075
Make perlcritic happy
thufschmitt a084efc
Extract some parts of `State::buildRemote` into auxiliary functions
thufschmitt 46ae574
Fix build with Nix 2.6pre
thufschmitt cdff806
Flake: Update
thufschmitt 77df6b4
Merge remote-tracking branch 'thufschmitt/nix-ca' into nix-ca4
aciceri 8b34b2f
Don't query outputs in case of CA derivations
aciceri d8639ac
Using new libstore method and fixes due to CA derivations
aciceri de2fc58
`string` -> `std::string`
aciceri 0ef435e
Forgot to remove old comment in the merge
aciceri db89bbe
Fixed regression to #1126
aciceri 8d52640
DB model changed
aciceri 954e41f
Keep track if step build outputs are CA or not
aciceri 3fdc9e4
Improved UI to correctly handle CA derivations
aciceri 391ada9
"Content addressed" field in builds details
aciceri 8311b49
Merge branch 'NixOS:master' into aciceri/ca-derivations
aciceri File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remote-building CA Derivations seems to fail here because the destStore already has the realisation registered.incorrect, see below. My hydra has this patch applied although you can probably remove the try {} catch for the localStore and pattern match the error to make sure it is about a constraint failure when inserting into RealisationsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, also removing the try/catch for the local store and pattern matching the other errors.
Do you already know how can we check that the error is caused by a constraint failure?
However I would prefer leaving the final word to @thufschmitt about this part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t know enough about nix’s c++ code to know how to match a specific errorWill try changing the catch type to SQLiteErroralso it seems i was wrong about the “remove the localStore try catch blocks” because the error only seems to occur on the local parts my bad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying this out made me discover that the issue is not that the realisation already exists but that the out output is not copied over for some reason