Skip to content

Conversation

@kerma
Copy link
Contributor

@kerma kerma commented Feb 9, 2023

Hey @andreynering! There's an issue with command line variable precedence which has bothered me for a while. I think it has created some confusion in issues as well, and to this day the documentation doesn't really clarify it. So I think there is an opportunity for improvement/change here.

Namely, CLI tools usually operate in a way that command line arguments take precedence over configuration files, and values set in environment variables. This seems to be the "unix way" since early days.

The problem with task today is that if you have a variable defined on included file level, or on a task vars section, you have to jump through hoops to override them per execution.

In other words, my "expected behavior" only works, when the variable is defined on root task level. And it does not work, when I define the variable in included task level, or for a specific task. Some examples, all executed with task YOU=cli

Works as expected, prints "Hello cli"

version: '3'

vars:
  YOU: World 

tasks:
  default:
    cmds:
      - echo "Hello {{.YOU}}"

Works:

version: '3'
includes: 
  internal: ./internal.yaml
vars:
  YOU: World 
tasks:
  default:
    cmds:
      - task: internal:default
---
# internal.yaml
version: '3'
tasks:
  default:
    cmds:
      - echo "Hello {{.YOU}}"

Does not work, prints "Hello World":

version: '3'

tasks:
  default:
    vars:
      YOU: World 
    cmds:
      - echo "Hello {{.YOU}}"

Doesn't work:

version: '3'
includes: 
  internal: ./internal.yaml
tasks:
  default:
    cmds:
      - task: internal:default
        vars:
          YOU: World
---
# internal.yaml
version: '3'
tasks:
  default:
    cmds:
      - echo "Hello {{.YOU}}"

This works as a small demo, but I have a similar setup where it does not. I just haven't figured out yet where the conflict is:

version: '3'
includes: 
  internal: ./internal.yaml
tasks:
  default:
    cmds:
      - task: internal:default
---
# internal.yaml
version: '3'
vars:
  YOU: World 
tasks:
  default:
    cmds:
      - echo "Hello {{.YOU}}"

etc..

To demo what I'm after I changed the evaluation order in the CompilerV3.getVariables. This is not a working fix as it breaks existing behavior regarding environment evaluation, highlighted by broken tests. But if you're willing to take this behavior in, I'll try to fix it properly.

@ameershira-uman
Copy link

Just a note, this also touches on the issue i have whereby the included taskfile behavoir changes implicitly just because its included. This is error prone, impossible to maintain and manage.

@andreynering
Copy link
Member

Hi @kerma and @ameershira-uman,

Unfortunately changes like this, if done, will have to wait until the next major version is released, because they are backwards incompatible.

That said, I'm not sure we really want to change the variable order. It worked more like you say you expected on previous major versions (CLI had preference over others), but the change to make it like it is today was intentional to give control to the task. You can make vars overridable by using default: MY_VAR: '{{.MY_VAR | default "foobar"}}'.

Regarding what @ameershira-uman said, I believe making variables scoped by Taskfile is probably a good consideration, but we'll probably wait until the next major version to release it.

@wtfzambo
Copy link

wtfzambo commented Mar 2, 2023

Tbh I don't think this a great idea, the current approach of setting defaults is more intuitive for the developer and similar to what you'd expect in other programming languages.

E.g.

version: '3'

tasks:
  default:
    vars:
      YOU: '{{default "World" .YOU}}'
    cmds:
      - echo "Hello {{.YOU}}"

isn't too different than:

def default(you='World'):
    print(f'Hello {you}')

On the contrary, this is how I imagine the suggested proposal would look like in another language:

def default(**kwargs):
    you = 'World'
    print(f'Hello {you}')

>>> default()
Hello World

>>> default(you='CLI')
Hello CLI

@blob626
Copy link

blob626 commented Jul 14, 2023

The way it is currently definitely feels odd to me. Every other tool if I want to override the default I can just pass it in via the cli, here I have to rely on the person who created the Taskfile to have set it up properly for that to work, which is wrong.

I would strongly argue that this is not more friendly for the developer. What would be friendly would be to just be able to set vars from the cli, like every other tool I use.

@pd93 pd93 mentioned this pull request Feb 3, 2025
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.

5 participants