Detect cycles in flake follows. #9169
Conversation
This change results in an error thrown as opposed to segfaulting due to stack overflow. Fixes NixOS#9144
github-actions
bot
added
the
with-tests
src/libexpr/flake/lockfile.cc
Outdated
| auto pos = root; | ||
|
|
||
| auto pathS = printInputPath(path); |
There was a problem hiding this comment.
Do we need to eagerly print the paths? Couldn't we have visited be a std::vector<FlakeId> and only print the paths if we get a cycle?
(not that it matters too much from a performance pov, but it would make the intent clearer I think)
There was a problem hiding this comment.
I'm a bit rusty on c++(no pun intended :)), but:
I used a similar approach although using std::vector<InputPath>, then it turned out that InputPath is itself a vector, so to avoid the copy I decided to switch to std::vector<const InputPath&>, but apparently c++ does not like vectors of vector references, which have to be wrapped in std::reference_wrapper. So I decided to convert to path strings for visited.
Like you mention, even using vector<InputPath> will likely not matter in terms of performance, switched back to that.
There was a problem hiding this comment.
Oh indeed, we're copying too much any way. Well, so be it then, we'll live with that.
This change results in an error thrown as opposed to segfaulting due to stack overflow.
Fixes #9144
Motivation
Context
Priorities
Add 馃憤 to pull requests you find important.