task icon indicating copy to clipboard operation
task copied to clipboard

Templating, command lists and file globs

Open twhiston opened this issue 6 years ago β€’ 25 comments

Maybe i just am not looking at this from the right angle but I was trying to use the templating to generate a series of commands that can be run based on the input variables. For example lets say i have a list of variables for dockerfile versions/contexts

NS: tomwhiston
REPO: my-project
NAME: my-project
VERSIONS: |
  prod
  prodv3-builder
  prodv3-runtime
  test-base
  test

and the following task

build:
  cmds:
    - |
      {{- range $i, $version := .VERSIONS | splitLines -}}
         {{ if $version }}docker build -t "{{ $.NS }}/{{ $.REPO }}:{{ $version }}" ./context/{{ $version | replace "-" "/" }}{{ end }}
      {{ end -}}

Currently this task will always succeed, even if, for example, one of the contexts does not exist. So how would one go about implementing something like this, where any command that fails will abort operation, and without having a lot of calls to a task and setting specific vars to pass? ie not

- task: build
  vars: {NS: "tomwhiston", REPO: "my-project", VERSION: "prod" }

Ideally the outcome I would like is that if I add a new context I simple name it in the vars list and run the commands unaltered. If this is not possible currently would you be interested in such a feature being added?

twhiston avatar Dec 18 '17 23:12 twhiston

What happens if you start with set -e?

build:
  cmds:
    - |
      set -e
      {{- range $i, $version := .VERSIONS | splitLines -}}
         {{ if $version }}docker build -t "{{ $.NS }}/{{ $.REPO }}:{{ $version }}" ./context/{{ $version | replace "-" "/" }}{{ end }}
      {{ end -}}

smyrman avatar Dec 19 '17 09:12 smyrman

that will only work if you do

build:
  cmds:
    - |
      set -e
      {{ range $i, $version := .VERSIONS | splitLines -}}
         {{ if $version }}docker build -t "{{ $.NS }}/{{ $.REPO }}:{{ $version }}" ./context/{{ $version | replace "-" "/" }}{{ end }}
      {{ end -}}

note the subtle difference with removing the - from the start of the range. If you don't do this it will just make a broken first command that does not fail, so it's not obvious to catch. So it seems error prone to work this way as subtle problems may be silent. It also causes some issues if you need to change into a folder, because the whole output seen as one command so you are not returned to the root folder after each execution. This makes chaining together some cd ./wherever calls in your command a fail. You can solve this by using pushd and popd instead of cd, but its then not portable as pushd is not available in posix sh (for example). To me it would be cleaner if it was possible to implement another key maybe template: and then be able to generate via a go template a slice of commands that would be run independently. If you are interested in that idea i would for sure be up for working on it.

twhiston avatar Dec 19 '17 10:12 twhiston

makes sense

smyrman avatar Dec 19 '17 12:12 smyrman

cool, i'll try to implement this and make a pr over the next week or so

twhiston avatar Dec 19 '17 12:12 twhiston

note the subtle difference with removing the - from the start of the range.

Sorry, a bit quick reply, I was actually replying to your this comment πŸ‘†. As for making a template: key-word or similar for generating multiple commands it would be great to get @andreynering's opinion first. I think it could be a useful feature, although personally I would be happy with whatever works (including the for loop within a command), even if there would be likely to be some initial debugging issues. Documentation could be added to make it less painful.

In task we get a lot of features through the templating system, that could feel more natural if included in the yaml layout (not only this case, but also e.g. variable defaults); it's hard to know when it is actually more beneficial to (re-)implement them, and have the complexity of (maintaining) two ways to achieve the same goal?

. You can solve this by using pushd and popd instead of cd, (...)

For simple casese, cd - might work, to cd to the previous folder.

smyrman avatar Dec 19 '17 12:12 smyrman

If you don't do this it will just make a broken first command that does not fail, so it's not obvious to catch.

The simplest thing is probably to not use the whitespace control (-) operator at all, but rather let there be some blank lines in the generated shell script -- did it work btw?

smyrman avatar Dec 19 '17 13:12 smyrman

If I understood well, this feature request overlap with #29 and #73. Am I wrong?

andreynering avatar Dec 19 '17 13:12 andreynering

The linkage against #29 is an interesting one, because that describes a case that is not really implementable through the template syntax - i.e. issue multiple commands with different parameters.

Regarding #73, I haven't really tried to use the watch feature, so I can't comment to much there...

smyrman avatar Dec 19 '17 13:12 smyrman

Maybe something like this?

default:
  cmds:
    - echo hello
    - range: "my world task"
      split: " "
      cmd: ecoh-task
    - range: "my world echo"  # split: default to white-space,
                              # rangeVar: default to ELEMENT,
                              # rangeIndex: default to INDEX?
      split: " "
      sh: ecoh {{.INDEX}} {{.ELEMENT}}

echo-task:
  cmds:
    - echo {{.INDEX}} {{.ELEMENT}}

smyrman avatar Dec 19 '17 13:12 smyrman

I would be happy with whatever works (including the for loop within a command), even if there would be likely to be some initial debugging issues.

What I personally dislike about that is that you have false positives when things are not configured correctly, which can lead you to believe that everything is running correctly if not checking the output carefully. If it failed at any time anything was wrong then i would be totally on board with not adding a new key/feature for this. Its just if I want to use task to orchestrate some testing actions success by default is not something that is suitable for us.

The simplest thing is probably to not use the whitespace control (-) operator at all

I suppose this is true, but its a common part of templating syntax and you cant stop people using it, working in a company and using task i want my output to be as clean as possible for the rest of the team to read, and to remove all chances that a seemingly unrelated change (eating some whitespace) changes the actual execution of the tasks.

If I understood well, this feature request overlap with #29 and #73. Am I wrong?

I guess this is closer to #29 than #73 as i dont really care about watching files, it's more about writing a very generic command that you can pass variables to for execution and which runs each command separately, assessing the exit code.

what i was thinking was something like

template:
   -|
     {{- range $i, $version := .VERSIONS | splitLines -}}
         {{ if $version }}docker build -t "{{ $.NS }}/{{ $.REPO }}:{{ $version }}" ./context/{{ $version | replace "-" "/" }}{{ end }}
      {{ end -}} 
  -|
   {{- range $i, $version := .VERSIONS | splitLines -}}
         dgoss run ...
   {{end}}

which would produce an output like

- docker build ...
- docker build ...
- dgoss  run ...
- dgoss  run ...

etc....... the output could be treated like a normal cmd slice and just iterate through it running the commands and checking the exit codes

twhiston avatar Dec 19 '17 13:12 twhiston

@twhiston,

what i was thinking was something like

template:
   -|
     {{- range $i, $version := .VERSIONS | splitLines -}}
         {{ if $version }}docker build -t "{{ $.NS }}/{{ $.REPO }}:{{ $version }}" ./context/{{ $version | replace "-" "/" }}{{ end }}
      {{ end -}} 
  -|
   {{- range $i, $version := .VERSIONS | splitLines -}}
         dgoss run ...
   {{end}}

Does this not have the same issue that the sh case got in that you need to make sure white-space control is treated correctly to get the newlines in the right space? What if I want to add a command in top of the for loop?

template:
   -|
     echo hi there
     {{- range $i, $version := .VERSIONS | splitLines -}}
         {{ if $version }}docker build -t "{{ $.NS }}/{{ $.REPO }}:{{ $version }}" ./context/{{ $version | replace "-" "/" }}{{ end }}
      {{ end -}} 

What do you feel about the range cmd/sh keyword suggestion?

smyrman avatar Dec 19 '17 13:12 smyrman

@smyrman yeah, newlines will always be a bit of a nightmare i guess, personally i would add that echo as a seperate template slice entry, as it is not doing anything dynamic (or i would allow cmd and template and merge them together so you could put that command in the cmd) but you are totally right that we could not stop anyone doing it as above, and then yeah new line nightmare again. Also i guess we have that problem if people split over lines for readability but use the whitespace controls to ensure it is all output in a single line when parsed.

the range cmd/sh seems like a cool solution, but if range could be a var that would be even better , i'm thinking for example that you could create a very simple resuable task file for building and testing docker images if you deal with the dynamic parts in the vars file and not in the tasks, this is actually the way we currently deal with building docker images using make, super generic targets and project specific vars file included

twhiston avatar Dec 19 '17 14:12 twhiston

but if range could be a var that would be even better

I was presuming we allowed `range: "{{.MYVAR}}". Then you have the option to do both.

Β for building and testing docker images if you deal with the dynamic parts in the vars file and not in the tasks

A bit of-topic, but we use templated tasks for this. I think I can share at least some of it, with some container names sensored if you are interested.

smyrman avatar Dec 19 '17 22:12 smyrman

I was presuming we allowed `range: "{{.MYVAR}}".

yeah i think you are totally right. If we are in agreement about this i'd like to offer again to do the implementation for it :)

I think I can share at least some of it

i'd be really interested to see those templated tasks if its possible to share them, thanks very much!

twhiston avatar Dec 20 '17 08:12 twhiston

If you are interested in doing a PR, that would be great! There are some tricky bits with evaluation order / variable expansion that you would need to be aware of as I believe templates in vars (and cmds) are currently expanded before the task is started.

The best thing might be to alter the method that does this expansion to expand a range to multiple CMDS before issuing the command, and also evaluate the (range) template variables within that method...

Don't be afraid to start the PR before you are done (marked as WIP) to get early feedback, or to post some ideas on how to attack the problem here or on gopher slack.

i'd be really interested to see those templated tasks if its possible to share them, thanks very much!

Here they are. These are currently in the state of rewrite, and I want to share the latest and greatest so the exact version in the gist have not been tested, but we have something very similar that works. https://gist.github.com/smyrman/fdc43a72168ec7af16c2e35600c465e9. Feel free to ping me on gopher-slack if you have any questions.

smyrman avatar Dec 20 '17 10:12 smyrman

Just to feed the brainstorming, not rejecting any other idea. Something I has in mind for #29 is:

echo:
  sources: **/*.txt # foo.txt, bar.txt, foo/bar.txt
  foreach: true
  cmds:
    echo "{{.SOURCE}}"
task echo
# echo foo.txt
# echo bar.txt
# echo foo/bar.txt

So, maybe we can do something like this:

# will run for each source file
foreach: source
# will run for each variable value
foreach: {var: VAR, split: " "}

andreynering avatar Dec 20 '17 13:12 andreynering

@andreynering @smyrman Sorry for the delay, took a bit of a break over Christmas. As you have made some progress on #29 and #31 does that change your thoughts on how to implement this? I'll actually make a pass at doing this over the coming weeks so would be good to get a quick update. @smyrman thanks for the templated task files example!

twhiston avatar Jan 16 '18 18:01 twhiston

@twhiston Sorry for the long time to respond you. I had other priorities in the issue list, and I'll slowly start to think about this again.

andreynering avatar Feb 18 '18 12:02 andreynering

@andreynering no problem at all, happy to discuss it whenever you have time, no rush :)

twhiston avatar Feb 18 '18 12:02 twhiston

@twhiston I think a good start is looking to the foreach syntax I proposed in this comment above (or similar).

But if you have issues or better ideas for the syntax, we can always discuss in a possible PR. (If you want, open PR as WIP and ask for feedback).

Thanks πŸ˜„

andreynering avatar Feb 18 '18 13:02 andreynering

Any updates on the foreach syntax?

AlvarBer avatar Dec 28 '20 09:12 AlvarBer

@AlvarBer Nothing for now, sorry

andreynering avatar Jan 02 '21 14:01 andreynering

I have first-class ideas for how to implement this in v4

ghostsquad avatar Apr 16 '22 16:04 ghostsquad

@ghostsquad Once you're done to write a few examples of what you have in mind, that would certainly be interesting. πŸ™‚

andreynering avatar Apr 16 '22 18:04 andreynering

@andreynering and others, a recent discussion that can provide some insight into this is here: https://github.com/go-task/task/discussions/700#discussioncomment-2527969

ghostsquad avatar Apr 16 '22 20:04 ghostsquad

Work is in progress for this feature at #1220.

andreynering avatar Jun 17 '23 23:06 andreynering

It has been almost 6 years, but a loop feature was finally just released thanks to @pd93 work!

https://taskfile.dev/usage/#looping-over-values

ItsBeen84Years

andreynering avatar Jul 25 '23 01:07 andreynering

I don't know if it's positive or negative that it was waiting for 6 years to implement :) I know that it is an OSS project so it is how it is, but still, probably something discouraging to start using Task in the organization and/or consider sponsorship.

marverix avatar Jul 26 '23 10:07 marverix

@marverix We had a lot a activity throughout these 6 years. Yet, some features take some time to happen, like the loop feature we just released.

Keep in mind that all maintainers and contributors have days jobs and a life outside GitHub. We have few hours a week to dedicate to this project. So, that some things needs time to happen doesn't bother me, we do what we can.

Whether you are going to adopt Task or not it's your decision. I regularly review PRs, so if you want to contribute to a given feature, that's a possibility as well.

andreynering avatar Jul 26 '23 11:07 andreynering

@andreynering Sorry if I sounded arrogant. That wasn't my goal. As a developer who is also active in the Open Source world, I know what you are talking about. I meant that you can laugh at such things privately on the Discord channel. So it was more of a sincere concern.

About contribution: I would love to, but not everyone can write in Golang. But, I will try my best to figure out how to write something on my own.

Sorry again, Br Marek

marverix avatar Jul 26 '23 11:07 marverix