Skip to content

formats: deprecate accidental lib.types aliases#415666

Merged
philiptaron merged 1 commit intoNixOS:masterfrom
MattSturgeon:pkgs/formats/no-inherit-types
Jun 10, 2025
Merged

formats: deprecate accidental lib.types aliases#415666
philiptaron merged 1 commit intoNixOS:masterfrom
MattSturgeon:pkgs/formats/no-inherit-types

Conversation

@MattSturgeon
Copy link
Contributor

@MattSturgeon MattSturgeon commented Jun 10, 2025

Introduced in #335232 (commit 1f6ce17), see discussion: #335232 (comment)

cc @philiptaron

Things done

Tested using the repl:

$ nix repl
Nix 2.28.3
Type :? for help.

nix-repl> pkgs = import ./. { }

nix-repl> pkgsNoAliases = import ./. { config.allowAliases = false; }

nix-repl> pkgs.formats.attrsOf
evaluation warning: `formats.attrsOf' is deprcated, use `lib.types.attrsOf' instead.
«lambda attrsOf @ /home/matt/nixpkgs/pr/lib/types.nix:752:17»

nix-repl> pkgsNoAliases.formats.attrsOf
error:
       … while evaluating the attribute 'formats.attrsOf'
         at /home/matt/nixpkgs/pr/pkgs/top-level/all-packages.nix:996:5:
          995|     })
          996|     formats
             |     ^
          997|     ;

       error: attribute 'attrsOf' missing
       at «string»:1:1:
            1| pkgsNoAliases.formats.attrsOf
             | ^
  • 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/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@MattSturgeon MattSturgeon force-pushed the pkgs/formats/no-inherit-types branch from 2f752f0 to 88536a4 Compare June 10, 2025 19:39
@nix-owners nix-owners bot requested review from Stunkymonkey and h7x4 June 10, 2025 19:42
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jun 10, 2025
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went looking to see how formats is used.

$ rg -wo 'formats\.\w+' --no-filename | sort | uniq -c | sort -rh
    129 formats.yaml
    124 formats.json
     97 formats.toml
     89 formats.ini
     20 formats.keyValue
     14 formats.iniWithGlobalSection
     11 formats.hocon
     11 formats.elixirConf
     10 formats.text
     10 formats.dataclass
      9 formats.javaProperties
      7 formats.php
      7 formats.libconfig
      6 formats.nix
      5 formats.c
      4 formats.asn1
      4 formats.add_parser
      3 formats.xml
      3 formats.pythonVars
      3 formats.lua
      3 formats.cdn
      2 formats.systemd
      2 formats.nrbf
      1 formats.yaml_1_1
      1 formats.tone_stream
      1 formats.sndfile
      1 formats.png
      1 formats.passthru
      1 formats.native_file
      1 formats.local_stream
      1 formats.gitIni

We should deprecate every item that was accidentally added.

@MattSturgeon
Copy link
Contributor Author

MattSturgeon commented Jun 10, 2025

We should deprecate every item that was accidentally added.

Looking at #166044, javaProperties is intentional.

Testing locally, it seems that all "unused" attrs are deprecated by this PR:

Details

$ nix repl
Nix 2.28.3
Type :? for help.
nix-repl> pkgs = import ./. {config.allowAliases = false;}

nix-repl> used = {
              yaml = null;
              json = null;
              toml = null;
              ini = null;
              keyValue = null;
              iniWithGlobalSection = null;
              hocon = null;
              elixirConf = null;
              text = null;
              dataclass = null;
              javaProperties = null;
              php = null;
              libconfig = null;
              nix = null;
              c = null;
              asn1 = null;
              add_parser = null;
              xml = null;
              pythonVars = null;
              lua = null;
              cdn = null;
              systemd = null;
              nrbf = null;
              yaml_1_1 = null;
              tone_stream = null;
              sndfile = null;
              png = null;
              passthru = null;
              native_file = null;
              local_stream = null;
              gitIni = null;
          }

nix-repl> fmt = pkgs.formats

nix-repl> builtins.intersectAttrs used fmt
{
  cdn = «lambda cdn @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:462:5»;
  elixirConf = «lambda elixirConf @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:534:5»;
  gitIni = «lambda gitIni @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:316:11»;
  hocon = «lambda format @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats/hocon/default.nix:35:5»;
  ini = «lambda ini @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:221:11»;
  iniWithGlobalSection = «lambda iniWithGlobalSection @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:260:11»;
  javaProperties = «lambda javaProperties @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats/java-properties/default.nix:17:5»;
  json = «lambda json @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:89:5»;
  keyValue = «lambda keyValue @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:355:5»;
  libconfig = «lambda format @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats/libconfig/default.nix:30:5»;
  lua = «lambda lua @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:737:5»;
  php = «lambda format @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats/php/default.nix:5:5»;
  pythonVars = «lambda pythonVars @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:799:5»;
  systemd = { ... };
  toml = «lambda toml @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:413:5»;
  xml = «lambda xml @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:856:5»;
  yaml = «lambda yaml_1_1 @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:131:5»;
  yaml_1_1 = «lambda yaml_1_1 @ /home/matt/nixpkgs/pr/pkgs/pkgs-lib/formats.nix:131:5»;
}

nix-repl> builtins.removeAttrs fmt (builtins.attrNames used)
{ }

Concerningly, some of the matches you had do not actually exist, although that's unrelated to this PR:

nix-repl> builtins.removeAttrs used (builtins.attrNames fmt)
{
  add_parser = null;
  asn1 = null;
  c = null;
  dataclass = null;
  local_stream = null;
  native_file = null;
  nix = null;
  nrbf = null;
  passthru = null;
  png = null;
  sndfile = null;
  text = null;
  tone_stream = null;
}

Hopefully most of those are false-positives for your regex 🤞

@MattSturgeon MattSturgeon force-pushed the pkgs/formats/no-inherit-types branch from 88536a4 to feef690 Compare June 10, 2025 20:32
@philiptaron
Copy link
Contributor

Looking at #166044, javaProperties is intentional.

No, no, these are the ones that are used, not the ones that aren't used. Apologies for the confusion.

@MattSturgeon
Copy link
Contributor Author

Looking at #166044, javaProperties is intentional.

No, no, these are the ones that are used, not the ones that aren't used. Apologies for the confusion.

Gotcha. In that case I think this PR covers them all:

nix-repl> builtins.removeAttrs fmt (builtins.attrNames used)
{ }

@MattSturgeon
Copy link
Contributor Author

I'm assuming this is niche/internal enough that we don't need to add a release note?

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this mistake of mine.

@philiptaron philiptaron merged commit 5d24bc9 into NixOS:master Jun 10, 2025
15 of 18 checks passed
@MattSturgeon MattSturgeon deleted the pkgs/formats/no-inherit-types branch June 10, 2025 20:42
@philiptaron

This comment was marked as off-topic.

@philiptaron philiptaron mentioned this pull request Jun 10, 2025
13 tasks
@philiptaron
Copy link
Contributor

There's a CI failure about aliases here: #415662

😕

@philiptaron
Copy link
Contributor

#415719

@trofi
Copy link
Contributor

trofi commented Jun 11, 2025

Bisect says that this change broke the nixos-rebuild for me. An example nixpkgs test is nixosTests.man:

$ nix build --no-link -f. nixosTests.man -L
...
lazy-options.json>            42| in
lazy-options.json>            43| lib.optionalAttrs pkgs.config.allowAliases aliases
lazy-options.json>              |                   ^
lazy-options.json>            44| // rec {
lazy-options.json>
lazy-options.json>        error: expected a set but found null: null
lazy-options.json> Cacheable portion of option doc build failed.

#415719 proposed by Philip does fix the failure for me when applied to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants