Skip to content

Commit

Permalink
cmd/cue: check all file I/O errors in fmt
Browse files Browse the repository at this point in the history
First, we weren't checking any errors while opening and parsing CUE
files for formatting via Decoder.Err. This is somewhat harmless, as
cue/load currently opens all of those files, so any error like "file
does not exist" or "file cannot be read" would likely already be caught.

However, we called Encoder.Close without checking its returned error,
and it turns out that when encoding into a filename on disk,
that ultimately returns the ioutil.WriteFile error.
This meant that we ignored any errors while opening, writing, and
closing the destination file, potentially doing less work silently.

This is what happened in #1791. Before the fix, I can reliably reproduce
the problem by lowering my hard limit on open files like so:

	$ ulimit -n 256

With 500 badly formatted CUE files, we would run:

	$ cue fmt *.cue

And the result was that, without an error, only 249 files were changed
on disk. After the fix, the error is very clear:

	$ cue fmt *.cue
	open [...]/250.cue: too many open files

Note that this CL only fixes the ignored errors, not the cause of the
"too many open files" error.

Add a test; with a read-only file containing badly formatted CUE,
`cue fmt` used to hide the fact that it failed to write to the file.
That makes for a fairly simple error test.
We can't test the ignored Decoder error as easily, because as explained
above, most reasonable errors will already be caught by cue/load today.

Updates #1791.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: Ic4beed9c6139194d71566722cf0f8749fa8150d6
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/552624
Unity-Result: CUEcueckoo <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Paul Jolly <[email protected]>
  • Loading branch information
mvdan committed Apr 14, 2023
1 parent 5e66c83 commit 8c6d9ba
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
7 changes: 6 additions & 1 deletion cmd/cue/cmd/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ func newFmtCmd(c *Command) *cobra.Command {

files = append(files, f)
}
if err := d.Err(); err != nil {
exitOnErr(cmd, err, true)
}

e, err := encoding.NewEncoder(file, &cfg)
exitOnErr(cmd, err, true)
Expand All @@ -87,7 +90,9 @@ func newFmtCmd(c *Command) *cobra.Command {
err := e.EncodeFile(f)
exitOnErr(cmd, err, false)
}
e.Close()
if err := e.Close(); err != nil {
exitOnErr(cmd, err, true)
}
}
}
return nil
Expand Down
21 changes: 21 additions & 0 deletions cmd/cue/cmd/testdata/script/fmt_issue1791.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Note that the permission errors below cover unix and windows.

# We can't read nor write.
chmod 000 in.cue
! exec cue fmt in.cue
stderr 'permission denied|Access is denied'

# We can read, but not write. We used to ignore some open/write errors.
chmod 444 in.cue
! exec cue fmt in.cue
stderr 'permission denied|Access is denied'

# We can read and write.
chmod 666 in.cue
exec cue fmt in.cue
cmp in.cue in.cue.golden

-- in.cue --
foo: "bar"
-- in.cue.golden --
foo: "bar"

0 comments on commit 8c6d9ba

Please sign in to comment.