Skip to content

Conversation

@ulyssessouza
Copy link
Contributor

What I did
This harmonizes the absolute path of project's working dir with the possible relative path of the env-file

Related issue
Resolves docker/for-mac#6229

(not mandatory) A picture of a cute animal, if possible in relation with what you did

@ulyssessouza ulyssessouza marked this pull request as ready for review April 28, 2022 05:36
@ulyssessouza ulyssessouza requested review from glours and ndeloof April 28, 2022 05:36

ef := o.EnvFile
if ef != "" && !filepath.IsAbs(ef) {
ef = filepath.Join(project.WorkingDir, o.EnvFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand the intent here. afaict filepath.Join(project.WorkingDir, o.EnvFile) should already make this path absolute, but maybe I missed something?
could you please clarify this in your PR description as the Docker Desktop issue is not public

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the example where:

  • /some/path is where we execute the command
  • /some/path/wrkdir is the path passed as --project-directory
  • ./wrkdir/envs/my.env as the path passed in --env-file

in this we would end with /some/path/wrkdir + ./wrkdir/envs/my.env which would repeat wrkdir resulting in /some/path/wrkdir/wrkdir/envs/my.env which is wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah ok, because we resolve envfile based on project dir (which is definitely a bug) and we should resolve relative to command working dir. Then filepath.Abs is relevant. Earlier version of the diff I reviewed was doing some voodoo magic with WorkingDir so my confusion

Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
@ulyssessouza ulyssessouza force-pushed the fix-env-file-relative-path branch from 47254db to 4ca5b8d Compare April 30, 2022 07:36
@ndeloof ndeloof merged commit 69e21d8 into docker:v2 Apr 30, 2022
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.

--env-file option messes up .env path when stopping app in Desktop

2 participants