Skip to content

Add support for recursive search#129

Open
fzakaria wants to merge 4 commits intonix-community:mainfrom
fzakaria:add-recursive-calculate
Open

Add support for recursive search#129
fzakaria wants to merge 4 commits intonix-community:mainfrom
fzakaria:add-recursive-calculate

Conversation

@fzakaria
Copy link
Copy Markdown

The Nix, Lix & Snix (twix) codebase both support a recursive call for the lookup of the input derivation replacement paths.

Augment hashes.go to include the ability to recursively lookup derivations and calculate their paths.

A demonstration of this is done in the test but it can also be done by looking up derivations on the /nix/store filesystem for instance.

The Nix, Lix & Snix (twix) codebase both support a recursive call
for the lookup of the input derivation replacement paths.

Augment hashes.go to include the ability to recursively lookup
derivations and calculate their paths.

A demonstration of this is done in the test but it can also be done
by looking up derivations on the /nix/store filesystem for instance.
Comment thread pkg/derivation/hashes.go
@@ -141,6 +141,18 @@ func (d *Derivation) CalculateOutputPaths(inputDrvReplacements map[string]string
// We solve this having calculateDrvReplacement accept a map of
// /its/ replacements, instead of recursing.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The diff is a bit hard to read, but this docstring does explicitly say we expect a (immutabe) hashmap with the hashes already precomputed. Doing a call to CalculateDrvReplacementRecursive inside then seems wrong.

Copy link
Copy Markdown
Member

@flokli flokli left a comment

Choose a reason for hiding this comment

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

It's been a while since I touched this code, but I'd feel more comfortable if the code would be moved around the bit, so CalculateDrvReplacement does not call CalculateDrvReplacementRecursive.

@fzakaria
Copy link
Copy Markdown
Author

@flokli up to you on the change.
I modeled it similarly to what I saw in Snix & CppNix which need recursive calls to follow the input derivations to get the replacements.

I thought that the original code can be replaced with the recursive variant with a fixed function provider was a good demonstration of the interface ,😅🫠
(Writing from phone)

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