2024-02-12 Nix team meeting minutes #123

thufschmitt · February 14, 2024, 1:07pm

Attendees: @Ericson2314 @edolstra @roberth @tomberek @pennae @thufschmitt

Continous benchmarks

Continuous benchmarks

opened 06:47AM - 09 Jun 21 UTC

**Is your feature request related to a problem? Please describe.** The Nix ev…aluator is very performance-sensitive (getting in the hot path in a lot of use-cases), however it’s never tracked, leading to issues such as https://github.com/NixOS/nix/issues/4847 (and making changes like https://github.com/NixOS/nix/pull/4511 more painful because there’s no standard way to evaluate the performance changes). **Describe the solution you'd like** In my ideal dreamed world: 1. A standard set of benchmarks that could be run with `make bench` and `nix build .#benchmarks` 2. A CI job to run them on a regular basis 3. A nice dashboard reporting the results of the benchmarks through the git history of the project so that we can quickly track performance regressions

Restore `builtins.pathExists` behavior on broken symlinks by alois31 · Pull Request #9985 · NixOS/nix · GitHub

github.com/NixOS/nix

Comment by thufschmitt to Restore `builtins.pathExists` behavior on broken symlinks

NixOS:master ← alois31:symlink-resolution

Discussed during the Nix maintainers meeting on 2024-02-12. @ericson2314 will h…elp move this forward (well, he already started). <details> - Looks good, just needs someone to check the code. - Question related to https://github.com/NixOS/nix/issues/9585 - Should we keep the historical behavior (as reported by the issue) or the 2.20 one? - Actually, #9585 is a regression on 2.18, prior versions did return `true` on the example. - Conclusion: The current behavior is the right one, keep it - Assigned to @ericson2314 https://github.com/NixOS/nix/pull/9985#issuecomment-1938674380 https://github.com/NixOS/nix/issues/9585#issuecomment-1938673243 </details>

deduplicate inherit-from source expr work by pennae · Pull Request #9847 · NixOS/nix · GitHub

parser rewrite by pennae · Pull Request #9989 · NixOS/nix · GitHub

Confirmed something we want. Still blocked on other stuff

improve error reports somewhat by pennae · Pull Request #9874 · NixOS/nix · GitHub

Assigned to @tomberek for review

Add sandboxed building for FreeBSD using jails by rhelmot · Pull Request #9968 · NixOS/nix · GitHub

github.com/NixOS/nix

Comment by Ericson2314 to Add sandboxed building for FreeBSD using jails

NixOS:master ← rhelmot:freebsd

Discussed in the Nix team meeting. Conclusions: - Assigned to @ericson2314 to… follow-up - Can land the cleanups in a preperatory PR details: <details> Contributor (@rhelmot) interested in adding a FreeBSD sandbox, willing to refactor the sandbox code to enable that - @edolstra: The refactoring is welcome, but we don't have the capacity to maintain the FreeBSD code - @roberth: (maintain it ourselves) - @rhelmot also revived the FreeBSD stdenv in Nixpkgs (https://github.com/NixOS/nixpkgs/pull/254801) - @roberth: Would be great if we could make them code owner or similar - @edolstra: Mac bugs are painful because foreign to most maintainers and block features - @roberth: FreeBSD might be easier than MacOS for that (and also uncover the same bugs, making MacOS less of a trouble) - Proposal (@thufschmitt): If the refactorings allow having the FreeBSD sandbox non-intrusive, then we can accept it without commitments on maintaining it ourselves (like we already do for the broader FreeBSD support). - Agreement - @ericson2314: In particular, we can merge the preperatory cleanups before we've decided on the FreeBSD-specific code. Assigned to @ericson2314 to follow-up </details> Extra note from me: hope jumping through these hoops is not too disheartening @rhelmot. You've done very good work here and in Nixpkgs on this stuff! I look forward to working with you on getting it all merged.

Getting more maintainers

@thufschmitt: People like @pennae or @999years are very prolific and high-quality contibutors and could be “raised” into maintainers
@edolstra: No objection to people reviewing things, but merging them should be the responsibility of the maintainers team
@pennae: Things reviewed by others should be merged then
@ericson2314: Out of capacity for reviews. Need to grow the team
@edolstra: Not in favor of growing the team atm. Adding maintainers who can review things sounds good though.
@roberth other strategies
- Getting funding to allow the maintainers to spend more time on it
- Improving the process
- Specialization of maintainer responsibility / ownership
Proposal (@thufschmitt): Formally set a group of people that we trust for reviewing stuff.
- Codeowners would be nice, but requires commit access
- First step: Add people to the nix-issue-triage GH team and trust their reviews
  - Not perfect because won’t allow running the CI for new contributors or making approvals show-up as approving the PR, but first step
- Can add @pennae and @9999years at least
- Agreement

CLI stabilization

`nix path-info`

Previous proposal: Split the output into sections, but no agreement on what the sections should contain.

Minimum set of fields that stay the same on copy (not sure if we want to promise this, but seem to follow the rule) (same set as exportReferencesGraph?)
narHash
narSize
path
references
ca
Group metadata about the build together, e.g. ?
build.deriver
build.time (TBD)
Store-specific state?
store.registrationTime
store.lastVerifiedTime (TBD)
store.signatures
store.ultimate

@edolstra: Not that many fields that we need to namespace them
@pennae: Namespacing doesn’t hurt, why not just do it?
@edolstra: Would break backwards-compat. Can be fine, but needs to be justified

`nix-store --query` alternative

@tomberek: How to do: Haskell for all: How to correctly cache build-time dependencies using Nix
- Have a recommended way to provide the information commonly needed and served by “–query”
  - runtime closure + sizes
  - buildtime closure
  - runtime of the build closure (–include-outputs)
    - either intersected with current store state or not
  - referrers
  - …
possible query language for full-generality?
- would have to be similar to a graph database ql
- may not perform well on stores such as binary caches
  - though we do have a narinfo cache
Proposal (@thufschmitt):
- Align the json and no-json outputs to always display all the (non-computed) output fields
- Have a --recursive flag to work on the whole closure
- Rough agreement

What about the reverse closure?

Not part of nix path-info atm. But we’ll probably want it in the new CLI
@roberth: From experience, virtually always used to debug, for delete / gc
Can leave it out for now, if people want it in the new CLI, they can add a feature-request for it

Relationship to nix why-depends?

nix why-depends is a porcelain command, building on top of the same features as path-info, but different from it
@Ericson2314: I don’t really like the ValidPathInfo data type, which is the main purpose of this command. I also don’t like the path-info name, because the information is about the store object not the mere path that refers to it.

This actually works out nicely in a way. We seem to agree this command doesn’t sit perfectly with us, and the name doesn’t sit perfectly with me, so if we stabilze this as is, we’re in effect “saving a better name” (that we have yet to pick) for a better command we have yet to invent.

This is nicer than having the “bad command” steal the “best name”

For the future: Flesh out issues for contributors to work on

Atemu · February 14, 2024, 2:09pm

Wow, so many exciting improvements happening! As someone who doesn’t follow Nix development closely, thank you very much for the digest

imincik · February 14, 2024, 4:53pm

Yes, thanks a lot for the notes. I am always interested to hear what the Nix team does.