Skip to content

Commit

Permalink
cmd/cue: do not initialize cmd.Command twice in cue cmd
Browse files Browse the repository at this point in the history
This allows `cue cmd --cpuprofile` to work, avoids other issues
such as initializing cueexperiment and cuedebug twice unnecessarily,
and also adds an assertion so that future double-init bugs like this one
don't go unnoticed for a while.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I97dd014cb2997f20122dd761822a32e483d9e951
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198496
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Matthew Sackman <[email protected]>
  • Loading branch information
mvdan committed Jul 29, 2024
1 parent 17b3e52 commit 3f84ba6
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
8 changes: 5 additions & 3 deletions cmd/cue/cmd/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,15 @@ func customCommand(c *Command, typ, name string, tools *cue.Instance) (*cobra.Co
Use: usage,
Short: lookupString(o, "$short", short),
Long: lookupString(o, "$long", long),
RunE: mkRunE(c, func(cmd *Command, args []string) error {
// Note that we don't use mkRunE here, as the parent func is already wrapped by
// another mkRunE call, and all Command initialization has already happened.
RunE: func(cmd *cobra.Command, args []string) error {
// TODO:
// - parse flags and env vars
// - constrain current config with config section

return doTasks(cmd, name, tools)
}),
return doTasks(c, name, tools)
},
}

// TODO: implement var/flag handling.
Expand Down
10 changes: 10 additions & 0 deletions cmd/cue/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,18 @@ type Stats struct {
}
}

var hasRunCommand bool

func mkRunE(c *Command, f runFunction) func(*cobra.Command, []string) error {
return func(cmd *cobra.Command, args []string) error {
// The init work below should only happen once per cmd/cue invocation;
// if it happens twice, we'll misbehave by writing stats twice
// or miscalculating pprof and stats numbers.
if hasRunCommand {
panic("cmd/cue/cmd.mkRunE init ran twice")
}
hasRunCommand = true

c.Command = cmd

statsEnc, err := statsEncoder(c)
Expand Down
4 changes: 1 addition & 3 deletions cmd/cue/cmd/testdata/script/pprof.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ exists cpu.out
exists mem.out

# The flags should still work with 'cue cmd', which runs a command within a command.
# TODO: this is currently broken, as we try to start pprof twice.
rm cpu.out
! exec cue cmd --cpuprofile=cpu.out hello
stderr 'cpu profiling already in use'
exec cue cmd --cpuprofile=cpu.out hello
go tool pprof -top cpu.out
stdout 'Type: cpu'

Expand Down

0 comments on commit 3f84ba6

Please sign in to comment.