Skip to content

Commit

Permalink
cmd/cue: rip out cue.Instance and cue.Merge from 'cue cmd'
Browse files Browse the repository at this point in the history
As can be seen by the cmd_many.txtar test just added,
we still support the scenario where 'cue cmd' is given multiple packages
by unifying all of their values together, rather than using cue.Merge.

Other than cue.Merge having been deprecated with DO NOT USE warnings
since early 2021, it seemed to have had somewhat odd semantics.
Structs in CUE are open by default, so unifying {} with {b: "b"}
should result in {b: "b"} without any issue, as we do now.

The merge semantics, which aren't documented and deprecated, disagree.
However, the passing test mimicking what a few users have reported using
still passes just as it did before, and the new semantics are now
well understood - if not documented as such yet - so this seems fine.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ia3195643612f629ac61a36f8469945ee345e0719
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193718
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Paul Jolly <paul@myitcv.io>
  • Loading branch information
mvdan committed Apr 24, 2024
1 parent 994a15d commit 7422d00
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 76 deletions.
75 changes: 12 additions & 63 deletions cmd/cue/cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import (
"cuelang.org/go/internal/core/adt"
"cuelang.org/go/internal/encoding"
"cuelang.org/go/internal/filetypes"
"cuelang.org/go/internal/value"
)

var requestedVersion = os.Getenv("CUE_SYNTAX_OVERRIDE")
Expand Down Expand Up @@ -765,80 +764,30 @@ func buildInstances(cmd *Command, binst []*build.Instance, ignoreErrors bool) []
return insts
}

func buildToolInstances(binst []*build.Instance) ([]*cue.Instance, error) {
instances := cue.Build(binst)
for _, inst := range instances {
if inst.Err != nil {
return nil, inst.Err
}
}

// TODO check errors after the fact in case of ignore.
for _, inst := range instances {
if err := inst.Value().Validate(); err != nil {
return nil, err
}
}
return instances, nil
}

func buildTools(cmd *Command, args []string) (*cue.Instance, error) {
func buildTools(cmd *Command, args []string) (cue.Value, error) {
cfg, err := defaultConfig()
if err != nil {
return nil, err
return cue.Value{}, err
}
loadCfg := *cfg.loadCfg
loadCfg.Tools = true
f := cmd.cmd.Flags()
setTags(&loadCfg, f, cmd.cmd.Parent().Flags())

binst := loadFromArgs(args, &loadCfg)
if len(binst) == 0 {
return nil, nil
}
included := map[string]bool{}

ti := binst[0].Context().NewInstance(binst[0].Root, nil)
for _, inst := range binst {
k := 0
for _, f := range inst.Files {
if strings.HasSuffix(f.Filename, "_tool.cue") {
if !included[f.Filename] {
_ = ti.AddSyntax(f)
included[f.Filename] = true
}
continue
}
inst.Files[k] = f
k++
}
inst.Files = inst.Files[:k]
}
builds := loadFromArgs(args, &loadCfg)

insts, err := buildToolInstances(binst)
values, err := cmd.ctx.BuildInstances(builds)
if err != nil {
return nil, err
return cue.Value{}, err
}

inst := insts[0]
if len(insts) > 1 {
inst = cue.Merge(insts...)
// 'cue cmd' with multiple package arguments, such as './...',
// currently runs a single command on the unification of all given packages.
// See https://cuelang.org/issue/1325.
v := values[0]
for _, v2 := range values[1:] {
v = v.Unify(v2)
}

r := value.ConvertToRuntime(inst.Value().Context())
for _, b := range binst {
for _, i := range b.Imports {
if _, err := r.Build(i); err != nil {
return nil, err
}
}
}

// Set path equal to the package from which it is loading.
ti.ImportPath = binst[0].ImportPath

inst = inst.Build(ti)
return inst, inst.Err
return v, nil
}

func shortFile(root string, f *build.File) string {
Expand Down
14 changes: 5 additions & 9 deletions cmd/cue/cmd/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import (
const commandSection = "command"

func lookupString(obj cue.Value, key, def string) string {
str, err := obj.Lookup(key).String()
str, err := obj.LookupPath(cue.MakePath(cue.Str(key))).String()
if err == nil {
def = str
}
Expand All @@ -62,8 +62,8 @@ func splitLine(s string) (line, tail string) {
// addCustomCommands iterates over all commands defined under field typ
// and adds them as cobra subcommands to cmd.
// The func is only used in `cue help cmd`, which doesn't show errors.
func addCustomCommands(c *Command, cmd *cobra.Command, typ string, tools *cue.Instance) {
commands := tools.Lookup(typ)
func addCustomCommands(c *Command, cmd *cobra.Command, typ string, tools cue.Value) {
commands := tools.LookupPath(cue.MakePath(cue.Str(typ)))
if !commands.Exists() {
return
}
Expand All @@ -80,11 +80,7 @@ func addCustomCommands(c *Command, cmd *cobra.Command, typ string, tools *cue.In
}

// customCommand creates a cobra.Command out of a CUE command definition.
func customCommand(c *Command, typ, name string, tools *cue.Instance) (*cobra.Command, error) {
if tools == nil {
return nil, errors.New("no commands defined")
}

func customCommand(c *Command, typ, name string, tools cue.Value) (*cobra.Command, error) {
// TODO: validate allowing incomplete.
o := tools.Lookup(typ, name)
if !o.Exists() {
Expand Down Expand Up @@ -149,7 +145,7 @@ func customCommand(c *Command, typ, name string, tools *cue.Instance) (*cobra.Co
return sub, nil
}

func doTasks(cmd *Command, typ, command string, root *cue.Instance) error {
func doTasks(cmd *Command, typ, command string, root cue.Value) error {
cfg := &flow.Config{
Root: cue.MakePath(cue.Str(commandSection), cue.Str(command)),
InferTasks: true,
Expand Down
7 changes: 3 additions & 4 deletions cmd/cue/cmd/testdata/script/cmd_many.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ mv itemsList_tool.cue itemsList.cue
exec cue export ./...
cmp stdout stdout.export-post.golden

# Attempt to ls in this state, which fails, as each of the CUE packages
# hold an items list with different lengths.
! exec cue cmd ls ./...
stderr 'itemsList: incompatible list lengths \(0 and 1\)'
# Verify that cue cmd ls ./... still works the same.
exec cue cmd ls ./...
cmp stdout stdout.ls-pre.golden

-- cue.mod/module.cue --
module: "mod.com"
Expand Down

0 comments on commit 7422d00

Please sign in to comment.