Skip to content

requires_check_nil #2315

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

timrulebosch
Copy link

For variables without a value, the requires checks do not fail.

In the following taskfile, each variable evaluates to "" (a nil value), and still the required checks pass.

version: '3'
tasks:
  default:
    vars:
      A: null
      B: ''
      C: '{{.C}}'
      D:
        sh: echo ""
    cmds:
      - echo "A={{.A}}"
      - echo "B={{.B}}"
      - echo "C={{.C}}"
      - echo "D={{.D}}"
    requires:
      vars: [A, B, C, D]

This PR introduces a check for "nil" values which corrects this behavior.

Additional notes:
In RunTask the call sequence is very sensitive. areTaskRequiredVarsSet depends on dynamic variables being resolved, which happens in CompiledTask (a later call), however shouldRunOnCurrentPlatform (an earlier call) must occur before dynamic variables are expanded.

The check for areTaskRequiredVarsSet occurs between those two calls (FastCompiledTask and CompiledTask). Therefore it does not have access to resolved dynamic variables. Moving calls around causes tests to fail, indicating intricate dependencies.

Solution was to add a call FastCompiledTaskWithVars to the sequence which resolves the dynamic vars. Those dynamic vars should be cached, based on my code analysis, and therefore I don't expect any noticeable performance hit.

fixes #2243

Signed-off-by: Timothy Rule (VM/EMT3) <Timothy.Rule@de.bosch.com>
Signed-off-by: Timothy Rule (VM/EMT3) <Timothy.Rule@de.bosch.com>
Signed-off-by: Timothy Rule (VM/EMT3) <Timothy.Rule@de.bosch.com>
Signed-off-by: Timothy Rule (VM/EMT3) <Timothy.Rule@de.bosch.com>
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.

Requires does not check if a variable is set
1 participant