Skip to content

Fix using PWD in .env files #1052

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
Merged
merged 2 commits into from
Mar 1, 2024
Merged

Fix using PWD in .env files #1052

merged 2 commits into from
Mar 1, 2024

Conversation

reegnz
Copy link
Contributor

@reegnz reegnz commented Jan 17, 2023

This change introduces PWD support to the dotenv functionality, so that it can resolve PWD in .env files.

When .env is loaded, PWD will be set to the same folder that .env file can be found in variable can be used.
A user would also expect that the PWD variable is matching the folder that the .env file resides in, and symlinks should not interfere with that either.

Fixes #1051

@seanorama
Copy link

This would help a lot! Can we get it merged?

@reegnz reegnz mentioned this pull request Feb 2, 2024
Closed
@zimbatm
Copy link
Member

zimbatm commented Feb 28, 2024

Just pushed a small fix, otherwise it looks good to go. Let me know what you think!

reegnz and others added 2 commits February 28, 2024 13:56
@reegnz @zimbatm
Fix using PWD in .env files
aa64f0c 
This change introduces PWD support to the `dotenv` functionality, so
that it can resolve PWD in `.env` files.

When .env is loaded, PWD will be set to the same folder that .env file
can be found in variable can be used.
A user would also expect that the PWD variable is matching the folder
that the .env file resides in, and symlinks should not interfere with
that either.

Fixes direnv#1051
@zimbatm
fix: don't resolve the symlinks
6deffee 
Treat symlinks like bind-mounts.
@reegnz
Copy link
Contributor Author

reegnz commented Mar 1, 2024 •
edited
Loading

Hey @zimbatm I'm not entirely sure why the resolving of the symlinks is a problem and got removed. Could you elaborate?
I've found it to be more reliable if there's no ambiguity in case folders are symlinked to other places.
I mean I can live with it without evaluating the symlinks, just curious why we'd want the ambiguity.

Eg. putting a dotenv ../.env in .envrc the PWD would resolve to different values without resolving the correct location of the .env file if the folder is symlinked to multiple places (actual use-case, but I can solve it another way where it's not necessary to point out to a higher directory, but not as clean).

Anyway if there's any good reason for not wanting to resolve symlinks I'd still take PWD support without it, so feel free to merge without supporting resolution of symlinks.

@reegnz
Copy link
Contributor Author

reegnz commented Mar 1, 2024 •
edited
Loading

OK, I've contemplated on it a bit, and realized it works close to as intended with your fix.
Let's get this merged, I think I'm satisfied with this solution for now.
Will follow up with the symlink resolve in a separate PR to discuss that further.

@zimbatm
Copy link
Member

zimbatm commented Mar 1, 2024 •
edited
Loading

It depends on the usage. Maybe some users want to use the .env file as a template and symlink it into many places. Let's say the file contains PATH=$PWD/bin:$PATH. Then, it would be weird if the PATH contained a reference to the source folder instead of the destination. That's just one potential usage of symlinks.

@zimbatm zimbatm merged commit 7323cc3 into direnv:master Mar 1, 2024
@reegnz reegnz deleted the dotenv_pwd_fix branch March 6, 2024 13:25
@Fryuni
Copy link

Fryuni commented Apr 15, 2024 •
edited
Loading

The last release was 3 hours before this got merged 🥲

Any ETA for a release including this fix?

@gsong gsong mentioned this pull request Oct 6, 2024
Closed
@zimbatm
Copy link
Member

zimbatm commented Oct 7, 2024

finally got around to do release v2.34.0

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

@seanorama seanorama seanorama approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot reference PWD in .env file for variable expansion
4 participants
@reegnz @seanorama @zimbatm @Fryuni