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.

Sign up for GitHub

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

Jump to bottom

feat(dotenv): allow passing full paths to dotenv files #1080

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(dotenv): allow passing full paths to dotenv files #1080

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6cfd49b
feat(dotenv): allow passing full paths to dotenv files
alirezamirsepassi Mar 28, 2024
9a6694a
feat(dotenv): use path type for dotenv files
alirezamirsepassi Mar 28, 2024
8b066ff
fix(dotenv): fix type of files option in dotenv integration
alirezamirsepassi Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions src/modules/integrations/dotenv.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
let
cfg = config.dotenv;

normalizeFilenames = filenames: if lib.isList filenames then filenames else [ filenames ];
dotenvFiles = normalizeFilenames cfg.filename;
dotenvPaths = map (filename: (self + ("/" + filename))) dotenvFiles;

parseLine = line:
let
parts = builtins.match "([^[:space:]=#]+)[[:space:]]*=[[:space:]]*(.*)" line;
Expand Down Expand Up @@ -37,13 +33,18 @@ let
'';
in
{
imports = [
(lib.mkRenamedOptionModule [ "dotenv" "filename" ] [ "dotenv" "files" ])
];

options.dotenv = {
enable = lib.mkEnableOption ".env integration, doesn't support comments or multiline values.";

filename = lib.mkOption {
type = lib.types.either lib.types.str (lib.types.listOf lib.types.str);
default = ".env";
description = "The name of the dotenv file to load, or a list of dotenv files to load in order of precedence.";
files = lib.mkOption {
type = lib.types.either lib.types.path (lib.types.listOf lib.types.path);
apply = lib.toList;
default = config.devenv.root + "/.env";
Copy link
Contributor

@shyim shyim Mar 28, 2024 •
edited
Loading

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 🤔

Copy link
Author

@alirezamirsepassi alirezamirsepassi Mar 28, 2024 •
edited
Loading

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? 🤔

Copy link
Contributor

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

Copy link
Author

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?

Copy link
Contributor

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 😅

Copy link
Author

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? 🤔

Copy link

@mschnee mschnee Mar 28, 2024 •
edited
Loading

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";
  };

Copy link
Member

Choose a reason for hiding this comment

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

@shyim can you try using this PR to see if it addresses #257

Copy link
Contributor

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 🤔

Copy link
Member

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

description = "The path of the dotenv file to load, or a list of dotenv files to load in order of precedence.";
};

resolved = lib.mkOption {
Expand All @@ -61,17 +62,17 @@ in
config = lib.mkMerge [
(lib.mkIf cfg.enable {
env = lib.mapAttrs (name: value: lib.mkDefault value) config.dotenv.resolved;
enterShell = lib.concatStringsSep "\n" (map createMissingFileMessage dotenvPaths);
dotenv.resolved = mergeEnvFiles dotenvPaths;
enterShell = lib.concatStringsSep "\n" (map createMissingFileMessage cfg.files);
dotenv.resolved = mergeEnvFiles cfg.files;
assertions = [{
assertion = builtins.all (lib.hasPrefix ".env") dotenvFiles;
assertion = builtins.all (file: lib.hasPrefix ".env" (builtins.baseNameOf (toString file))) cfg.files;
message = "The dotenv filename must start with '.env'.";
}];
})
(lib.mkIf (!cfg.enable && !cfg.disableHint) {
enterShell =
let
dotenvFound = lib.any lib.pathExists dotenvPaths;
dotenvFound = lib.any lib.pathExists cfg.files;
in
lib.optionalString dotenvFound ''
echo "💡 A dotenv file was found, while dotenv integration is currently not enabled."
Expand Down