Skip to content

nix-direnv: use nix (2.4) and remove enableFlakes#145236

Merged
lovesegfault merged 1 commit intoNixOS:masterfrom
lovesegfault:nix-direnv-stable-nix
Nov 10, 2021
Merged

nix-direnv: use nix (2.4) and remove enableFlakes#145236
lovesegfault merged 1 commit intoNixOS:masterfrom
lovesegfault:nix-direnv-stable-nix

Conversation

@lovesegfault
Copy link
Copy Markdown
Member

Follow-up to #144197 since nixUnstable is no longer needed for flake
support.

Motivation for this change
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.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.

@roberth
Copy link
Copy Markdown
Member

roberth commented Nov 9, 2021

Maybe nix shouldn't be baked into this package at all; couldn't it just use the environment's Nix? That would make it easier for users to upgrade their nix to whichever version works best for them, considering that flakes are still in development.

@lovesegfault lovesegfault force-pushed the nix-direnv-stable-nix branch from dc6fdfc to 6c1b98e Compare November 9, 2021 19:40
@lovesegfault
Copy link
Copy Markdown
Member Author

Maybe nix shouldn't be baked into this package at all; couldn't it just use the environment's Nix? That would make it easier for users to upgrade their nix to whichever version works best for them, considering that flakes are still in development.

Maybe? @Mic92 probably knows best here.

Comment thread pkgs/tools/misc/nix-direnv/default.nix Outdated
@Mic92
Copy link
Copy Markdown
Member

Mic92 commented Nov 9, 2021

Maybe nix shouldn't be baked into this package at all; couldn't it just use the environment's Nix? That would make it easier for users to upgrade their nix to whichever version works best for them, considering that flakes are still in development.

If someone does not want to use flakes, they don't have to. use flake is independent from use nix.

@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 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 Nov 9, 2021
lovesegfault added a commit to lovesegfault/home-manager that referenced this pull request Nov 9, 2021
Since NixOS/nixpkgs#144197 flake support is
always available. The upstream `enableFlakes` option is scheduled to be
removed altogether in NixOS/nixpkgs#145236
@roberth
Copy link
Copy Markdown
Member

roberth commented Nov 9, 2021

Maybe nix shouldn't be baked into this package at all; couldn't it just use the environment's Nix? That would make it easier for users to upgrade their nix to whichever version works best for them, considering that flakes are still in development.

If someone does not want to use flakes, they don't have to. use flake is independent from use nix.

True, but not what I was trying to say. I think some people will want to upgrade from stable nix (which also supports flakes) to nixUnstable which will contain more fixes.
With the current setup, this requires users to set nix in an overlay, which seems unnecessary.
Most tools don't pin Nix, so I wonder if this one needs to.

Follow-up to NixOS#144197 since nixUnstable is no longer needed for flake
support.
berbiche pushed a commit to nix-community/home-manager that referenced this pull request Nov 10, 2021
Since NixOS/nixpkgs#144197 flake support is
always available. The upstream `enableFlakes` option is scheduled to be
removed altogether in NixOS/nixpkgs#145236
@Mic92
Copy link
Copy Markdown
Member

Mic92 commented Nov 10, 2021

Maybe nix shouldn't be baked into this package at all; couldn't it just use the environment's Nix? That would make it easier for users to upgrade their nix to whichever version works best for them, considering that flakes are still in development.

If someone does not want to use flakes, they don't have to. use flake is independent from use nix.

True, but not what I was trying to say. I think some people will want to upgrade from stable nix (which also supports flakes) to nixUnstable which will contain more fixes. With the current setup, this requires users to set nix in an overlay, which seems unnecessary. Most tools don't pin Nix, so I wonder if this one needs to.

I personally don't care if it is pinned or not. I introduced this hard-coding based on feedback with other people: nix-community/nix-direnv#52 cc @anka-213 @hmenke

@lovesegfault lovesegfault merged commit 754e2a5 into NixOS:master Nov 10, 2021
@lovesegfault lovesegfault deleted the nix-direnv-stable-nix branch November 10, 2021 06:44
@roberth
Copy link
Copy Markdown
Member

roberth commented Nov 10, 2021

I personally don't care if it is pinned or not.

I do care for the poor soul who upgrades their Nix and stuff still doesn't work. Sadly, I don't have infinite time.

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

Labels

8.has: clean-up This PR removes packages or removes other cruft 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants