`cue fmt` differences between `darwin/arm64` and `linux/amd64`

[jszwedko]

We are seeing some odd differences running cue fmt on darwin/arm64 vs. linux/amd64 that we can't quite figure out. Opening this in case anything jumps out to you all. Basically, on linux/amd64 we are seeing it make some whitespace changes it doesn't make on darwin/arm64.

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

cue version v0.4.3 linux/amd64

and

cue version v0.4.3 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What did you do?

If you checkout https://github.com/vectordotdev/vector at revision dddd453b1 and then run cue fmt website/**/*.cue on linux/amd64 you will see it make whitespace changes. If you run the same cue fmt website/**/*.cue on darwin/arm64 it does not make any whitespace changes.

In both cases, bash 5.1.16 was used. I verified the shell globbing returns the same list of files in both cases.

What did you expect to see?

I expected it to make the same changes on darwin/arm64 it did on linux/amd64.

What did you see instead?

On linux/amd64 it makes the changes, on darwin/arm64 it does not.

[jszwedko]

I ran darwin/amd64 under Rosetta, too, and it seems to have the same behavior where it doesn't make the whitespace changes. This might mean it is something Darwin specific.

[jszwedko]

Just wanted to bump this since we continue to have team members and external contributors run into it and become confused why CI formats differently than locally on OSX.

[blefevre]

This seems to related to processing a large list of files. When I remove the globbing and target each file individually:

find website -name '*.cue' | xargs -I _ cue fmt _

The file formats successfully.

[myitcv]

Thanks for raising, @jszwedko and apologies for this getting dropped. Looking into it now.

[myitcv]

@jszwedko I ran the repro steps:

$ git clone https://github.com/vectordotdev/vector
$ cd vector/
$ git checkout dddd453b1
error: pathspec 'dddd453b1' did not match any file(s) known to git

I have a hunch what is going on here and will try and come up with an independent repro, but would like to confirm that against the actual problem you are seeing.

[myitcv]

(sorry, hit Cmd+Enter instead of Enter)

... so if you can provide a repro from your side that would be great, thanks.

[jszwedko]

Hey @myitcv ,

Apologies for the delay. That commit was on a deleted branch. I pushed a new one up, 2dbc91729, that has invalid cue formatting that cue fmt --verbose --trace website/**/*.cue doesn't format. Interestingly, as https://github.com/cue-lang/cue/issues/1791#issuecomment-1338472946 notes, the files are formatted if formatted individually via find website -name '*.cue' | xargs -I _ cue fmt _.

I don't think this is a shell issue:

vector on  jszwedko/cue-fmt-failure-example [$] is 📦 v0.27.0 via 🦀 v1.64.0 on ☁️
❯ ls -1 website/**/*.cue | wc -l
     664

vector on  jszwedko/cue-fmt-failure-example [$] is 📦 v0.27.0 via 🦀 v1.64.0 on ☁️
❯ getconf ARG_MAX
1048576

vector on  jszwedko/cue-fmt-failure-example [$] is 📦 v0.27.0 via 🦀 v1.64.0 on ☁️
❯ ls -1 website/**/*.cue | wc -c
   35819

Again it seems to format correctly on Linux (Ubuntu) too, which is odd.

Hopefully this helps with reproduction! Let me know if you need any additional details.

[myitcv]

@mvdan please can you take a look into this?

[mvdan]

@jszwedko I've been unable to reproduce this on Linux so far. Just to double check, are your runs on Mac with shopt -s globstar? If globstar is not enabled, ** roughly behaves like *, so then you're just formatting website/cue/reference.cue:

$ shopt -u globstar
$ ls -1 website/**/*.cue
website/cue/reference.cue
$ shopt -s globstar
$ ls -1 website/**/*.cue | wc -l
664

Worse, that's a silent downgrade of sorts, because the command still succeeds, and bash does not reject ** as invalid syntax.

[mvdan]

Worth noting that that would also explain why find with xargs works: that method does not rely on globstar.

[jszwedko]

Thanks for the response @mvdan !

This doesn't seem to be a globstar issue for me:

On the OSX box that does not format all files correctly:

$ ls -1 website/**/*.cue | wc -l
     664

This matches what I see on the Ubuntu box, which does format all files correctly.

[mvdan]

Only now catching up to your reply :) That is surprising, globstar not being turned on did sound like the most likely explanation for the inconsistency.

We aren't able to reproduce the issue, and we even tried on one of our Mac laptops. Would you be able to do a bit of pair debugging with me, to try and narrow down what is the difference between your system and mine? If we could do a thirty minute call sometime this month and share our screens, hopefully we'll be able to figure it out. You can DM me on Slack at mvdan, or email me at the same username at cue dot works.

[myitcv]

As part of easing back into CUE after some time off, I stumbled across updates to this issue and spent a bit of time investigating.

I think I've found the issue. @jszwedko - appreciate your patience in helping to debug here.

The critical part of being able to reproduce this is to use the official build of v0.4.3. Otherwise, bash versions etc per @jszwedko. Given that setup, the steps outlined in https://github.com/cue-lang/cue/issues/1791#issuecomment-1352155807 should be sufficient.

The main thing to observe is that the official release of v0.4.3 was built using go1.18.1. Notice that using go1.19.5 to build v0.4.3 "fixes" the problem. i.e. we see that fmt has actually done some work.

Bisecting the Go version to find when things got "fixed" points to:

https://go-review.googlesource.com/c/go/+/393354

which very much corresponds to the fact that a key aspect of this bug is a large number of files being passed as arguments to cue fmt.

Per an offline exchange with @mvdan, it's possible we are failing to close the files that are the args to cue fmt. Perhaps deferring the close until the entire command has finished?

It's also strange that we aren't getting an error. Are we swallowing an os.Open() error somewhere, an error that would otherwise have pointed to us hitting an open file limit?

Orthogonal but related. cue version does not currently include the Go version used to build cmd/cue. This would seem like a useful thing to include, because it might well have flagged this "dimension" earlier.

@mvdan - please can you follow up on these points? Thanks

[mvdan]

Found a bit of time to investigate this evening :) This was a double bug. Not only were we deferring closing files in a loop, which caused Mac users to run into open file limits before Go 1.19, but we were also ignoring I/O errors (open, write, close, etc) in two crucial bits of code which meant that the "too many open files" error wasn't even showing in the reproducers above. That's why some files weren't being formatted and no error was being reported; we were failing to open more files, and we were happily ignoring those errors and moving along.

See https://review.gerrithub.io/c/cue-lang/cue/+/552624 and https://review.gerrithub.io/c/cue-lang/cue/+/552625, which will hopefully ship with v0.6.0-alpha.2. Although with Go 1.19 or 1.20, the open file soft limit is already being raised by Go, so it should be far less likely to run into this bug in practice.

[jszwedko]

Oh wow, that is subtle. Thanks for addressing this @mvdan !