tonerdo / dotnet-env

A .NET library to load environment variables from .env files
MIT License
427 stars 50 forks source link

2.4.0 Doesn't Work - Fails to Load .env #76

Closed Fosol closed 1 year ago

Fosol commented 1 year ago

The latest update breaks DotNetEnv.Env.Load(); as it doesn't bother loading the .env file anymore.

Downgrade to 2.3.0 and it works.

rogusdev commented 1 year ago

Hey, thank you very much for the feedback. Could you perhaps try the example I suggest in: https://github.com/tonerdo/dotnet-env/issues/69#issuecomment-1408940848 and also elaborate on the conditions you are running this in?

Fosol commented 1 year ago

I have a Entity Framework Factory to run database migrations.

public class CustomContextFactory : IDesignTimeDbContextFactory<TNOContext>
{
    public TNOContext CreateDbContext(string[] args)
    {
        DotNetEnv.Env.Load();
        var builder = new ConfigurationBuilder()
            .SetBasePath(Directory.GetCurrentDirectory());
        builder.AddEnvironmentVariables();
        var config = builder.Build();
        var cs = config.GetConnectionString("Default");
        // Should have connection string from .env, but 2.4.0 returns nothing
    }
}

In the same folder as the project is a .env file with the following.

ConnectionStrings__Default=**********
rogusdev commented 1 year ago

Thanks again. For the sake of helping me debug this, can you try this while using 2.4.0: DotNetEnv.Env.Load($"{Directory.GetCurrentDirectory()}.env") ?

That said, after we have debugged this issue, in 2.4.0, you should also be able to simplify your use case, I believe, with this new feature: https://github.com/tonerdo/dotnet-env#using-net-configuration-provider

Fosol commented 1 year ago

The following works.

DotNetEnv.Env.Load($"{Directory.GetCurrentDirectory()}/.env");

Which means whatever code was changed in 2.4.0 to use the default current directory doesn't work anymore.

Fosol commented 1 year ago

Additionally .AddDotNetEnv(".env") doesn't work either. It will only work if I once again specify the current directory.

.AddDotNetEnv($"{Environment.CurrentDirectory}/.env")

rogusdev commented 1 year ago

OK, I'll roll that change back. Thanks!

MrDave1999 commented 1 year ago

@Fosol

I think you should first indicate with which tools the test was performed. For example, I tried to reproduce this problem using the following:

And everything works for me. The .env file loads. I run my migrations with:

dotnet ef migrations add InitialCreate
Fosol commented 1 year ago

<TargetFramework>net7.0</TargetFramework>

MrDave1999 commented 1 year ago

@Fosol Ok, tell me how I can reproduce this problem. Everything works for me (I tried your code).

Fosol commented 1 year ago

Take a look at the open source code here. Note this doesn't have the updated 2.4.0 applied, but it's the same code.

I have tested the above changes and when setting the current directory it works. So the change that broke it must not be using the same location.

https://github.com/bcgov/tno/blob/dev/libs/net/dal/TNOContextFactory.cs

Fosol commented 1 year ago

If you review the PR that caused the issue you'll see why it doesn't work.

https://github.com/tonerdo/dotnet-env/pull/72/files

Switching to AppContext.BaseDirectory from Directory.GetCurrentDirectory() is a change to the original logic. It's a breaking change. They don't mean the same thing.

Fosol commented 1 year ago

Perhaps AppContext.BaseDirectory might actually be more desirable going forward, as it should fetch the .env where the main execution occurs, or perhaps it may be confusing as you want the .env as close to the project as possible. Up to you what you want to do going forward, but it should be noted as a breaking change and documented on the front page.

MrDave1999 commented 1 year ago

@Fosol Good. I'll check this out later. You can try this dotenv library to check if this same problem occurs? (it also uses AppContext.BaseDirectory).

Fosol commented 1 year ago

Yes that will have the same "issue". It looks like the history of that library also changed the same way at some point, which would have resulted in a breaking change.

Technically both approaches are valid. One approach may be more desirable or intuitive depending on how they are commonly used. Up to you what you decided to do, just realize the change from 2.3.0 to 2.4.0 is a breaking change.

MrDave1999 commented 1 year ago

@Fosol I seem to have found the problem.

Directory.GetCurrentDirectory() returns: C:\Users\syslan\Desktop\tno\libs\net\dal. And AppContext.BaseDirectory returns: C:\Users\syslan\Desktop\tno\libs\net\dal\bin\Debug\net7.0\.

And DotNetEnv.Env.Load(); by default it searchs for the .env file in the current directory (i.e., in C:\Users\syslan\Desktop\tno\libs\net\dal\bin\Debug\net7.0\). That's why it doesn't load anything (assuming that the .env is in the same directory where the .csproj is located).

The solution is to use DotNetEnv.Env.TraversePath().Load(); so that it searchs in the parent directories. This problem does not occur with dotenv.core because by default it searches in parent directories.

Fosol commented 1 year ago

Traversing the parent directories can result in unexpected results and should be opted in rather than default behaviour.

My default expectation is that the .env would reside where I executed the dotnet command. This is also how it used to behave.

The change would most likely not impact a deployed assembly, however local development and testing is impacted. I would be surprised if anyone would want .env files to show up in the build folder.

MrDave1999 commented 1 year ago

@Fosol Yes, it is that AppContext.BaseDirectory would return C:\Users\syslan\Desktop\tno\libs\net\dal if the project targets to .NET Framework. There is an breaking change in this library as you mention.

What unexpected results might occur if DotNetEnv.Env.Load(); were to search by default in parent directories?

For example, dotenv.net also has this behavior as default.

Fosol commented 1 year ago

Might be multiple .env files in a solution. Traversing may find one that isn't relevant.

rogusdev commented 1 year ago

I have reverted this change for the newly released 2.5.0 and unlisted 2.4.0 .