Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

treewide: remove with lib in meta, will be dismembered later #371665

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lucasew
Copy link
Contributor

@lucasew lucasew commented Jan 7, 2025

Things done

Remove with lib in meta, proactively, in the whole repo, once and for all

Methodology

Basically, all packages with a with lib in meta will have meta = with lib; in it,
so I just replaced treewide this string to meta = in all **/*.nix.

This naive alteration would break a lot of stuff, like everything, so I had to fix the
broken stuff.

For this there is nil which is a code checker and also
implements the language server protocol. Then I did a simple bash for that iterates over
**/*.nix and applies the LSP to it discarding the output. Files with failure then are
listed and saved in a nil-failures.log. Why log? Log is in the gitignore so no chance
of causing a mess. At least from this part.

To start fixing what is basically thousands and thousands of files, I used substituteInPlace
with some common patterns, not overthinking too much to catch them all. Examples:

  • license = licenseslicense = lib.licenses
  • maintainers = with maintainersmaintainers = with lib.maintainers

And many others, basically following this same pattern of simple text substitution, no regex.

Between the substitutions, I ran that nil loop again to recalculate the failing files and then
decided the next replacement.

After about a dozen iterations, there remained about 800 files to be fixed. The following files
were not changed because of a false positive where __curPos is undefined, but it seemed like an
implementation detail:

  • lib/tests/modules/merge-module-with-key.nix
  • lib/tests/modules/test-mergeAttrDefinitionsWithPrio.nix
  • lib/tests/modules/types-attrTag.nix
  • nixos/modules/profiles/nix-builder-vm.nix
  • pkgs/applications/emulators/wine/sources.nix
  • pkgs/development/compilers/corretto/mk-corretto.nix
  • pkgs/top-level/release-cross.nix

By consequence, this PR also fixes issues where version is not defined in meta.changelog, which
in some cases I had to rewrite some mkDerivation using the finalAttrs form. There were rare cases,
about 10, probably.

Also by consequence, nil warned about unused recs and unused arguments, which I removed. Also, there were
unused let variables, which I didn't touch. A remarkable case was a arch something in some ROCm files.

There were some rare cases where passthru were defined after meta, then I moved it up.

TL;DR: this shouldn't have any rebuilds or eval issues.

Review tips

Download the full PR patch and look through the diff from there. Most of the time the diff will be boringly the same.

I'll squash the commits when this is about to get merged

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 7, 2025
@lucasew
Copy link
Contributor Author

lucasew commented Jan 7, 2025

Please, bot. Do not tag everyone.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 7, 2025
@lucasew lucasew force-pushed the 20250106-no-with-lib branch from 083ec14 to d737646 Compare January 7, 2025 03:27
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 7, 2025
@lucasew lucasew force-pushed the 20250106-no-with-lib branch from d737646 to 2d07585 Compare January 7, 2025 03:38
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 7, 2025
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: qt/kde 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: vim 6.topic: xfce The Xfce Desktop Environment 6.topic: nodejs 6.topic: hardware 6.topic: coq "A formal proof management system" 6.topic: pantheon The Pantheon desktop environment 6.topic: cinnamon Desktop environment 6.topic: docker tools 6.topic: R 6.topic: LXQt The Lightweight Qt Desktop Environment 6.topic: Lumina DE The Lumina Desktop Environment 6.topic: Enlightenment DE The Enlightenment Desktop Environment 6.topic: vscode 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 6.topic: xen-project The Xen Project hypervisor labels Jan 7, 2025
@lucasew lucasew mentioned this pull request Jan 7, 2025
13 tasks
@github-actions github-actions bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 7, 2025
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 7, 2025
@lucasew lucasew force-pushed the 20250106-no-with-lib branch from 6a8821e to deb4639 Compare January 7, 2025 13:27
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 7, 2025
@lucasew lucasew force-pushed the 20250106-no-with-lib branch from 557aa4d to 1a2d582 Compare January 7, 2025 15:40
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 7, 2025
@lucasew lucasew changed the title treewide: remove with lib in meta treewide: remove with lib in meta, will be dismembered later Jan 7, 2025
@lucasew lucasew force-pushed the 20250106-no-with-lib branch from 9035276 to b0f0f58 Compare January 7, 2025 18:06
@lucasew lucasew force-pushed the 20250106-no-with-lib branch from b0f0f58 to 1c880b4 Compare January 7, 2025 18:28
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 8, 2025
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 6.topic: printing 6.topic: nixos-container Imperative and declarative systemd-nspawn containers 6.topic: testing Tooling for automated testing of packages and modules 6.topic: jitsi 6.topic: systemd 6.topic: mate The MATE Desktop Environment 6.topic: flakes The experimental Nix feature 6.topic: lib The Nixpkgs function library and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Jan 8, 2025
Signed-off-by: lucasew <[email protected]>
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 8, 2025
@github-actions github-actions bot added 6.topic: module system About "NixOS" module system internals and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Jan 8, 2025
@Ma27
Copy link
Member

Ma27 commented Jan 8, 2025

Remove with lib in meta, proactively, in the whole repo, once and for all

What even is the problem?

My impression is that people religiously chase every single with lib they encounter even though some have its uses and we still don't have a better language construct.

Because to be clear, writing out lib.maintainers.x is just visual noise that I dislike far more than meta = with lib;, a usage where the with is still pretty constrained.

@lucasew
Copy link
Contributor Author

lucasew commented Jan 8, 2025

Because to be clear, writing out lib.maintainers.x is just visual noise that I dislike far more than meta = with lib;, a usage where the with is still pretty constrained.

with lib.maintainers; [ ... ] is fine. The problem is open scoped withs.

I removed some of those actually but I shouldn't. This PR will not get merged. It's just a starting point for a migration where someone brings a subset of it's diff and tweak stuff if necessary.

@lucasew
Copy link
Contributor Author

lucasew commented Jan 8, 2025

I am working on adding static checks on CI. This is me hyperfocusing too soon, and studying about the problem and it's different nuances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cinnamon Desktop environment 6.topic: coq "A formal proof management system" 6.topic: docker tools 6.topic: Enlightenment DE The Enlightenment Desktop Environment 6.topic: flakes The experimental Nix feature 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: hardware 6.topic: jitsi 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 6.topic: lib The Nixpkgs function library 6.topic: Lumina DE The Lumina Desktop Environment 6.topic: LXQt The Lightweight Qt Desktop Environment 6.topic: mate The MATE Desktop Environment 6.topic: module system About "NixOS" module system internals 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: nixos-container Imperative and declarative systemd-nspawn containers 6.topic: nodejs 6.topic: pantheon The Pantheon desktop environment 6.topic: printing 6.topic: qt/kde 6.topic: R 6.topic: systemd 6.topic: testing Tooling for automated testing of packages and modules 6.topic: vim 6.topic: vscode 6.topic: xen-project The Xen Project hypervisor 6.topic: xfce The Xfce Desktop Environment 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants