cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
4.92k stars 278 forks source link

cmd/cmd: Infer tasks across package boundaries #1535

Open verdverm opened 2 years ago

verdverm commented 2 years ago

Is your feature request related to a problem? Please describe.

To make tasks sharable, they can be put into regular cue files, with care, by setting $id and other fields as needed. (see included txtar)

However, cue cmd will not discover or run them (cue cmd err1). Task references do work for tasks outside of the command to be run, where the referenced value is in the same package (cue cmd bar). One can make cross-package references work by creating a package local reference to the imported task (cue cmd [ok1,ok2]). However this also fails if the imported value references another task within its own package (cue cmd err2)

Can task discovery and dependency resolution be changed such that the cross-package references work without the local references?

Repro

env COW=MOO FOO=BAR

exec cue cmd foo
cmp stdout foo.txt

exec cue cmd bar
cmp stdout foo.txt

exec cue cmd ok1
cmp stdout cow.txt

exec cue cmd ok2
cmp stdout cow.txt

exec cue cmd err1
cmp stdout cow.txt

exec cue cmd err2
cmp stdout cow.txt

-- cue.mod/module.cue --
module: "hof.io/repro"

-- cow/say.cue --
package cow

// a simple task to be imported
Say: {
  $id: "tool/os.Getenv"
  COW: string
}

// referenced task to be imported
env: {
  $id: "tool/os.Getenv"
  COW: string
}
// this is what is actually imported
Moo: {
  sound: env.COW
}

-- repro_tool.cue --
package repro

import (
  "tool/cli"
  "tool/os"

  "hof.io/repro/cow"
)

// typical
command: foo: {
  get: os.Getenv & {
    FOO: string
  }
  out: cli.Print & {
    text: get.FOO
  }
}

// referenced
Get: os.Getenv & {
  FOO: string
}
command: bar: {
  out: cli.Print & {
    text: Get.FOO      // infers local task in outer scope
  }
}

// errors, want these to work
command: err1: {
  out: cli.Print & {
    text: cow.Say.COW  // error: non-concrete string
  }
}
command: err2: {
  get: cow.Moo
  out: cli.Print & {
    text: get.sound    // infer fails when imported value references a local task within that package
  }
}

// workarounds for the first error
command: ok1: {
  get: cow.Say

  out: cli.Print & {
    text: get.COW      // infer imported task within flow root value
  }
}

Get2: cow.Say
command: ok2: {
  out: cli.Print & {
    text: Get2.COW     // infer imported task in outer scope
  }
}

// workaround for the second error requires the local reference within the same package which is being imported

-- foo.txt --
BAR
-- cow.txt --
MOO

Output

> env COW=MOO FOO=BAR
> exec cue cmd foo
[stdout]
BAR
> cmp stdout foo.txt
> exec cue cmd bar
[stdout]
BAR
> cmp stdout foo.txt
> exec cue cmd ok1
[stdout]
MOO
> cmp stdout cow.txt
> exec cue cmd ok2
[stdout]
MOO
> cmp stdout cow.txt

> exec cue cmd err
[stderr]
command.err.out.text: invalid string argument: non-concrete value string:
    ./repro_tool.cue:33:3
    ./repro_tool.cue:34:5
    tool/cli:4:3
[exit status 1]
FAIL: /tmp/testscript671789775/0/script.txt:15: unexpected command failure
error running infer-pkg.txt in /tmp/testscript671789775/0

> exec cue cmd err2
[stderr]
command.err2.out.text: invalid string argument: non-concrete value string:
    ./repro_tool.cue:38:3
    ./repro_tool.cue:39:5
    tool/cli:4:3
[exit status 1]
FAIL: /tmp/testscript512566896/0/script.txt:18: unexpected command failure
error running infer-pkg.txt in /tmp/testscript671789775/0
mpvl commented 2 years ago

@verdverm This is what the Config.InferTasks feature is for. Granted, dependency analysis is really hard for this when simultaneously not trying to evaluate an entire configuration. So at the moment the implementation is a bit feeble. The intent is to evaluate the entire configuration and/or have a quick mechanism for identifying tasks.

Can you point out to what extent this feature is not working for you?

verdverm commented 2 years ago

Can you point out to what extent this feature is not working for you?

  1. So the above cases (err1,err2) are intended to work and reading the entire config should fix this, and is on the long list of todos?
  2. If that is the case, should this be a bugfix rather than a feature request?
  3. The err1,err2 cases do not currently work. I use ok1,ok2 to work around this, though it is not a preferred UX to pass on to users and the patterns are a bit viral (cascading).

Note, I am using the Config.InferTask in my custom flow, which improved something worse to the above, which was repro'd using cue cmd for easier inclusion into tests here.