Add benchmark support in shellFor #102323
Conversation
Benchmark dependencies are not included by shellFor unless we pass `doBenchmark = true` when generating the derivation. This commit passes doBenchmark argument from shellFor to the derivation so that benchmark dependencies get included.
|
@harendra-kumar This seems reasonable. Do you have some sort of example to prove that this is working as expected? |
|
@cdepillabout yes, I have been using it for a while to create a nix-shell for streamly for running the benchmarks, see this PR for the nix expression in use. My initial solution was more complex, but I gave it some soak time and it emerged to be simpler. |
There was a problem hiding this comment.
I've stared at this shellFor code a while, and I've come to the conclusion that I don't think the change in this PR is the proper solution.
I think instead what you need to do is enable doBenchmark on the packages you want benchmark dependencies for. In the example in the comment on the shellFor function, it would be p.frontend, p.backend, and p.common.
Is there some reason this isn't working?
|
It does not work. I just tried it and pushed it in this commit composewell/streamly@890d966 . You can pull the repo, change branch to "nix" and try the instructions in the commit message to give it a try. |
|
It took me quite a few hours, but I finally picked apart @harendra-kumar It turns out you were right. I commented about fixing this in #102598 (comment). |
|
@cdepillabout thanks for working on this. I am closing this in favor of #102683 . |
Motivation for this change
Benchmark dependencies are not included by shellFor unless we pass
doBenchmark = truewhen generating the derivation. This commitpasses doBenchmark argument from shellFor to the derivation so that
benchmark dependencies get included.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)