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

evaluator: execution depends on file order #2555

Open nxcc opened 1 year ago

nxcc commented 1 year ago

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

$ cue-0.6.0 version
cue version v0.6.0

go version go1.20.6
      -buildmode exe
       -compiler gc
       -trimpath true
     CGO_ENABLED 0
          GOARCH amd64
            GOOS linux
         GOAMD64 v1

Does this issue reproduce with the latest stable release?

yes

What did you do?

testscript:

exec mv z.cue a.cue
exec cue-0.6.0 export .

-- b.cue --
package kube
oAuthProxy: [ID=_]: {name: ID}
for _, obj in oAuthProxy {(#oAuthProxy & {in: obj}).out}
#oAuthProxy: {
    in: {name: string}
    out: {deployment: "\(in.name)-proxy": {}}
}

-- c.cue --
package kube
oAuthProxy: prometheus: {}

-- z.cue --
package kube
deployment: {}
for _, deployment in deployment {}

What did you expect to see?

PASS

What did you see instead?

deployment: field "prometheus-proxy" not allowed by earlier comprehension or reference cycle

Comment

disabling the first line (exec mv...) make this test pass, so failure depends on the file order (alphabetical)

mvdan commented 1 year ago

Did you by any chance test older CUE versions? It would be interesting to see if this was a regression between v0.5.0 and v0.6.0, for example.

Certainly a bug to be fixed, either way.

mvdan commented 1 year ago

I also wonder why the earlier fix at https://review.gerrithub.io/c/cue-lang/cue/+/542725 wouldn't be enough here; we should be sorting the files as we load them, so the order they are fed in should not matter at all.

nxcc commented 1 year ago

cue 0.5.0 also shows this issue

mvdan commented 1 year ago

I also wonder why the earlier fix at https://review.gerrithub.io/c/cue-lang/cue/+/542725 wouldn't be enough here; we should be sorting the files as we load them, so the order they are fed in should not matter at all.

On second thought, what you're doing here is not swapping the order of files in the command line, you're renaming one so that it ends up in a different lexicographical order. So the earlier CL is unrelated.

nxcc commented 1 year ago

Yes, sorting seems to work. It's about order of parsing/executing that is influenced by order of reading.