Skip to content

nix: Remove profileEnv#437

Closed
akshaymankar wants to merge 1 commit intomasterfrom
akshaymankar/no-nix-path
Closed

nix: Remove profileEnv#437
akshaymankar wants to merge 1 commit intomasterfrom
akshaymankar/no-nix-path

Conversation

@akshaymankar
Copy link
Copy Markdown
Member

The overridden NIX_PATH doesn't seem to be used anywhere.

@akshaymankar akshaymankar requested a review from arianvp March 24, 2021 14:06
@lucendio
Copy link
Copy Markdown
Contributor

lucendio commented Apr 9, 2021

The overridden NIX_PATH doesn't seem to be used anywhere.

I think it is - again.

@akshaymankar
Copy link
Copy Markdown
Member Author

I am not sure if it is used even in that PR, i see that @arianvp changed the --no-out-link to --out-link .nix-env, but I think it is done so that we nix-store --gc doesn't delete the stuff required for wire-server-deploy. I still see no use of $NIX_PATH or <nixpkgs>.

@arianvp
Copy link
Copy Markdown
Contributor

arianvp commented Apr 12, 2021

NIX_PATH is just there as a weird convenience such that nix-shell -p <package here> works inside the direnv'd directory. It's not needed it was a convenience hack that Florian added.

@akshaymankar
Copy link
Copy Markdown
Member Author

I am a little unsure on what convenience it provides. Is it just that if you want to runnix-shell -p <something>, it will use the nixpkgs pinned in the repo?

@arianvp
Copy link
Copy Markdown
Contributor

arianvp commented Apr 14, 2021

That's the only convenience yes. I'm not sure if it's worth it.

Especially when nix 3.0 comes (any time now!) with the Flake support; NIX_PATH won't even really exist anymore...

I am convinced we can just remove it. It probably confuses people more than it helps them..

@akshaymankar
Copy link
Copy Markdown
Member Author

Cool, I will fix conflicts and ask for reviews again soon :)

The overridden NIX_PATH doesn't seem to be used anywhere.
@akshaymankar akshaymankar force-pushed the akshaymankar/no-nix-path branch from 6847c91 to e4b5af1 Compare May 3, 2021 10:21
@akshaymankar
Copy link
Copy Markdown
Member Author

@arianvp I fixed the conflicts, please feel free to review it and merge if you approve.

'';
};

pythonForAnsible = pkgs.python3.withPackages (p: [ p.boto p.boto3 p.six p.cryptography ]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this unused?

@flokli
Copy link
Copy Markdown
Contributor

flokli commented Sep 22, 2021

The convenience here also ensures we all use the same terraform minor version, in case we need to nix-shell one of these in.

Is this so much of a burden to not keep around?

@flokli flokli changed the base branch from develop to master September 22, 2021 11:49
@akshaymankar
Copy link
Copy Markdown
Member Author

Stale. Closing, will reopen if I get bothered by this.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants