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

Refactor vars and templating to fix some bugs for v3 #218

Closed
stephenprater opened this issue Jun 18, 2019 · 10 comments · Fixed by #311
Closed

Refactor vars and templating to fix some bugs for v3 #218

stephenprater opened this issue Jun 18, 2019 · 10 comments · Fixed by #311
Labels
area: variables Changes related to variables.
Milestone

Comments

@stephenprater
Copy link
Contributor

If you are using dynamic variables, the shell invocations that populate those variables must not fail, regardless of the value of the templated variable.

Example:

tasks:
   release:
     vars:
       KIND: 'holepunch'
       RELEASED_TAG:
         sh:  git describe --abbrev=0 --tags --match "{{.KIND}}*"
     cmds:
        - echo "Doesn't matter won't run"

This task won't run because the sh command that populates RELEASED_TAG will fail when it is run with a KIND value of <no value> - ie git describe --abbrev=0 --tags --match "<no value>*"0

This feels like an undesirable behavior to me, since the only way to avoid this is to pass the variable in on the CLI, even though it has a static value.

Suggested solution:

Ignore errors on dynamic var assignments until the last expansion pass.

@smyrman
Copy link
Contributor

smyrman commented Jun 20, 2019

@stephenprater, a possible workaround for you now could be to rely on the default function.

version: "2.6"
tasks:
   release:
     vars:
       RELEASED_TAG:
         sh:  git describe --abbrev=0 --tags --match '{{.KIND | default "holepunch"}}*'
     cmds:
      - echo '{{.RELEASE_TAG}}'

Or if you also need to use KIND in cmds, you would need to do this:

version: "2.6"
tasks:
   release:
     vars:
       KIND: '{{.KIND | default "holepunch"}}'
       # Workaround for go-task/task#218; dynamic variables are evaluated before static ones.
       RELEASED_TAG:
         sh:  git describe --abbrev=0 --tags --match '{{.KIND | default "holepunch"}}*'
     cmds:
      - echo '{{.RELEASE_TAG}}'
      - echo '{{.KIND}}'

Fix vars for v3?

@andreynering, maybe one could also consider getting rid of the whole multiple passes all-together, as the multiple passes is something that could easily introduce side effects, even today. It might also be easier to reason about if variables are simply passed in order.

Perhaps for v3 #195, it's a good idea to change vars and env to be a list so they can be processed in order. This would also make it easier to extend vars with more fields.

vars:
- key: KIND
  desc: The kind of release to do
  default: holepunch
- key: RELEASED_TAG
  desc: The final release tag to use (evaluated from KIND by default)
  defaultEval: git describe --abbrev=0 --tags --match "{{.KIND}}*"

It's worth thinking about the design a bit more, this is just an example. One could look on tools such as Tusk, Terraform or even Kubernetes manifests for inspiration.

@andreynering
Copy link
Member

Yeah, I think since v3 is coming, we're open to discuss changes on how variables is implemented, since we definitely have bugs and undesired behavior on some use cases.

@andreynering andreynering added the type: bug Something not working as intended. label Jun 21, 2019
@andreynering
Copy link
Member

andreynering commented Jun 21, 2019

I think that maybe we could have ordered maps without having to convert it to slices using new changes introduced by go-yaml/yaml v3:

https://blog.ubuntu.com/2019/04/05/api-v3-of-the-yaml-package-for-go-is-available

We have an issue blocking us from using v3: go-yaml/yaml#450. But other than that, using ordered maps is probably better than slices, since it means less backward incompatibility between Task v2 -> v3.

@andreynering andreynering changed the title Dynamic Variable sh invocations cannot fail on first templating pass. Refactor vars and templating to fix some bugs for v3 Jun 21, 2019
@andreynering andreynering added this to the v3 milestone Jun 21, 2019
@andreynering andreynering mentioned this issue Jun 21, 2019
18 tasks
@andreynering andreynering added the area: variables Changes related to variables. label Mar 22, 2020
@galindro
Copy link

galindro commented Mar 24, 2020

One thing good that I'm trying to achieve is load envrionment variables exported by a sh file from a dependent task to the main task. For example:

vars.env
export MYVAR1=$(my command)
Taskfile.yml
version: '2'
tasks:
  load-vars:
    cmds:
      - . vars.env
  apply:
    deps:
      - load-vars
    cmds:
      - echo $MYVAR1

Do you guys think that will it be possible in v3? @andreynering

@andreynering
Copy link
Member

Hi @galindro,

I don't think it'll work as you suggested.

But keep in mind global env: is already a thing, and there's this proposal to run something before every command.

@galindro
Copy link

I like the idea from #204. Is it being considered to be released on v3?

andreynering added a commit that referenced this issue Mar 29, 2020
This shouldn't have any behavior changes for now. This is a code
refactor that should allow us to do further improvements on how
variables are handled, specially regarding respecting the declaration
order in Taskfiles, which should make it easier for the users.

Initial work on #218
andreynering added a commit that referenced this issue Apr 5, 2020
This shouldn't have any behavior changes for now. This is a code
refactor that should allow us to do further improvements on how
variables are handled, specially regarding respecting the declaration
order in Taskfiles, which should make it easier for the users.

Initial work on #218
@andreynering
Copy link
Member

@galindro Yeah, it's possible

@andreynering andreynering linked a pull request May 16, 2020 that will close this issue
@andreynering
Copy link
Member

After #311 some of the confusion with variables should have been fixed. Task will now respect the order of declaration of variables.

If possible, give the v3 branch a try and give some feedback before the final release.

@marco-m
Copy link
Contributor

marco-m commented Aug 17, 2020

Hello @andreynering I am going through the documentation https://taskfile.dev/ now that 3.0 is released.

Am I wrong or this part: https://taskfile.dev/#/usage?id=variables-expansion is not valid anymore after the merge of #311 ?

@andreynering
Copy link
Member

@marco-m Thanks for letting me know, I just updated it.

@pd93 pd93 removed the type: bug Something not working as intended. label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: variables Changes related to variables.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants