poethepoet icon indicating copy to clipboard operation
poethepoet copied to clipboard

Included task envfile doesn't consider cwd

Open rjaduthie opened this issue 1 year ago • 6 comments

Summary

Circumstances

When including a task from another file definition - e.g. ./subproject/pyproject.toml - and using the cwd property in the root pyproject.toml.

Effect

If an envfile is used in the task definition in the submodule project, the path to the envfile doesn't consider the cwd when it's imported.

To recreate

Assuming there's an executable my_task and an env file ./subproject/.env.my_task:

./pyproject.toml:

[[tool.poe.include]]
path = "subproject/pyproject.toml"
cwd = "subproject"

./subproject/pyproject.toml

[tool.poe.tasks.my_task]
envfile = ".env.my_task"
cmd = "my_task"

Execution of poetry run poe my_task in the root project will have the variables as defined in ./subproject/.env.my_task.

Possible solutions

I've had a bit of a look around the code. Possibly when adding envs to the EnvVarsManager, the cwd can be respected for include'd tasks?

The code is quite complex (... I haven't yet worked out the interaction with the _get_upstream_invocations etc.), so I haven't got a PR for this issue. I might have so time to look later, but hope that I can get some pointers to how to approach this fix.

rjaduthie avatar Jul 27 '23 09:07 rjaduthie

(I thought I found that the issue was with a virtualenv, hence closing, but it actually seems not.)

rjaduthie avatar Jul 27 '23 11:07 rjaduthie

Hi @rjaduthie, thanks for raising this!

I see how that could be surprising. If I understand correctly what you're demonstrating then I guess the solution would be to make this line do something a bit like this one. You want to have go at it?

One complication with this however is that one might indeed wish for included tasks to be executed in a different directory, but still manage all the local (non checked in) .env files in the project root. So it be nice if we could work out how to have it both ways. Some ideas:

  1. always search both locations with a certain precedence
  2. support templating env vars into the value for envfile, e.g. envfile = "${POE_PROJECT}/.env" or create a new env var to also support envfile = "${POE_INCLUDE}/.env" I suspect the first option makes sense for your use case? but we could do both... What do you think?

nat-n avatar Aug 05 '23 20:08 nat-n

Thanks for the hints. I'll try to take a look at this at some point and suggest a PR to fix.

In a follow up, I think there's a similar effect with the specification of commands as well. Using the example from my original post, if the subproject task uses a sequence, then the cwd directive is not observed - i.e.

[tool.poe.tasks.my_task]
sequence = 
[
  reference_task,
  { cmd = "scripts_dir_within_subproject/my_task.sh" },
]

... here the second task within the sequence is within a subdir of the subproject, but Poe doesn't "include" it with the correct cwd.

roger-duthie-pivotal avatar Aug 21 '23 17:08 roger-duthie-pivotal

To respond to your question about the design options:

I wonder how obvious/intuitive it could be to a user if implementing the first option.

The second option could also get tricky, if you want to override an inherited behaviour.

A third option might be to have a flag within a task or the include block that specifies which behaviour to use - e.g. use_original_envfile=true - or something similarly suitable.

roger-duthie-pivotal avatar Aug 21 '23 17:08 roger-duthie-pivotal

@rjaduthie FYI I've started looking into this. It's more complex than I realised and I think justifies some refactor of how tasks manage and pass certain types of configuration.

nat-n avatar Oct 08 '23 13:10 nat-n

The issue with sequence task members inheriting cwd should be fixed now with v0.24.2

I'm still working on a fix for the issue included task files which involves a larger refactor.

nat-n avatar Nov 04 '23 10:11 nat-n

Just to say, we started using the updated package and it now works well for setting the working directory in sequenced tasks. Thank you for that fix!

roger-duthie-pivotal avatar Mar 04 '24 11:03 roger-duthie-pivotal

The 0.26.0 release includes a minor breaking change that addresses the original issue. The directory specified by the cwd option when including config will now be used at the base path for resolving relative paths for envfiles referenced within the included file.

The docs have been updated to reflect this.

nat-n avatar Apr 27 '24 22:04 nat-n