Skip to content

nixfmt-classic: add (eventual) deprecation and removal#447790

Merged
MattSturgeon merged 2 commits intoNixOS:masterfrom
jfly:deprecate-nixfmt-classic
Oct 6, 2025
Merged

nixfmt-classic: add (eventual) deprecation and removal#447790
MattSturgeon merged 2 commits intoNixOS:masterfrom
jfly:deprecate-nixfmt-classic

Conversation

@jfly
Copy link
Contributor

@jfly jfly commented Oct 1, 2025

This implements steps 1 and 2 of our nixfmt-classic deprecation plan: NixOS/nixfmt#340.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@jfly jfly requested a review from a team October 1, 2025 17:35
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 1, 2025
@jfly
Copy link
Contributor Author

jfly commented Oct 1, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 447790
Commit: 93627b2595ca3f6a8b003d0344ef5c277251d98e

@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Oct 1, 2025
else
lib.id
)
haskellPackages.nixfmt.bin;
Copy link
Contributor

@MattSturgeon MattSturgeon Oct 1, 2025

Choose a reason for hiding this comment

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

I wonder if we should be doing the deprecation (i.e. step 2) at the underlying haskellPackages.nixfmt package? Since the top-level nixfmt-classic is already just an alias for it...

I can't see if/how/where the haskellPackages set handles aliases and deprecation, though... Nothing obvious in the entrypoint. Maybe @NixOS/haskell will know?

Whatever we do should be gated behind config.allowAliases, of course.

Copy link
Member

Choose a reason for hiding this comment

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

Needs to be implemented manually, you can grep for allowAliases. Note that package removal is currently not possible (in a sensible way), but will probably be implemented sometime this autumn (cc @wolfgangwalther).

Copy link
Contributor

Choose a reason for hiding this comment

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

I spoke to @wolfgangwalther async:

haskell packages are auto-generated. You can't add aliases. You can only do this at the source once #441204 is done, by removing the package from haskellPackages. For now you need to alias it on the top-level only.

So I believe the correct thing to do is merge this as-is, then move the deprecation into the haskellPackages scope if/when that package-set adds support for deprecating specific packages.

Naïvely, we could add an overlay to the end of haskellPackages's extensions stack, and have that overlay implement the deprecations:

(final: prev: lib.optionalAttrs final.config.allowAliases {
  nixfmt =
    (
      if lib.oldestSupportedReleaseIsAtLeast 2605 then
        throw "haskellPackages.nixfmt has been removed as it is deprecated and unmaintained."
      else if lib.oldestSupportedReleaseIsAtLeast 2511 then
        lib.warnOnInstantiate "haskellPackages.nixfmt is deprecated and unmaintained. We recommend switching to nixfmt."
      else
        lib.id
    )
      prev.nixfmt;
})

However, this means that the package will still be present without any deprecation when allowAliases = false, which is unusual. That's because the package is automatically generated from hackage, so we can't move its base definition to a conditional extension.

@nixpkgs-ci nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Oct 2, 2025
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

Moving the deprecation to the underlying haskellPackages scope seems non-trivial, so I believe merging this as-is is correct for now.

Thanks @jfly

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 2, 2025
@jfly
Copy link
Contributor Author

jfly commented Oct 6, 2025

Thanks, @MattSturgeon! Could you merge this?

@MattSturgeon MattSturgeon added this pull request to the merge queue Oct 6, 2025
@MattSturgeon
Copy link
Contributor

Could you merge this?

Yep. I didn't want to merge immediately in case anyone else had additional feedback.

Merged via the queue into NixOS:master with commit ce47eed Oct 6, 2025
33 of 35 checks passed
@jfly jfly deleted the deprecate-nixfmt-classic branch October 18, 2025 21:28
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: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants