Skip to content

Commit

Permalink
cue/load: provide reason for exclusion of files
Browse files Browse the repository at this point in the history
Fixes #741
Issue #52

Change-Id: I8b61262be1fec41cdafd5ff78ee096a6dd6893fd
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9682
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
  • Loading branch information
mpvl committed May 5, 2021
1 parent 975ba50 commit 1f1c3f6
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 42 deletions.
3 changes: 2 additions & 1 deletion cmd/cue/cmd/testdata/script/eval_hiddenfail.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
cmp stderr expect-stderr

-- expect-stderr --
build constraints exclude all CUE files in . (ignored: .foo.cue)
build constraints exclude all CUE files in .:
.foo.cue: filename starts with a '.'
-- .foo.cue --
package foo

Expand Down
3 changes: 2 additions & 1 deletion cmd/cue/cmd/testdata/script/issue174.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
! cue export ./issue174
cmp stderr expect-stderr
-- expect-stderr --
build constraints exclude all CUE files in ./issue174 (ignored: issue174/issue174.cue)
build constraints exclude all CUE files in ./issue174:
issue174/issue174.cue: no package name
-- issue174/issue174.cue --
import 'foo'

Expand Down
5 changes: 4 additions & 1 deletion cue/build/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package build

import "cuelang.org/go/cue/errors"

// A File represents a file that is part of the build process.
type File struct {
Filename string `json:"filename"`
Expand All @@ -23,7 +25,8 @@ type File struct {
Form Form `json:"form,omitempty"`
Tags map[string]string `json:"tags,omitempty"` // code=go

Source interface{} `json:"-"` // TODO: swap out with concrete type.
ExcludeReason errors.Error `json:"-"`
Source interface{} `json:"-"` // TODO: swap out with concrete type.
}

// A Encoding indicates a file format for representing a program.
Expand Down
8 changes: 8 additions & 0 deletions cue/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,14 @@ func (e *wrapped) Error() string {
}
}

func (e *wrapped) Is(target error) bool {
return Is(e.main, target)
}

func (e *wrapped) As(target interface{}) bool {
return As(e.main, target)
}

func (e *wrapped) Msg() (format string, args []interface{}) {
return e.main.Msg()
}
Expand Down
21 changes: 11 additions & 10 deletions cue/load/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,20 @@ func (e *NoFilesError) Error() string {
path := e.Package.DisplayPath

if len(e.Package.IgnoredFiles) > dummy {
b := strings.Builder{}
b.WriteString("build constraints exclude all CUE files in ")
b.WriteString(path)
b.WriteString(":")
// CUE files exist, but they were ignored due to build constraints.
msg := "build constraints exclude all CUE files in " + path + " (ignored: "
var files []string
for i, f := range e.Package.IgnoredFiles {
if i == 4 {
files = append(files[:4], "...")
break
for _, f := range e.Package.IgnoredFiles {
b.WriteString("\n ")
b.WriteString(filepath.ToSlash(e.Package.RelPath(f)))
if f.ExcludeReason != nil {
b.WriteString(": ")
b.WriteString(f.ExcludeReason.Error())
}
files = append(files, filepath.ToSlash(e.Package.RelPath(f)))
}
msg += strings.Join(files, ", ")
msg += ")"
return msg
return b.String()
}
// if len(e.Package.TestCUEFiles) > 0 {
// // Test CUE files exist, but we're not interested in them.
Expand Down
37 changes: 28 additions & 9 deletions cue/load/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ func (l *loader) importPkg(pos token.Pos, p *build.Instance) []*build.Instance {
file, err := filetypes.ParseFile(f.Name(), filetypes.Input)
if err != nil {
p.UnknownFiles = append(p.UnknownFiles, &build.File{
Filename: f.Name(),
Filename: f.Name(),
ExcludeReason: errors.Newf(token.NoPos, "unknown filetype"),
})
continue // skip unrecognized file types
}
Expand Down Expand Up @@ -356,21 +357,31 @@ func (fp *fileProcessor) add(pos token.Pos, root string, file *build.File, mode
// badFile := func(p *build.Instance, err errors.Error) bool {
badFile := func(err errors.Error) bool {
fp.err = errors.Append(fp.err, err)
file.ExcludeReason = fp.err
p.InvalidFiles = append(p.InvalidFiles, file)
return true
}

match, data, err := matchFile(fp.c, file, true, fp.allFiles, fp.allTags)
if err != nil {
switch {
case match:

case err == nil:
// Not a CUE file.
p.OrphanedFiles = append(p.OrphanedFiles, file)
return false

case !errors.Is(err, errExclude):
return badFile(err)
}
if !match {
if file.Encoding == build.CUE && file.Interpretation == "" {

default:
file.ExcludeReason = err
if file.Interpretation == "" {
p.IgnoredFiles = append(p.IgnoredFiles, file)
} else {
p.OrphanedFiles = append(p.OrphanedFiles, file)
}
return false // don't mark as added
return false
}

pf, perr := parser.ParseFile(fullPath, data, parser.ImportsOnly, parser.ParseComments)
Expand All @@ -379,7 +390,7 @@ func (fp *fileProcessor) add(pos token.Pos, root string, file *build.File, mode
return true
}

_, pkg, _ := internal.PackageInfo(pf)
_, pkg, pos := internal.PackageInfo(pf)
if pkg == "" {
pkg = "_"
}
Expand All @@ -405,15 +416,17 @@ func (fp *fileProcessor) add(pos token.Pos, root string, file *build.File, mode
case pkg != "_":

default:
file.ExcludeReason = excludeError{errors.Newf(pos, "no package name")}
p.IgnoredFiles = append(p.IgnoredFiles, file)
return false // don't mark as added
}

if !fp.c.AllCUEFiles {
if include, err := shouldBuildFile(pf, fp); !include {
if err != nil {
if err := shouldBuildFile(pf, fp); err != nil {
if !errors.Is(err, errExclude) {
fp.err = errors.Append(fp.err, err)
}
file.ExcludeReason = err
p.IgnoredFiles = append(p.IgnoredFiles, file)
return false
}
Expand All @@ -425,6 +438,8 @@ func (fp *fileProcessor) add(pos token.Pos, root string, file *build.File, mode
fp.firstFile = base
} else if pkg != p.PkgName {
if fp.ignoreOther {
file.ExcludeReason = excludeError{errors.Newf(pos,
"package is %s, want %s", pkg, p.PkgName)}
p.IgnoredFiles = append(p.IgnoredFiles, file)
return false
}
Expand Down Expand Up @@ -478,12 +493,16 @@ func (fp *fileProcessor) add(pos token.Pos, root string, file *build.File, mode
if fp.c.loader.cfg.Tests {
p.BuildFiles = append(p.BuildFiles, file)
} else {
file.ExcludeReason = excludeError{errors.Newf(pos,
"_test.cue files excluded in non-test mode")}
p.IgnoredFiles = append(p.IgnoredFiles, file)
}
case isTool:
if fp.c.loader.cfg.Tools {
p.BuildFiles = append(p.BuildFiles, file)
} else {
file.ExcludeReason = excludeError{errors.Newf(pos,
"_tool.cue files excluded in non-cmd mode")}
p.IgnoredFiles = append(p.IgnoredFiles, file)
}
default:
Expand Down
13 changes: 10 additions & 3 deletions cue/load/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ display:`,
cfg: dirCfg,
args: args("./anon"),
want: `
err: build constraints exclude all CUE files in ./anon (ignored: anon/anon.cue)
err: build constraints exclude all CUE files in ./anon:
anon/anon.cue: no package name
path: example.org/test/anon
module: example.org/test
root: $CWD/testdata
Expand Down Expand Up @@ -154,7 +155,10 @@ imports:
cfg: dirCfg,
args: args("example.org/test/hello:nonexist"),
want: `
err: build constraints exclude all CUE files in example.org/test/hello:nonexist (ignored: anon.cue, test.cue, hello/test.cue)
err: build constraints exclude all CUE files in example.org/test/hello:nonexist:
anon.cue: no package name
test.cue: package is test, want nonexist
hello/test.cue: package is test, want nonexist
path: example.org/test/hello:nonexist
module: example.org/test
root: $CWD/testdata
Expand Down Expand Up @@ -247,7 +251,10 @@ files:
},
args: args("./toolonly"),
want: `
err: build constraints exclude all CUE files in ./toolonly (ignored: anon.cue, test.cue, toolonly/foo_tool.cue)
err: build constraints exclude all CUE files in ./toolonly:
anon.cue: no package name
test.cue: package is test, want foo
toolonly/foo_tool.cue: _tool.cue files excluded in non-cmd mode
path: example.org/test/toolonly:foo
module: example.org/test
root: $CWD/testdata
Expand Down
32 changes: 21 additions & 11 deletions cue/load/match.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ import (
"cuelang.org/go/cue/token"
)

var errExclude = errors.New("file rejected")

type cueError = errors.Error
type excludeError struct {
cueError
}

func (e excludeError) Is(err error) bool { return err == errExclude }

// matchFile determines whether the file with the given name in the given directory
// should be included in the package being constructed.
// It returns the data read from the file.
Expand All @@ -42,7 +51,7 @@ func matchFile(cfg *Config, file *build.File, returnImports, allFiles bool, allT
}

if file.Encoding != build.CUE {
return
return false, nil, nil // not a CUE file, don't record.
}

if file.Filename == "-" {
Expand All @@ -52,32 +61,33 @@ func matchFile(cfg *Config, file *build.File, returnImports, allFiles bool, allT
return
}
file.Source = b
data = b
match = true // don't check shouldBuild for stdin
return
return true, b, nil // don't check shouldBuild for stdin
}

name := filepath.Base(file.Filename)
if !cfg.filesMode && strings.HasPrefix(name, ".") {
return
return false, nil, &excludeError{
errors.Newf(token.NoPos, "filename starts with a '.'"),
}
}

if strings.HasPrefix(name, "_") {
return
return false, nil, &excludeError{
errors.Newf(token.NoPos, "filename starts with a '_"),
}
}

f, err := cfg.fileSystem.openFile(file.Filename)
if err != nil {
return
return false, nil, err
}

data, err = readImports(f, false, nil)
f.Close()
if err != nil {
err = errors.Newf(token.NoPos, "read %s: %v", file.Filename, err)
return
return false, nil,
errors.Newf(token.NoPos, "read %s: %v", file.Filename, err)
}

match = true
return
return true, data, nil
}
15 changes: 9 additions & 6 deletions cue/load/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,22 +305,22 @@ func injectTags(tags []string, l *loader) errors.Error {

// shouldBuildFile determines whether a File should be included based on its
// attributes.
func shouldBuildFile(f *ast.File, fp *fileProcessor) (bool, errors.Error) {
func shouldBuildFile(f *ast.File, fp *fileProcessor) errors.Error {
tags := fp.c.Tags

a, errs := getBuildAttr(f)
if errs != nil {
return false, errs
return errs
}
if a == nil {
return true, nil
return nil
}

_, body := a.Split()

expr, err := parser.ParseExpr("", body)
if err != nil {
return false, errors.Promote(err, "")
return errors.Promote(err, "")
}

tagMap := map[string]bool{}
Expand All @@ -331,9 +331,12 @@ func shouldBuildFile(f *ast.File, fp *fileProcessor) (bool, errors.Error) {
c := checker{tags: tagMap, loader: fp.c.loader}
include := c.shouldInclude(expr)
if c.err != nil {
return false, c.err
return c.err
}
return include, nil
if !include {
return excludeError{errors.Newf(a.Pos(), "@if(%s) did not match", body)}
}
return nil
}

func getBuildAttr(f *ast.File) (*ast.Attribute, errors.Error) {
Expand Down

0 comments on commit 1f1c3f6

Please sign in to comment.