FIx calculation of task.Dir for tasks in included taskfiles.

[trulede]

Relocate the calculation of an included tasks task.Dir until the templating is available.

Previously, task.DIr would be calculated earlier which resulted in SmartJoin() incorrectly evaluating the second parameter, which had not been expanded (i.e. {{.FOO}}. To get correct behaviour, SmartJoin() needed to be called later, during compilation of the imported task, so that the second parameter could first be expanded, and then used correctly with SmartJoin().

fixes #2443
fixes #2497
fixes #1690
fixes #951

[trulede]

@andreynering @vmaerten In this PR I've relocated the point where an imported task.Dir is calculated, and as a result it is possible to use template variables in the include.dir. This is certainly helpful as many users of the Import feature encouter this limitation as some point and it is confusing as to why it does not work.

However, some tests (2) are proving to be remarkably flakey. These are sometimes passing, sometimes failing.

• TestIncludedTaskfileVarMerging/foo (taskfile)
• TestIncludedTaskfileVarMerging/bar (taskfile)

These test cases have two imported Taskfiles, each with the same global var DIR, and each setting that var to a different value. I wonder if there is a data-race (existing or introduced).

You might have ideas why that is the case? It seems unrelated to the effects of #2350, and somehow related to this PR. In either case I would likely add some additinal testcases to cover the change in functionality added by this PR.

[andreynering]

Hey @trulede!

First of all, thanks for your patience! I know if have many PRs opened and we are overdue on reviewing them.

I debugged this flaky test a little bit, but couldn't quickly find the problem, and want to focus on other PRs for today.

But to add context, this was originally fixed by @pd93 on the following PR, and your changes probably made that fix insufficient for some reason.

• fix: included variable merging #1649

[trulede]

@andreynering OK, thanks. I will take a look.

[trulede]

@andreynering I've sorted out the flakiness and added a test covering the specific scenario (of the PR, not the flake).

If the var being referenced is "dynamic", the outcome might be unexpected (possibly similar to the current behaviour). Its not clear if that could be resolved since dynamic vars are resolved on later passes (of the var stack) and they need a working directory to be resolved (which to use ....).

That limitation seems reasonable with the current implementation. It might be resolved with refactoring of the import mechanism.