Skip to content

Conversation

@fzakaria
Copy link

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.
// the calculation would be recursive.
//
// We solve this having calculateDrvReplacement accept a map of
// /its/ replacements, instead of recursing.
Copy link
Collaborator

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
Collaborator

@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
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