Skip to content

Commit

Permalink
cue/interpreter/embed: fix handling of dir matching glob
Browse files Browse the repository at this point in the history
Currently, the MVP embed implementation sees @embed(glob=dir/*) and
tries to embed dir/subdir. However '*' should only match files, and so
subdir should be skipped.

This CL fixes the implementation.

Per the design in https://cuelang.org/discussion/3264, we will likely
add support for '**' to match 0 or more path elements later. For now the
use of '**' results in an error, but we also update that error message
to indicate that we don't "yet" support '**'.

Fixes #3267.

Signed-off-by: Paul Jolly <[email protected]>
Change-Id: I6e17aaf4e39b1b794951a9e934b7dd47e052ad4c
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1197338
Reviewed-by: Marcel van Lohuizen <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
myitcv committed Jul 8, 2024
1 parent a0f2cac commit 970d10f
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 8 deletions.
5 changes: 3 additions & 2 deletions cmd/cue/cmd/testdata/script/embed.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,14 @@ b: _ @embed(file="input.yaml")

c: _ @embed(file="test.json", type=text)

d: _ @embed(glob="y/*.*", type=yaml)
d: _ @embed(glob="y/*", type=yaml)

d: _ @embed(glob="x/*.yaml") // merge into the same map

f: _ @embed(file="openapi.json", type=openapi)

g: _ @embed(file="openapi.json") // test no auto mode!


special: {
// These are all valid.
underscoreFile: _ @embed(file="y/_test.json")
Expand Down Expand Up @@ -61,6 +60,8 @@ a1: 2
{ "z": 47 }
-- .y/test.json --
{ "z": 48 }
-- y/subdir/another.json --
{ "z": 49 }
-- _z/test.json --
-- x/input.yaml --
a1: 2
Expand Down
4 changes: 1 addition & 3 deletions cmd/cue/cmd/testdata/script/embed_err.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,13 @@ x: 5
./test.cue:11:8
@embed: open y/test.json: no such file or directory:
./test.cue:13:13
@embed: cannot embed directories:
./test.cue:15:8
@embed: path not normalized, use "x" instead:
./test.cue:17:13
@embed: cannot refer to parent directory:
./test.cue:19:15
@embed: only relative files are allowed:
./test.cue:21:12
@embed: double star not supported in glob:
@embed: double star not (yet) supported in glob:
./test.cue:23:15
@embed: streaming not implemented: found more than one value in file:
./test.cue:25:11
Expand Down
15 changes: 12 additions & 3 deletions cue/interpreter/embed/embed.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ type compiler struct {

// file system cache
dir string
fs fs.FS
fs fs.StatFS
pos token.Pos
}

Expand Down Expand Up @@ -178,7 +178,7 @@ func (c *compiler) Compile(funcName string, scope adt.Value, a *internal.Attr) (
// TODO: obtain a fs.FS from load or something similar.
dir := filepath.Dir(pos.File().Name())
if c.dir != dir {
c.fs = os.DirFS(dir)
c.fs = os.DirFS(dir).(fs.StatFS) // Documented as implementing fs.StatFS
c.dir = dir
}

Expand Down Expand Up @@ -213,7 +213,7 @@ func (c *compiler) processGlob(glob, scope string, schema adt.Value) (adt.Expr,
}

if strings.Contains(glob, "**") {
return nil, errors.Newf(c.pos, "double star not supported in glob")
return nil, errors.Newf(c.pos, "double star not (yet) supported in glob")
}

m := &adt.StructLit{}
Expand All @@ -224,6 +224,15 @@ func (c *compiler) processGlob(glob, scope string, schema adt.Value) (adt.Expr,
}

for _, f := range matches {
// TODO: lots of stat calls happening in this MVP so another won't hurt.
// We don't support '**' initially, and '*' only matches files, so skip
// any directories.
if fi, err := c.fs.Stat(f); err != nil {
return nil, errors.Newf(c.pos, "failed to stat %s: %v", f, err)
} else if fi.IsDir() {
continue
}

expr, err := c.decodeFile(f, scope, schema)
if err != nil {
return nil, err
Expand Down

0 comments on commit 970d10f

Please sign in to comment.