cue-lang / cue

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

cuecontext: cross package tags remain unresolved #1484

Open jgillich opened 2 years ago

jgillich commented 2 years ago

What version of CUE are you using (cue version)?

master/0.4.1 (Go) and 0.4.0 (cli)

Does this issue reproduce with the latest release?

Yes

What did you do?

Clone this repo or copy the files below https://github.com/jgillich/cue-sample

// foo/foo.cue
package foo

import "encoding/json"

#foo: string @tag(foo)
json.Unmarshal(#foo)

// bar/bar.cue
package bar

import "github.com/mypkg/foo"

f: foo
// main.go
package main

import (
    "log"

    "cuelang.org/go/cue/cuecontext"
    "cuelang.org/go/cue/load"
)

func main() {
    config := &load.Config{}
    config.Tags = []string{"foo={\"foo\":1}"}
    instances := load.Instances([]string{"./bar", "./foo"}, config)
    for _, b := range instances {
        if b.Err != nil {
            log.Fatal(b.Err)
        }
    }
    cueContext := cuecontext.New()
    defs, err := cueContext.BuildInstances(instances)
    if err != nil {
        log.Fatal(err)
    }
    json, err := defs[0].MarshalJSON()
    if err != nil {
        log.Fatal(err)
    }
    log.Printf("json: %v", string(json))
}

What did you expect to see?

The expected result is achieved through the cli:

$ cue export ./bar ./foo --inject 'foo={"foo":1}'
{
    "f": {
        "foo": 1
    }
}
{
    "foo": 1
}

What did you see instead?

The go code produces the following error:

foo.go:35: cue: marshal error: f: error in call to encoding/json.Unmarshal: non-concrete value string
exit status 1

This was traced down to be caused by using cuecontext.BuildInstances over cue.Build. More information in this discussion: https://github.com/cue-lang/cue/discussions/1477

mpvl commented 2 years ago

I see this is different from #1070?

Was this working before?

jgillich commented 2 years ago

This is a bug in cuecontext.BuildInstances, it works as expected using cue.Build. #1070 is related but it's not the same, sorry if the title is a bit confusing.

mpvl commented 2 years ago

Thanks for the clarification!

mpvl commented 2 years ago

@jgillich I see where the difference is coming from. BuiltInstance considers each package as its own evaluation, whereas Build evaluates each package only once. From an encapsulation point of view BuildInstances is the correct interpretation. Otherwise, it means the evaluation of a package would differer depending on what other packages happen to be specified on the command line. So I would consider this a bug fix.

However, I like your proposal of allowing tags of the format pkg/path.identifier in tags in #1070. That would provide an out for this case. Would that work for you?

jgillich commented 2 years ago

Yea absolutely 👍

mpvl commented 2 years ago

One more possibility, I've commented on that in #1070