Skip to content

Commit ea70704

Browse files
committed
interpreter/embed: do not require CUE_EXPERIMENT=embed
In general, if someone is using `embed.New`, they are explicitly asking to enable embedding, so there should be no need to additionally set CUE_EXPERIMENT. This means that we do need to explicitly check `CUE_EXPERIMENT` inside `cmd/cue` to decide whether to include the embed interpreter. Take the opportunity to remove the mutable `rootContextOptions` variable, avoiding appending to it each time `cmd.New` is called, and moving the `Command.ctx` initialization to `mkRunE` where we've already invoked `cueexperiment.Init`. Also change `internal/core/runtime` to always error if an unknown interpreter is mentioned in an `@extern` attribute. This code path was not encountered previously because `cmd/cue` always added at least one interpreter (embed), but seems to make more sense because in general `@extern` attributes should mean something and if there's no interpreter found, it probably signals a real problem. Signed-off-by: Roger Peppe <[email protected]> Change-Id: Id52402cf8e2d7fc0965e38b126c4058338614508 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202903 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Daniel Martí <[email protected]>
1 parent e58994d commit ea70704

File tree

5 files changed

+18
-34
lines changed

5 files changed

+18
-34
lines changed

cmd/cue/cmd/root.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ import (
5555

5656
type runFunction func(cmd *Command, args []string) error
5757

58+
// wasmInterp is set when the cuewasm build tag is enbabled.
59+
var wasmInterp cuecontext.ExternInterpreter
60+
5861
func statsEncoder(cmd *Command) (*encoding.Encoder, error) {
5962
file := os.Getenv("CUE_STATS_FILE")
6063
if file == "" {
@@ -105,6 +108,14 @@ func mkRunE(c *Command, f runFunction) func(*cobra.Command, []string) error {
105108
if err := cuedebug.Init(); err != nil {
106109
return err
107110
}
111+
var opts []cuecontext.Option
112+
if wasmInterp != nil {
113+
opts = append(opts, cuecontext.Interpreter(wasmInterp))
114+
}
115+
if cueexperiment.Flags.Embed {
116+
opts = append(opts, cuecontext.Interpreter(embed.New()))
117+
}
118+
c.ctx = cuecontext.New(opts...)
108119
// Some init work, such as in internal/filetypes, evaluates CUE by design.
109120
// We don't want that work to count towards $CUE_STATS.
110121
adt.ResetStats()
@@ -198,13 +209,9 @@ func New(args []string) (*Command, error) {
198209
DisableSuggestions: true,
199210
}
200211

201-
embedding := cuecontext.Interpreter(embed.New())
202-
rootContextOptions = append(rootContextOptions, embedding)
203-
204212
c := &Command{
205213
Command: cmd,
206214
root: cmd,
207-
ctx: cuecontext.New(rootContextOptions...),
208215
}
209216
c.cmdCmd = newCmdCmd(c)
210217

@@ -252,8 +259,6 @@ func New(args []string) (*Command, error) {
252259
return c, nil
253260
}
254261

255-
var rootContextOptions []cuecontext.Option
256-
257262
// rootWorkingDir avoids repeated calls to [os.Getwd] in cmd/cue.
258263
// If we can't figure out the current directory, something is very wrong,
259264
// and there's no point in continuing to run a command.

cmd/cue/cmd/root_cuewasm.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,11 @@
1717
package cmd
1818

1919
import (
20-
"cuelang.org/go/cue/cuecontext"
2120
"cuelang.org/go/cue/interpreter/wasm"
2221
)
2322

24-
// The wasm interpreter can be enabled by default once we are ready to ship the feature.
25-
// For now, it's not ready, and makes cue binaries heavier by over 2MiB.
2623
func init() {
27-
rootContextOptions = append(rootContextOptions, cuecontext.Interpreter(wasm.New()))
24+
// The wasm interpreter can be enabled by default once we are ready to ship the feature.
25+
// For now, it's not ready, and makes cue binaries heavier by over 2MiB.
26+
wasmInterp = wasm.New()
2827
}

cmd/cue/cmd/testdata/script/embed.txtar

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
exec cue eval
2-
cmp stdout out/noembed
1+
! exec cue eval
2+
cmp stderr out/noembed
33

44
env CUE_EXPERIMENT=embed
55

@@ -108,19 +108,8 @@ language: version: "v0.9.0"
108108
}
109109
}
110110
-- out/noembed --
111-
a: _
112-
b: _
113-
c: _
114-
d: _
115-
f: _
116-
g: _
117-
special: {
118-
underscoreFile: _
119-
dotFile: _
120-
dotdotFile: _
121-
underscoreDir: _
122-
dotDir: _
123-
}
111+
no interpreter defined for "embed":
112+
./test.cue:1:1
124113
-- out/eval --
125114
a: {
126115
x: 34

cue/interpreter/embed/embed.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ import (
9696
"cuelang.org/go/internal"
9797
"cuelang.org/go/internal/core/adt"
9898
"cuelang.org/go/internal/core/runtime"
99-
"cuelang.org/go/internal/cueexperiment"
10099
"cuelang.org/go/internal/encoding"
101100
"cuelang.org/go/internal/filetypes"
102101
"cuelang.org/go/internal/value"
@@ -149,11 +148,6 @@ type compiler struct {
149148
// (@embed(file=...)) or a glob of files (@embed(glob=...)).
150149
// and decodes the given files.
151150
func (c *compiler) Compile(funcName string, scope adt.Value, a *internal.Attr) (adt.Expr, errors.Error) {
152-
// This is a really weird spot to disable embedding, but I could not get
153-
// the wasm tests to pass without doing it like this.
154-
if !cueexperiment.Flags.Embed {
155-
return &adt.Top{}, nil
156-
}
157151

158152
file, _, err := a.Lookup(0, "file")
159153
if err != nil {

internal/core/runtime/extern.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ type Compiler interface {
6161
// injectImplementations modifies v to include implementations of functions
6262
// for fields associated with the @extern attributes.
6363
func (r *Runtime) injectImplementations(b *build.Instance, v *adt.Vertex) (errs errors.Error) {
64-
if r.interpreters == nil {
65-
return nil
66-
}
6764

6865
d := &externDecorator{
6966
runtime: r,

0 commit comments

Comments
 (0)