cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.14k stars 294 forks source link

cmd/cue: _tool.cue files are included in dependencies but should not be #3256

Open rogpeppe opened 4 months ago

rogpeppe commented 4 months ago

*_tool.cue files are intended to contain commands for cue cmd - they make sense only when the package is intended to be run as an explicit argument to cue cmd.

However, currently, when cue cmd is running, all _tool.cue files in any dependency are merged into their respective packages regardless of whether those packages are explicitly named on the command line.

Originally, the packages in tool/... were not available to import in non tools files, so it was necessary to put any tools helper code inside _tool.cue files too, but that restriction hasn't been the case for a long time now.

We propose that _tool.cue files should be excluded from all packages except those explicitly mentioned on the cue cmd command line.

myitcv commented 3 months ago

@rogpeppe was this done as part of the work for ignoring _test.cue files?

rogpeppe commented 3 months ago

Here's a testscript that demonstrates the issue:

exec cue cmd test
cmp stdout want-stdout

-- want-stdout --
{"normal":true}
-- cue.mod/module.cue --
module: "cue.example/foo"
language: version: "v0.9.2"

-- foo_tool.cue --
package foo

import (
    "tool/cli"
    "encoding/json"

    "cue.example/foo/bar"
)

command: test: {
    print: cli.Print & {
        text: json.Marshal(bar)
    }
}

-- bar/bar.cue --
package bar

normal: true

-- bar/bar_tool.cue --
package bar

tool: true

As this is a backwards incompatible change, if/when we fix this, we could gate the changed behaviour on the declared language version.

mvdan commented 3 months ago

As Roger said in the last comment, this is a potentially breaking change that we want to do carefully in an alpha release, so I've pushed it back slightly to v0.11.0-alpha.1.