lib: Fix #30902 #34444
lib: Fix #30902 #34444
Conversation
Ericson2314
added
2.status: work-in-progress
Ericson2314
force-pushed
the
meta-check
branch
2 times, most recently
from
d0de3fa
to
3573805
Compare
|
@GrahamcOfBorg eval |
|
@dtzWill lets land this first. Frankly, I'm not sure how you made it so far without it :). |
|
Haha, not sure! Will take a look shortly. |
|
I can eval everything locally, but it kills @GrahamcOfBorg so not sure what that's about. I hope its not just me making everything more expensive. |
|
The message is from Boehm GC, to be clear. |
|
I could be wrong but I think evaluating all of nixpkgs is beyond our Borg friend's limits currently. Large hydra evaluations specifically strip out a bunch of stuff to reduce memory, so it's not without precedent to find evaluating all the things a large task :). That said, it may be worth quickly testing time/memory of a large evaluation before and after this change -- regressing too badly on this will negatively impact almost everyone and probably summon an angry mob. But just be sure this doesn't 2x evaluation time or something :P. Borg may want to use a larger heap, which I think is the usual way to workaround that error. I forget the environment variable name O:). Not sure where previous discussion of this ended up, for example: #33747 (comment) Ideally of course we'd be constantly making evaluation faster using less memory :D. |
OfBorg's evaluators have 16G of RAM.
Agreed :) Especially on:
I think we want Can someone do a bit of trial and error on that? I can set the variable on the evaluators. |
|
@GrahamcOfBorg eval also this ^ I'll keep tabs on the eval in my monitoring. |
|
I forget if it helped but I'll just note that we do use a boehmgc that is /not/ tuned for "large" heaps where IIRC "large" is definitely smaller than what we use on most evals. https://github.com/ivmai/bdwgc/blob/b5cab5aacd263577af3502ecf068612591d27704/configure.ac#L840 Err nevermind this, until I look into it more (or someone else does), just wanted to mention while on the subject. |
|
This PR doesn't seem to have a substantial impact ... but I only looked at four evals in some fairly lousy data. |
|
Note it complains of hitting limit re:heap sections, which appears to be ~8GB: It "may" make sense to consider building against a boehmgc with large config enabled. |
|
@dtzWill before pinging Eelco, can you try it and see if it makes an improvement? :) |
Sure. But since he introduced the enableLargeConfig option (8e2e421) it seemed likely he had existing knowledge/experience/opinions/insights on the subject before investigating myself. Anyway, point taken I'll drop it until I have evidence it helps :). |
|
It seems master went over the line, so we must have been mighty close: @GrahamcOfBorg eval |
|
Success on aarch64-linux (full log) Partial log (click to expand)
|
Ericson2314
force-pushed
the
meta-check
branch
2 times, most recently
from
188a993
to
8d40e0d
Compare
… world First, we need check against the host platform, not the build platform. That's simple enough. Second, we move away from exahustive finite case analysis (i.e. exhaustively listing all platforms the package builds on). That only work in a closed-world setting, where we know all platforms we might build one. But with cross compilation, we may be building for arbitrary platforms, So we need fancier filters. This is the closed world to open world change. The solution is instead of having a list of systems (strings in the form "foo-bar"), we have a list of of systems or "patterns", i.e. attributes that partially match the output of the parsers in `lib.systems.parse`. The "check meta" logic treats the systems strings as an exact whitelist just as before, but treats the patterns as a fuzzy whitelist, intersecting the actual `hostPlatform` with the pattern and then checking for equality. (This is done using `matchAttrs`). The default convenience lists for `meta.platforms` are now changed to be lists of patterns (usually a single pattern) in `lib/systems/for-meta.nix` for maximum flexibility under this new system. Fixes NixOS#30902
`packagePlatforms` now filters `supportedSystems` with the new-style `meta.platforms`, rather than just plopping it in as is.
GrahamcOfBorg
added
10.rebuild-darwin: 0
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Ok, pre-fixed all the misbehaving packages so this is ready for final review! No more rebuilds anymore. |
|
Success on x86_64-darwin (full log) Attempted: stdenv Partial log (click to expand)
|
|
I need to do a PR on top of this, so I'm going to merge it. Feel free to revert if eval time problems arise, but this should be good in all other respects. |
|
Success on x86_64-darwin (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
@Ericson2314 Could you help updating this platforms line in |
|
One can fall back on the legacy stuff by using https://github.com/NixOS/nixpkgs/blob/master/lib/systems/doubles.nix#L45 . I think that's easiest for now. |
|
Thank you! We can replace |
|
Yeah. The new ones are https://github.com/NixOS/nixpkgs/blob/master/lib/systems/for-meta.nix made from https://github.com/NixOS/nixpkgs/blob/master/lib/systems/inspect.nix . There's still no negation but if you combine the x86 pattern with all the unix ones, you can get |
Motivation for this change
See the "lib, stdenv: Check..." commit for details.
Work in progress because I need to overhaul
release-lib.nix, and maybe other uses ofmeta.platforms.Fixes #30902
Things done
Need to ensure
release-lib.nixpackages still eval fine.build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)CC @bgamari @dtzWill