cue
cue copied to clipboard
cue-lang
trim issues after upgrade from v0.8.2 to v0.9.0
What version of CUE are you using (cue version)?
$ cue version
cue version v0.9.0
go version go1.22.4
-buildmode exe
-compiler gc
-trimpath true
CGO_ENABLED 0
GOARCH arm64
GOOS darwin
cue.lang.version v0.9.0
Does this issue reproduce with the latest stable release?
Yes - is new with v0.9.0 - not present in v0.8.2
What did you do?
$ cue trim ./elevation/...
imported and not used: "strings":
./elevation/apps/config/satebatcher.cue:4:2
But I am....
$ ack -A 1 -B 1 "strings\." -- ./elevation/apps/config/satebatcher.cue
for zone in #edgeAppZones {
let suffix = strings.SliceRunes(zone.name, strings.LastIndex(zone.name, "-")+2, len(zone.name))
#satebatcherComponent & {
So I added _what: strings.Split("what-the", "-")[0] to the file ... and re-ran the trim
$ cue trim ./elevation/...
< LARGE DIFF with a bunch of stuff removed >
Aborting trim, output differs after trimming. This is a bug! Use -i to force trim.
You can file a bug here: https://cuelang.org/issues/new?assignees=&labels=NeedsInvestigation&template=bug_report.md&title=
make: *** [cue/trim] Error 1
Most, but not all of the - in the diff are for comprehensions....
I am going to see if I can create a minimal re-producer, but don't have one yet.
What did you expect to see?
No change really ... this is an upgrade to v0.9.0 from a project that was using v0.8.2
What did you see instead?
See above.
I've tried to put together a re-producer, but I haven't been able to and I don't think it's a good idea to publicly make available the private cue bits.
Oddly enough I changed our cue fmt makefile target to run and it runs fine (the mindepth there is due to the layout of the repo and how we use cue export)
find ./elevation -type d -mindepth 6 | xargs -n 1 -I % sh -c "echo cue trim -s --strict %; cue trim -s --strict %"
@freeformz if you're unable to reduce the reproducer to be shared publicly, can you send it to me privately, for example via Slack? I can then do the reduction work so that we can have a small reproducer to investigate and add as a regression test.
Without a reproducer it might be hard for us to investigate this bug properly, as you can imagine.
Apologies for the slowness, @freeformz, I've been on holiday recently.
@cuematthew and I have reduced the "unused import" error to this testscript:
exec cue trim input.cue
-- input.cue --
package apps
import "strings"
#values: {
list: _
}
#values & {
list: [
for never in []
let trimmed = strings.TrimSpace(never)
{ trimmed }
]
}
Rather than succeeding as expected, by for example trimming the file or doing nothing at all, we get a confusing error like yours:
> exec cue trim input.cue
[stderr]
imported and not used: "strings":
./input.cue:3:8
[exit status 1]
FAIL: test.txtar:1: unexpected command failure
It's worth pointing out that CUE v0.8.2 failed in your code when running cue trim with a single package, but when running on all packages, it seemingly hid the error - which doesn't look right:
$ cue-v0.8.2 trim ./elevation/apps/config
imported and not used: "strings":
./elevation/apps/config/satebatcher.cue:4:2
$ cue-v0.8.2 trim ./...
$ echo $?
0
Something changed between 0.8 and 0.9 where CUE now consistently shows the error, rather than inconsistently hiding it:
$ cue-v0.9.0 trim ./elevation/apps/config
imported and not used: "strings":
./elevation/apps/config/satebatcher.cue:4:2
$ cue-v0.9.0 trim ./...
imported and not used: "strings":
./elevation/apps/config/satebatcher.cue:4:2
So it seems to us like the underlying "imported and not used" bug in trim already existed in v0.8, it was just being hidden from you when you ran cue trim ./.... That bug still seems like a problem and we will look into it - but now we have a much smaller reproducer, and it's interesting to see that it existed for a while.
We found the cause :) In your config, you had a list comprehension that always resulted in zero elements (./elevation/apps/config._satebatcherValues.app.components, which also has conflicting list types between [...string] and [...{}]?), so cue trim was trying to delete the entire comprehension and leave an empty list. This comprehension is where strings.SliceRunes was used, so when the only use of the strings import was removed, we should remove the import as well.
cue trim uses https://pkg.go.dev/cuelang.org/go/cue/ast/astutil#Sanitize to remove such unused imports, but it didn't do that correctly, hence the error. I'll send a fix shortly for the astutil package.
It's worth noting that, after the fix, cue trim ./elevation/apps/config still results in an error, due to cue trim having other bugs:
Aborting trim, output differs after trimming. This is a bug! Use -i to force trim.
You can file a bug here: https://cuelang.org/issues/new?assignees=&labels=NeedsInvestigation&template=bug_report.md&title=
But you can use cue trim -i ./elevation/apps/config to skip past that sanity check error and trim the files on disk, which does result in the expected changes I explain above. We are aware of the "output differs after trimming" bugs, and @cuematthew is looking at a new trim algorithm to resolve those.
So, in short - CUE v0.8.2's cue trim ./... seemed to be a no-op. Not only did it fail to trim due to a bug, it didn't even show you any error message at all. CUE v0.9.0 and master fail due to the astutil bug above, which we will fix - and then trimming will make changes with the -i flag.
From a quick look, the changes that cue trim -i ./... does make after the astutil fix don't look right. However, as I showed above with the existing breadth of trim issues, that's not entirely surprising. We will leave this issue open after the astutil fix, so that we can try trimming your project with the new algorithm once it is ready.
@mvdan I'm not sure how ./elevation/apps/config._satebatcherValues.app.components could have conflicting types of [...string] and [...{}] - any thoughts on debugging that ?
AFAICT I'm essentially doing this: https://cuelang.org/docs/tour/expressions/fieldcomp/
_satebatcherValues: xyz.#values & {
app: {
name: _meta.name
components: [
for zone in _something
let suffix = strings.SliceRunes(zone.name, strings.LastIndex(zone.name, "-")+2, len(zone.name)) {
_satebatcherComponent & {
name: "\(suffix)"
...
}
},
]
}
}
The astutil.Sanitize fix is now at https://review.gerrithub.io/c/cue-lang/cue/+/1198351.
Please ignore my mention of a list type mismatch - we spent a while aggressively reducing your code to reach the dozen lines of CUE I shared yesterday, and at some point during that process we made the list comprehension produce a list of strings rather than a list of structs. Your original code doesn't have this issue, as you correctly point out. The reproducer I shared ended up not having this issue either in the end, as we swapped list: [...{}] for list: _ and the bug still reproduced.
@freeformz from your pov, is this issue fixed now with the new trim implementation? I know you mentioned some weeks ago on slack that you'd found some new trim issues, but I don't think I've seen any new issues being filed.
It's been two weeks since the comment above, and we released v0.13.0 last week, so I'm going to assume that the issue here is resolved, like most other cue trim issues that Matthew resolved.
If you did indeed encounter new issues, please file them and we'll take a look. Thanks!