-
Notifications
You must be signed in to change notification settings - Fork 330
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
feat(dotenv): allow passing full paths to dotenv files #1080
Conversation
src/modules/integrations/dotenv.nix
Outdated
files = lib.mkOption { | ||
type = lib.types.either lib.types.path (lib.types.listOf lib.types.path); | ||
apply = lib.toList; | ||
default = config.devenv.root + "/.env"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think allowing any path is good. This would make it more hard to remove impure
nix mode, as we can access files outside of our project 🤔
Aren't relative paths fine inside your project 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your response, I appreciate it. I agree with your point of view, but after doing some research, I found out that flakes copy the source to Nix store before evaluating it, and there's no way around this. As a result, using a git ignored dotenv file is impossible. 😭
Nevertheless, I do think that allowing users to choose the dotenv file path, even if it's outside of the project, is a great idea as it provides more flexibility. For example, I often have variables that I need to share across multiple projects, and a single dotenv file can be used for this purpose.
Aren't relative paths fine inside your project
No, as I mentioned the dotenv file is in my .gitignore file and it's not being copied to the nix store unfortunately
wdyt? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The project should not be copied into nix store with devenv provided nix 🤔 . But yes the gitignore has an effect 😓 .
Maybe we find a general solution to .gitignore, because of other issues like #1078
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the devenv CLI create the shell btw? 🤔 I'm not very familiar with rust honestly but what I understood from here, is it runs nix develop
with some options and arguments, right? 🤔 does it copy ignored files as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You stay in your local normal copy where you did run devenv shell. I am right now looking if to improve this 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@domenkozar hi there!
I saw that you changed config.devenv.root
to self
in the rewrite in rust project, this, unfortunately, breaks my environments where I use flakes as I mentioned in the original description of this PR. Do you happen to know the command that devenv runs to create the shell? Maybe including that command in the .envrc file along with the flakes template would make sense as in: currently the dotenv integration doesn't work in combination with flakes. wdyt? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also confirmed that the rust rewrite has broken dotenv support in devenv when using flakes. For now, I'm using the python-rewrite
branch.
flake-test via ❄ impure (devenv-shell-env) …
➜ direnv reload
direnv: loading ~/Projects/flake-test/.envrc
direnv: loading https://raw.githubusercontent.com/nix-community/nix-direnv/2.2.1/direnvrc (sha256-zelF0vLbEl5uaqrfIzbgNzJWGmLzCmYAkInj/LNxvKs=)
direnv: using flake . --impure
direnv: nix-direnv: renewed cache
💡 The dotenv file '.env' was not found.
Using the default flake created by nix flake init --template github:cachix/devenv
as documented here.
{
inputs = {
nixpkgs.url = "github:NixOS/nixpkgs/nixos-23.11";
systems.url = "github:nix-systems/default";
devenv.url = "github:cachix/devenv";
devenv.inputs.nixpkgs.follows = "nixpkgs";
};
nixConfig = {
extra-trusted-public-keys = "devenv.cachix.org-1:w1cLUi8dv3hnoSPGAuibQv+f9TZLr6cv/Hm9XgU50cw=";
extra-trusted-substituters = "https://devenv.cachix.org";
};
outputs = { self, nixpkgs, devenv, systems, ... } @ inputs:
let
forEachSystem = nixpkgs.lib.genAttrs (import systems);
in
{
packages = forEachSystem (system: {
devenv-up = self.devShells.${system}.default.config.procfileScript;
});
devShells = forEachSystem
(system:
let
pkgs = nixpkgs.legacyPackages.${system};
in
{
default = devenv.lib.mkShell {
inherit inputs pkgs;
modules = [
{
dotenv.enable = true;
# https://devenv.sh/reference/options/
packages = [ pkgs.hello ];
enterShell = ''
hello
'';
processes.run.exec = "hello";
}
];
};
});
};
}
Temporary Solution for Flakes
Use the python-rewrite
branch (shrug)
inputs = {
nixpkgs.url = "github:NixOS/nixpkgs/nixos-23.11";
systems.url = "github:nix-systems/default";
devenv.url = "github:cachix/devenv/python-rewrite";
devenv.inputs.nixpkgs.follows = "nixpkgs";
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use dotenv module and I don't see it why it should improve the performance 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would still evaluate self
that should be impacting the performance
This breaks on devenv with:
because the semantics of the paths are different, I'll think about it how to properly integrate it. |
Hey @domenkozar Thanks for your reply. IMO, the path type isn't very important for the module and it can be switched back to a simple string as the file path will be verified to exist later in the module using pathExists anyway. On second thought, we might not even want to have the path type as it will be copied to the Nix store if I'm not mistaken and it might contain some secrets that can't be managed directly/easily on the machine which later can be extracted by others. 🤔 let me know what you think. 🙏🏻 |
@domenkozar I switched the type back to string and wrapped the default value in a double quotation to avoid nix copying it to the store. Could you try again on devenv, please? |
@domenkozar any updates on this? |
Thanks for the ping, CI is running |
I get
|
8b066ff
to
a337f88
Compare
This pull request enables the dotenv integration to accept full paths to the dotenv files. Additionally, I've changed the option
filename
tofiles
and moved the normalize function to the option itself usinglib.toList
. I've also added a warning regarding the updated option name to prevent users from breaking their environments.P.S. I chose to use
config.devenv.root
instead ofself
becauseself
evaluates to the nix store source tree when used in a flake configuration. Since dotenv files are typically not committed to a git repository, they won't be present after the copy.