task icon indicating copy to clipboard operation
task copied to clipboard

Empty required variable still passing the required check?

Open trulede opened this issue 1 year ago • 2 comments

Discussed in https://github.com/go-task/task/discussions/1620

Originally posted by usersina April 26, 2024

  • Task version: v3.36.0
  • Operating system: Arch Linux x86_64 6.6.28-1-lts
  • Experiments enabled: No

Given the following Taskfile.yml:

version: "3"

silent: true

vars:
  IMAGE_NAME: my-web-app
  CONTAINER_REGISTRY: # docker.io

tasks:
  check-missing:
    desc: Test the task variables
    requires:
      vars: [CONTAINER_REGISTRY]
    cmds:
      - echo "CONTAINER_REGISTRY is {{.CONTAINER_REGISTRY}}"

My check-missing is not working as intended.

Actual output

CONTAINER_REGISTRY is

Expected output

task: Task "check-missing" cancelled because it is missing required variables: CONTAINER_REGISTRY

This works as intended if I simply delete CONTAINER_REGISTRY or comment it out, but why isn't it working if it's empty?

trulede avatar Jun 01 '24 00:06 trulede

Seems like the the code is only checking if the variable exists, not if it is also set.

Expectation is that nil or "" (empty string) would cause such a variable to fail the requires checks.

trulede avatar Jun 01 '24 00:06 trulede

Seems like the the code is only checking if the variable exists, not if it is also set.

Expectation is that nil or "" (empty string) would cause such a variable to fail the requires checks.

Documentation has this:

Variables set to empty zero length strings, will pass the requires check.

which is not so good. I really have to question that, both in what it means (what is an empty and zero length string), and if such behavior is even useful. The code behind variables is quite involved, so in many places you find variables being defaulted to an empty string (i.e. "") and other things.

Again with the documentation:

If you want to check that certain variables are set before running a task then you can use requires. This is useful when might not be clear to users which variables are needed, or if you want clear message about what is required. Also some tasks could have dangerous side effects if run with un-set variables.

which seems OK, but misses the use-cases where one task calls another with non-trivial variable passing, and included task files ... these are cases where requires would be useful. We have it in our task files, but it only acts as documentation for us (since it does not work).

I think the required checks should be:

  1. Does the variable exist (in the scope of a compiled task).
  2. Is the variable set.
  3. Is the variable set to a string of length greater than 0.

In the case that 3 would not be accepted as part of the implementation, then we need a way to set a variable to nil via the template engine (e.g a slim sprig function).

trulede avatar Jun 01 '24 09:06 trulede