[nix-cache] Prevent checking cache twice per package #2055
Conversation
internal/devpkg/narinfo_cache.go
Outdated
| sysInfo, err := p.sysInfoIfExists() | ||
| // let's be super safe to always avoid empty key. | ||
| if err == nil && sysInfo != nil && len(sysInfo.DefaultOutputs()) > 0 { | ||
| output = lo.Reduce( |
There was a problem hiding this comment.
Do the default outputs needed to be sorted to make the key stable?
There was a problem hiding this comment.
The implementation of DefaultOutputs is pretty stable, but it doesn't hurt.
internal/devpkg/narinfo_cache.go
Outdated
| output = lo.Reduce( | ||
| sysInfo.DefaultOutputs(), | ||
| func(acc string, o lock.Output, _ int) string { | ||
| return acc + o.Name | ||
| }, | ||
| "", | ||
| ) |
There was a problem hiding this comment.
Could the lo.Reduce be replaced by the following for easier-reading:
return strings.Join(sysInfo.DefaultOutputs(), ",")
?
It would also mean that the key matches the nix-installable-output format of <name>^<output>,<output>,...<output>. It doesn't really need to do this but is ... aesthetically pleasing :)
There was a problem hiding this comment.
outputs are not strings, but maybe if they have a String() method this will work? will try
There was a problem hiding this comment.
it doesn't work.
yeah, we call it once per output and once for So for a given package with a single (default) output |
Summary
Fixes bug described here #2054
A better (but a bit more involved) solution is to parallelize by output. Currently we parallelize by meta-output (which includes
__default_output__). This has two downsides:__default_output__can be multiple outputs, in which case we don't parallelize them at all.__default_output__is multiple outputs. (once as__default_output__and once for each individual outout that forms part of defaults).How was it tested?