Skip to content
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.

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

Duplicate dynamic variables in multiple included taskfiles are overwritten #830

Open
dkryptr opened this issue Aug 8, 2022 · 13 comments
Open

Comments

@dkryptr
Copy link

dkryptr commented Aug 8, 2022

  • Task version: >= 3.2.1
  • Operating System: WSL2 - Ubuntu 20.04

I believe this is a regression in functionality.

I have a monorepo that I haven't touched for around 6 or more months. I upgraded and narrowed down the version that the issue was introduced (v3.2.1). Each project is in its own folder with a taskfile that declare dynamic variables. There's a root Taskfile.yml that includes each project's Taskfile.yml and sets the directory to the project's directory.

The behavior in v3.2.0 is that dynamic variable's working directory is scoped to whatever directory the parent Taskfile set in the includes statement. I've found this isn't the case in >= v3.2.1.

I've included the folder structure along with all file contents and the command line output to recreate the issue.

monorepo/
├── Taskfile.yml
├── project1/
│   ├── version.txt
│   ├── Taskfile.yml
└── project2/
    ├── version.txt
    ├── Taskfile.yml
# monorepo/Taskfile.yml
version: '3'

includes:
  project1:
    taskfile: ./project1/Taskfile.yml
    dir: ./project1
  project2:
    taskfile: ./project2/Taskfile.yml
    dir: ./project2
monorepo/project1/version.txt
# monorepo/project1/Taskfile.yml
version: '3'

vars:
  VERSION:
    sh: cat version.txt

tasks:
  version:
    cmds:
      - echo {{.VERSION}}
monorepo/project2/version.txt
# monorepo/project2/Taskfile.yml
version: '3'

vars:
  VERSION:
    sh: cat version.txt

tasks:
  version:
    cmds:
      - echo {{.VERSION}}

Screenshot 2022-08-08 005931
Screenshot 2022-08-08 005913

@MarioSchwalbe
Copy link
Contributor

Some remarks:

  1. It is quite hard to match the source and the output. The source prints the version, but the output shows paths.
  2. Global variables (even in multiple files) share the same namespace (scope) and overwrite each other. So you will always get the last definition of VERSION.
  3. If the variable is defined within the task, you might experience: Wrong shell execution directory for task-local variables #839

@dominik-lekse
Copy link

@dkryptr Thanks for describing the issue precisely.

I have encountered the issue recently in a similar structured repository. Tracking it down was not straightforward.

The Taskfile.yml of each sub project is included in the root Taskfile.yml while the dir attribute points to path of a sub project.

# root/Taskfile.yml
version: '3'

includes:
  project1:
    taskfile: ./project1/Taskfile.yml
    dir: ./project1
  project2:
    taskfile: ./project2/Taskfile.yml
    dir: ./project2
# root/project1/Taskfile.yml
# root/project2/Taskfile.yml
version: '3'

vars:
  PROJECT_DIR:
    sh: pwd

tasks:
  whereami:
    cmds:
      - echo "{{.PROJECT_DIR}}"
      - pwd

Within this context, the observed behaviour is as follows.

$ task project1:whereami
task: [project1:whereami] echo "root/project2"
root/project2
task: [project1:whereami] pwd
root/project1

$ task project2:whereami
task: [project2:whereami] echo "root/project2"
root/project2
task: [project2:whereami] pwd
root/project2

The reason for this different between echo "{{.PROJECT_DIR}}" and pwd in task project1:whereami is as @MarioSchwalbe described: Global variables (even in multiple files) share the same namespace (scope) and overwrite each other.

I am wondering if there has been a decision to keep global variables of included Taskfiles in the same namespace rather, or if scope these variables has not yet been implemented.

From a user of task, I had the intuition that global variables are scoped, similar to scoped task names when including in a parent Taskfile.

@MarioSchwalbe
Copy link
Contributor

In fact, both use cases do make sense. If it is the same global scope, included taskfiles can overwrite variables of their parent taskfile to allow further customization. For instance, my taskfiles are usually structured linke this:

version: '3'

# Default config.
vars:
  VAR1: 1
  VAR2: 2
  # ...

includes:
  # Local override if present.
  my:
    taskfile: local.yml
    optional: yes

tasks:
  # ...

With respect to composability, however, this might become a problem.

@dominik-lekse
Copy link

@MarioSchwalbe I understand that in the use case you presented, you want to optionally override global variables in a parent Taskfile by global variables in an included Taskfile. For this case, overrides make sense

In contrast, in the use case, in which two Taskfiles are included in a parent Taskfile, the overrides should not occur.

I want to share some additional findings:

  1. Dynamic variable values are overridden across two included Taskfiles if and only if the sh command is identical.

  2. Static variable values are not override across two included Taskfiles.

It seems that the result of the command in the dynamic variable is cached and then reused across the scope. The issue did not occur with dynamic variables, which have the same name but use a different sh command. As a workaround, I have added a unique comment at the end of the command.

Therefore, I would argue that this issue is actually a duplicate of #524.

Example

# root/Taskfile.yml
version: '3'

includes:
  project1:
    taskfile: ./project1/Taskfile.yml
    dir: ./project1
  project2:
    taskfile: ./project2/Taskfile.yml
    dir: ./project2
# root/project1/Taskfile.yml
version: '3'

vars:
  PROJECT_NAME: project1
  PROJECT_DIR:
    # sh: pwd
    sh: 'pwd; # project1'

tasks:
  whereami:
    cmds:
      - echo "{{.PROJECT_NAME}}"
      - echo "{{.PROJECT_DIR}}"
      - pwd
# root/project2/Taskfile.yml
version: '3'

vars:
  PROJECT_NAME: project2
  PROJECT_DIR:
    # sh: pwd
    sh: 'pwd; # project2'

tasks:
  whereami:
    cmds:
      - echo "{{.PROJECT_NAME}}"
      - echo "{{.PROJECT_DIR}}"
      - pwd

@max-sixty
Copy link

max-sixty commented Jan 10, 2023

I hit I have an even simpler reproduction of this, no need for sh:

# Taskfile.yml
version: 3

vars:
  bucket: '{{default "1" .bucket}}'

# includes:
#  other: Taskfile2.yml

tasks:
  task1:
    - "echo {{.bucket}}"
# Taskfile2.yml
version: 3
vars:
  bucket: '{{default "2" .bucket}}'

Without any include statement:

task task1

# 1

But only by including the second taskfile (uncommenting the includes above), we get a different result:

task task1

# 2

...which seems quite wrong — that we can change the default variable just by including another Taskfile.

@toddlerya
Copy link

I also encountered the same problem

@megakoresh
Copy link

Exactly same issue issue as well. Although I cannot reproduce it with directly set variables, only with dynamic variables. And it does not seem to be a race condition, as using sleep 0.$((1 + $RANDOM % 10)) && cat .version does not appear to solve the problem.

@megakoresh
Copy link

This really does seem to be identical to #524 I can confirm that adding a comment or some other meaningless string after the variable definition solves the issue. I think this can be closed then, no? Effort should be in #524 to fix the problem

@max-sixty
Copy link

This really does seem to be identical to #524 I can confirm that adding a comment or some other meaningless string after the variable definition solves the issue. I think this can be closed then, no? Effort should be in #524 to fix the problem

The example in #830 (comment) is different IIUC — no need for sh:. Is that correct?

@megakoresh
Copy link

This really does seem to be identical to #524 I can confirm that adding a comment or some other meaningless string after the variable definition solves the issue. I think this can be closed then, no? Effort should be in #524 to fix the problem

The example in #830 (comment) is different IIUC — no need for sh:. Is that correct?

Well like I said, I cannot repro this without the sh: at least. If you can, then sure this is different (unless the caching logic in #524 also applies to static vars).

@max-sixty
Copy link

Well like I said, I cannot repro this without the sh: at least. If you can, then sure this is different (unless the caching logic in #524 also applies to static vars).

What output do you get from my repro above?

@megakoresh
Copy link

What output do you get from my repro above?

@max-sixty i did not get notification for this, so tagging so you get the notification. Yes, I was able to repro the issue with your second "simpler repro" steps (without sh and without the nested foldas). I suppose the trick here is to have the dynamic value (i.e. something evaluated). I suspect maybe the cache keys are just using the variable name or just the yaml line contents, without taking the taskfile into consideration, so thats why this happens.

Would be nice if someone familiar with the project internals took a look at this because if the issue is this easy to reproduce, then this should be relatively simple to find a fix, if you know where the code that is responsible for this lies, no?

@max-sixty
Copy link

OK, thanks for repro-ing.

Would be nice if someone familiar with the project internals took a look at this because if the issue is this easy to reproduce, then this should be relatively simple to find a fix, if you know where the code that is responsible for this lies, no?

Not necessarily — but feel free to take a look — the number of people who are familiar with the code is not fixed...

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

No branches or pull requests

6 participants