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.15k stars 297 forks source link

evalv3 does not obey CUE_DEBUG_SORT_ARCS=2 like the default evaluator #3460

Open mvdan opened 2 months ago

mvdan commented 2 months ago
$ cue version
cue version v0.11.0-alpha.1.0.20240920112956-687464eb1846+dirty

go version devel go1.24-165bf241f2 2024-09-19 00:40:50 +0000
      -buildmode exe
       -compiler gc
  DefaultGODEBUG asynctimerchan=1,gotypesalias=0,httpservecontentkeepheaders=1,multipathtcp=0,randseednop=0,tls3des=1,tlskyber=0,x509keypairleaf=0,x509negativeserial=1
     CGO_ENABLED 1
          GOARCH amd64
            GOOS linux
         GOAMD64 v3
             vcs git
    vcs.revision 687464eb1846e51628bb62e8e8804968bd300d69
        vcs.time 2024-09-20T11:29:56Z
    vcs.modified true
cue.lang.version v0.11.0

I would expect the following testscript to succeed:

env CUE_DEBUG_SORT_ARCS=2

env CUE_EXPERIMENT=
exec cue export
cmp stdout out.gold

env CUE_EXPERIMENT=evalv3
exec cue export
cmp stdout out.gold

-- file.cue --
package p

x: [_]: set: [string]: bool

x: [_]: {
    set: "aaa": true
    set: "zzz": true

    // Note that this bug depends on this field name sorting before the other.
    asList: [for p, _ in set {p}]
}

x: image1: set: ddd: true
-- out.gold --
{
    "x": {
        "image1": {
            "asList": [
                "aaa",
                "ddd",
                "zzz"
            ],
            "set": {
                "aaa": true,
                "ddd": true,
                "zzz": true
            }
        }
    }
}

However, it fails:

> env CUE_EXPERIMENT=evalv3
> exec cue export
[stdout]
{
    "x": {
        "image1": {
            "asList": [
                "ddd",
                "aaa",
                "zzz"
            ],
            "set": {
                "aaa": true,
                "ddd": true,
                "zzz": true
            }
        }
    }
}
> cmp stdout out.gold
diff stdout out.gold
--- stdout
+++ out.gold
@@ -2,8 +2,8 @@
     "x": {
         "image1": {
             "asList": [
-                "ddd",
                 "aaa",
+                "ddd",
                 "zzz"
             ],
             "set": {

FAIL: repro-evalv3.txtar:9: stdout and out.gold differ

evalv3 fails to sort the fields when looping in the list comprehension, so we get a non-sorted result.

mvdan commented 3 weeks ago

With CUE_EXPERIMENT=toposort having landed in https://review.gerrithub.io/c/cue-lang/cue/+/1202401, and it working on both evalv2 and evalv3, the need for this debug flag (and the fact that it broke with evalv3) is much less important to us. For example, https://github.com/cue-labs/services/blob/73749476ea5502f98e389160f4e431b6f1ab0806/deploy/baseline.sh#L8-L10 can be moved to using CUE_EXPERIMENT=toposort, which I will attempt shortly.

Do we want to keep CUE_DEBUG_SORT_ARCS around, perhaps as part of CUE_DEBUG instead? If so, we probably need to document it a little better and ensure it works with evalv3. If not, because we think that all end user use cases are resolved by toposort, and it's not really needed for our own debugging anymore, then we should delete it.

cc @mpvl @cuematthew