bonfire: init at 1.0.7-beta.4 - #550073
Conversation
9f17db8 to
2b5c754
Compare
![nixpkgs-branch-check[bot]](https://avatars.githubusercontent.com/u/487568?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
nixpkgs-ci
Bot
added
8.has: package (new)
|
Nixpkgs' CI currently fails with:
But I have no problem running this locally: nix-build ci --arg nixpkgs "$PWD" -A eval.singleSystem --argstr evalSystem x86_64-linux --arg chunkSize 8000 --argstr nixPath nixVersions.latest --out-link mergedCould it be that chunkSize is too high? Here's what in CI's log:
|
|
@NixOS/nixpkgs-ci could you please help me to understand/overcome that problem? Thanks |
|
I can reproduce this locally with I don't think it's chunkSize, I think that it's the truly massive number of instantiations (258,000) that building even just The total number of instantiations required to build just those three packages is 1,100,126. For context, with chunkSize 5000, the entire 2nd chunk has 118,163 instantiations. |
|
@mdaniels5757, thanks for your help. I reproduced the problem, first with:
where 3455 is the index of Here are the memory profiles (using This is under the 16GB recommended in the manual: Line 29 in d7d18d2 But when they get in the same chunk, and get evaluated at the very same time which is likely because they take more than a minute, their memory consumptions add up, here So I gave a try to #!/usr/bin/env nix
#! nix shell --impure --expr ``
#! nix let nixpkgs = builtins.getFlake "nixpkgs"; in
#! nix let pkgs = import nixpkgs {}; in
#! nix [
#! nix pkgs.inferno
#! nix pkgs.python3Packages.memory-profiler
#! nix ]
#! nix ``
#! nix --command bash
set -eux
drv=${1-bonfire-ember}
id=$(date +%s)
export NIX_COUNT_CALLS=1
export NIX_SHOW_STATS=1
export NIX_SHOW_STATS_PATH="prof/stats.$id.json"
mkdir -p prof
mprof run \
--output "prof/$drv.mprof.$id.dat" \
nix eval -f. "$drv" \
--eval-system x86_64-linux \
--option allow-import-from-derivation false \
--eval-profiler flamegraph \
--eval-profiler-frequency 10 \
--eval-profile-file "prof/$drv.nix.$id.eval"
svg=~/Downloads/"$drv.nix.$id.eval.svg"
inferno-flamegraph >"$svg" \
--minwidth 0.5 \
-i "prof/$drv.nix.$id.eval" \
--flamechart
mprof plot -t "$drv" "prof/$drv.mprof.$id.dat" \
--output "prof/$drv.mprof.$id.png"
geeqie "prof/$drv.mprof.$id.png" &
firefox "$svg" &Here's the output of Those dependencies had all in common to use |
Review dismissed automatically
nixpkgs-ci
Bot
added
6.topic: nixos
| in | ||
| beamPkgs.mixRelease ( | ||
| let | ||
| # ToDo(maintenance): use proper `finalAttrs` once `mixRelease` supports it. |
There was a problem hiding this comment.
This is merged, so should be reflected here.
|
@ofborg test bonfire |
|
sigh, I can't print the derivation from ofborg to see why it finds a different |
There was a problem hiding this comment.
I still say this is an anti pattern and a misconfiguration. Did you ever pursue configuring the deps so they write to somewhere else instead?
There was a problem hiding this comment.
in bonfire-setup, config/current_flavour/assets/ points to extensions/social/assets/ (when using social flavour), and surface_form_helpers cleans and generates into config/current_flavour/assets/hooks/, so maybe something could be done there, but I've not tried anything yet. Thanks for the idea.
I know you've put a bunch of work into this, but if you're not a user of this complex stack then who is? Why are they not the ones adding this package? Who will maintain it moving forward if they have no vested interest in it being in nixpkgs? There are layers upon layers of workarounds here, and it seems incredibly fragile to me. When the next update breaks who will fix it? What about the one after that? I understand the other repo was archived, but that doesn't necessarily mean nixpkgs is the best place for it if there are no users. As an aside, you're in the github ngi team, but not in the team-list.nix file. At first I thought you were submitting this for other people to maintain. The workarounds for deps_nix pulling in a completely different toolchain are frankly gross. I would prefer to see a fix submitted upstream to deps_nix to (optionally?) make the generated file acceptable in nixpkgs. This would be more valuable than hacking around the code we're committing here in nixpkgs. Have you pursued this? Do we really need to vendor all these Cargo.lock files? I understand the lumis situation and have worked with upstream to resolve that, but the latest release isn't quite right. For the others though, can we not use cargoHashes instead? Back to maintainability, I think this is bordering on or already is unmaintainable. While there are downsides to using mixFodDeps, a large portion of this PR would disappear if that was used instead, and would set future maintainers up for success. I fear this sets them up for failure and I am worried this will rot in nixpkgs accordingly. As someone who frequently builds all elixir/erlang packages, I'm concerned about what you've submitted here. |
`mix compile.surface` or `Fine.include_dir()` use `module_info/1` to get source files of BEAM dependencies. To work this requires: 1. `erlangDeterministicBuilds` set to `false` to preserve the build directory in `.beam` files. 2. to build dependencies in a directory persisting across nix builds: `$out/src` instead of `/build/source`. Issue: surface-ui/surface#762
nixpkgs-ci
Bot
added
10.rebuild-darwin: 1-10
phanirithvij
added
the
6.topic: NLNet / NGI
|
@adamcstephens, thanks for voicing your concerns, I hope I've addressed them all to your satisfaction below. Otherwise please let me know.
Unfortunately yes, that was a totally unexpected journey of 250h spanning 10 months,
For extra context, AFAIK, the overwhelming majority of software Though likely not as popular as mastodon, ejabberd, mobilizon, or even pleroma or akkoma, Bonfire's main developers have been helpful and responsive when I pinpointed any issue:
Maybe because it was that hard to package it.
Good chance is that it will remain the Nix@NGI team, For the record, this latest update of Bonfire (
Investigating why the packaging part broke is expectedly the hardest part,
Yes, and there were even more workarounds in previous versions And yes, the humonguous size of Bonfire's closure Workarounds are documented so that when a similar problem happens somewhere else I guess updating the package more regularly (Bonfire has new release candidates almost every week)
My current contract with Nix@NGI runs up to December 2026.
I can only speculate that it will remain me for the time being.
For sure, I can't speak for the Nix@NGI team, but as far as I'm concerned,
I'm sorry for the confusion, though I work for them I am not officially part of the Nix@NGI team
I opened an issue 8 months ago: It used to be a onliner
Good to know, and thanks for that.
I guess this could be achieved using the relevant
And so would reproducibility and whatever trust one can have in the resulting software
Yet But I do concede that caching at the package level (ie.
I understand, and you made your point very clear that this PR is too daunting Within the Nix@NGI team I pleaded in favor of properly packaging complex packages like Bonfire I guess Bonfire's devs will agree to have this PR retargeted Thanks for your review, |
|
Thank you @ju1m for your detailed response. In hindsight I realize my comment was more confrontational than I prefer, so I apologize if I came off too harsh. I've followed bonfire for a while and agree it would be cool to have nix packaging support for it.
It actually doesn't always have to be this way, and one of my personal goals is to shine light on this. See code-supply/deps_nix#50 for how deps_nix can be used to change this default. Yes, some deps (including rustler ones) rely on compile-time config, but much of the elixir ecosystem does not. In my personal projects I've been switching to defaulting no-app-config and only selectively enabling those that need it through overrides. I'd like these to be the exceptions, not the standard, and nixpkgs+deps_nix tooling should support this. I'm open to ideas for how we can improve this further. For example, one idea that just came to mind, can we create a simple
Thanks, I hadn't seen this. I've admittedly not looked too deeply into the rustler workarounds in deps_nix. Perhaps I should.
I'd actually strongly recommend this, and do so leveraging the updateScript. This will prove the script works as needed, and will allow you to feel more confident in the longer term. A release could bring other breaking changes, but at least you can be confident in the core of the updating scripts.
Personally, I'd be worried that much of my concerns would apply upstream as well. I've seen too many projects where a nix user contributed nix bits, only for them to languish and become broken. Doubly so with the complexity of this packaging. I don't mean to run you away from nixpkgs, or discourage you from continuing here. I'd feel a lot better if there was at least one committed user of this work. Without that, and with a limited timespan of paid maintenance, I'm worried this will end up like so many other packages in nixpkgs that don't have active users/maintainers. That said, I am but one committer amongst many and others can disagree and proceed with or without my support. Thanks again for hearing me out and thinking through these concerns. |
| # Warning(maint/update): bonfire having a huge dependency closure, | ||
| # expect a lot of downloads during several minutes. | ||
| # Besides, Bonfire takes about one hour to update | ||
| # because `deps_nix` somehow triggers some compiling when updating. |
There was a problem hiding this comment.
code-supply/deps_nix#51 can avoid deps_nix needing to compile a behavior. By extracting deps_nix itself from the project, my testing shows that it doesn't need to compile anything.
| # and load_from because rustler defaults to priv/native/#{crate} | ||
| # but deps_nix installs into priv/native/lib#{crate}. | ||
| # | ||
| # Issue: https://github.com/code-supply/deps_nix/issues/36 |
There was a problem hiding this comment.
This issue is resolved, so the code should be updated here.
| ); | ||
|
|
||
| beamPackages = beamPkgs // { | ||
| buildMix = |
There was a problem hiding this comment.
I would strongly recommend using per-dep overrides in mixNixDeps to fix these issues rather than overriding buildMix itself globally.
Also, post #554005 this should be done with overrideScope. But again, I'd prefer to not see this done at all.
| # at least :rustler_precompiled and :mime | ||
| # Issue: https://github.com/bonfire-networks/bonfire-app/issues/1696 |
There was a problem hiding this comment.
Is this still an issue to FixMe? The upstream issue was closed as possibly resolved.
There was a problem hiding this comment.
To avoid Nixpkgs' CI complain about missing enlines in .hash files.
There was a problem hiding this comment.
Unfortunately required to satisfy Nixpkgs' CI which is not aware that it's not used even though it's referenced in generated deps.nix
| in | ||
| beamPkgs.mixRelease ( | ||
| let | ||
| # ToDo(maintenance): use proper `finalAttrs` once `mixRelease` supports it. |
There was a problem hiding this comment.
This is merged, so should be reflected here.
| This package only exists to provide a location for an `updateScript` | ||
| updating `src` just once before calling each extensions' `update.script`. |
There was a problem hiding this comment.
It seems awkward to me to create a stub package like this. Is this just because of the limitations of the by-name structure? Is this really more of a package set, and should be treated differently?
| let | ||
| bonfire = generic { | ||
| inherit bonfire; | ||
| FLAVOUR = extension; |
There was a problem hiding this comment.
Should we not refer to these as flavours then, given that's what upstream calls them? Their use of the term extension seems to be much more granular.
h7x4
added
8.has: module (new)
6 participants
This PR migrates Bonfire from NGIpkgs (now archived) to Nixpkgs.
It's resurrecting ngi-nix/ngipkgs#2155
I'm not using Bonfire myself, so I've only tested it with
nixosTests.bonfire.Stacked upon #506184
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Acknowledgments