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

nixos/nix: add support for specifying additional nix paths #63201

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andir
Copy link
Member

@andir andir commented Jun 16, 2019

It is a common pitfall for NixOS users to declare some additional nix
search paths by setting nix.nixPath not noticing that they are
removing the default search paths.

This change introduces the option nix.extraNixPath which can be used
to add paths to the list of search paths without overriding the
defaults. Users most likely want to use this since it is less likely
they'll have to resort fiddling the NIX_PATH environment variable
themselves in order to repair their system.

/cc @lheckemann @flokli since we just helped a NixOS users at ZuriHack with recovering from such a situation.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.


This change is Reviewable

It is a common pitfall for NixOS users to declare some additional nix
search paths by setting `nix.nixPath` not noticing that they are
removing the default search paths.

This change introduces the option `nix.extraNixPath` which can be used
to add paths to the list of search paths without overriding the
defaults. Users most likely want to use this since it is less likely
they'll have to resort fiddling the `NIX_PATH` environment variable
themselvs in order to repair their system.
Copy link
Member

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

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

I don't like the solution, but the implementation seems good.

@andir
Copy link
Member Author

andir commented Jun 16, 2019

I don't like the solution, but the implementation seems good.

@lheckemann what would be a better solution?

@lheckemann
Copy link
Member

Replacing NIX_PATH with Something Better™ (Flakes?). I don't have any wonderful short-term ideas, unfortunately.

@andir
Copy link
Member Author

andir commented Jun 17, 2019

Replacing NIX_PATH with Something Better™ (Flakes?). I don't have any wonderful short-term ideas, unfortunately.

I don't see how flakes will be able to replace the include paths at all. It might work for custom package sources but not for things like passing extra inputs to your builds. The user we encountered on the weekend had to use it to pass some SSH configuration into the build process. Also being able to use <mypkgs> in any expression on my system can probably not be replaced by whatever.flakes { url= https://notgithub.com/foo/bar; branch = …} since it becomes very repetitive.

@jtojnar
Copy link
Contributor

jtojnar commented Jun 17, 2019

Do not we have some functor that would modify list merge rule?

E.g. option = mergeList [ foo ]; vs. option = mkForce xxx [ foo ]

@andir
Copy link
Member Author

andir commented Jun 17, 2019 via email

@jtojnar
Copy link
Contributor

jtojnar commented Jun 17, 2019

mkAfter does not seem to do anything. This, however, does:

{lib, ...}: {
  nix = {
    nixPath = lib.mkOptionDefault [
      "nixpkgs=/home/jtojnar/Projects/nixpkgs"
    ];
  };
}
$ nixos-option -I nixos-config=/etc/nixos/configuration.nix nix.nixPath
Value:
[ "nixpkgs=/nix/var/nix/profiles/per-user/root/channels/nixos" "nixos-config=/etc/nixos/configuration.nix" "/nix/var/nix/profiles/per-user/root/channels" "nixpkgs=/home/jtojnar/Projects/nixpkgs" ]

Default:
[ "nixpkgs=/nix/var/nix/profiles/per-user/root/channels/nixos" "nixos-config=/etc/nixos/configuration.nix" "/nix/var/nix/profiles/per-user/root/channels" ]

@jtojnar
Copy link
Contributor

jtojnar commented Jun 17, 2019

An alternative would be moving the default value from options to config section so it has the same priority (cf. environment.systemPath). But it would be a BC break.

@edolstra
Copy link
Member

IMHO we should avoid ad hoc extra... options (since you could add such an option for any list-typed option, which is rather ugly). If the issue is that the default search path gets overriden silently when you set a value, the solution is as @jtojnar suggested, i.e. change the default into a definition at regular priority:

config = {
  nix.nixPath = [
    "nixpkgs=/nix/var/nix/profiles/per-user/root/channels/nixos"
    "nixos-config=/etc/nixos/configuration.nix"
    "/nix/var/nix/profiles/per-user/root/channels"
  ];
};

Maybe using mkAfter to ensure that it doesn't override the user settings.

@danbst
Copy link
Contributor

danbst commented Aug 6, 2019

I agree with @edolstra about mkAfter. If Nix path search algorithm works "whoever is first, wins" then this is enough to allow both "append" and "override single" intents. The release notes should be added though.

@stale
Copy link

stale bot commented Jun 1, 2020

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
@andir
Copy link
Member Author

andir commented Jun 1, 2020

Still something I want to get back to eventually.. Keeping this open serves as a todo item for me.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
@stale
Copy link

stale bot commented Nov 28, 2020

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 28, 2020
@LoveIsGrief
Copy link
Contributor

Is this still an issue to resolve? What happens when -I NIX_PATH is passed to nix? It still seems to work, so I assume it's already falling back to default values or is that an incorrect assumption?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 11, 2022
@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 18, 2023
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 7, 2023
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank marked this pull request as draft March 20, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants