this repo has no description
cue/load: better treatment of files specified on the command line
Currently in modules mode, imports in files specified on the command
line are not added to the set of root packages.
Also, the entire module is always considered even when a file
specified on the command line doesn't use any of it.
This CL addresses these issues by walking the imports
of files explicitly specified on the command line,
and by avoiding loading all module imports when there
are only files specified on the command line.
This is not a complete solution: it is also desirable that
we walk the dependencies of only the packages and files
that are explicitly mentioned on the command line,
but doing that would require a considerable refactor
and for now, it is likely that mixing CUE files and packages
together on the command line is somewhat rare,
so walking all module imports whenever a package is
specified on the command line should be sufficient.
Fixes #3144
Fixes #3147
Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: Ie6236aeadfb14fc891eec2f4e6a905c7b37583bb
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194765
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
+145
-31
+1
-1
cmd/cue/cmd/script_test.go
+1
-1
cmd/cue/cmd/script_test.go
+22
cmd/cue/cmd/testdata/script/issue3144.txtar
+22
cmd/cue/cmd/testdata/script/issue3144.txtar
···
1
+
# Check that unrelated invalid files do not cause an error when only files
2
+
# are specified on the command line.
3
+
4
+
env CUE_EXPERIMENT=modules
5
+
env CUE_CACHE_DIR=$WORK/tmp/cache
6
+
7
+
exec cue export valid.cue
8
+
cmp stdout stdout.golden
9
+
10
+
-- stdout.golden --
11
+
{
12
+
"out": "bar"
13
+
}
14
+
-- cue.mod/module.cue --
15
+
module: "foo.test/bar"
16
+
language: version: "v0.9.0"
17
+
-- valid.cue --
18
+
#foo: "bar"
19
+
out: #foo
20
+
-- invalid.cue --
21
+
foo :: "bar"
22
+
out: foo
+27
cmd/cue/cmd/testdata/script/issue3147.txtar
+27
cmd/cue/cmd/testdata/script/issue3147.txtar
···
1
+
# Check that imports from files specified on the command line
2
+
# are still considered even when they are not part of the main module.
3
+
4
+
env CUE_EXPERIMENT=modules
5
+
env CUE_CACHE_DIR=$WORK/tmp/cache
6
+
exec cue export _foo/x.cue
7
+
cmp stdout stdout.golden
8
+
9
+
-- cue.mod/module.cue --
10
+
module: "mod.com@v0"
11
+
language: {
12
+
version: "v0.9.0"
13
+
}
14
+
-- cue.mod/pkg/example.com/banana/banana.cue --
15
+
package banana
16
+
17
+
X: 42
18
+
-- _foo/x.cue --
19
+
package p
20
+
21
+
import "example.com/banana"
22
+
23
+
x: banana.X
24
+
-- stdout.golden --
25
+
{
26
+
"x": 42
27
+
}
+27
-1
cue/load/import.go
+27
-1
cue/load/import.go
···
17
17
import (
18
18
"cmp"
19
19
"fmt"
20
+
"io"
20
21
"os"
21
22
pathpkg "path"
22
23
"path/filepath"
···
229
230
})
230
231
continue // skip unrecognized file types
231
232
}
232
-
233
233
sd.buildFiles = append(sd.buildFiles, file)
234
234
}
235
235
return sd
236
+
}
237
+
238
+
func setFileSource(cfg *Config, f *build.File) error {
239
+
if f.Source != nil {
240
+
return nil
241
+
}
242
+
fullPath := f.Filename
243
+
if fullPath == "-" {
244
+
b, err := io.ReadAll(cfg.stdin())
245
+
if err != nil {
246
+
return errors.Newf(token.NoPos, "read stdin: %v", err)
247
+
}
248
+
f.Source = b
249
+
return nil
250
+
}
251
+
if !filepath.IsAbs(fullPath) {
252
+
fullPath = filepath.Join(cfg.Dir, fullPath)
253
+
}
254
+
if fi := cfg.fileSystem.getOverlay(fullPath); fi != nil {
255
+
if fi.file != nil {
256
+
f.Source = fi.file
257
+
} else {
258
+
f.Source = fi.contents
259
+
}
260
+
}
261
+
return nil
236
262
}
237
263
238
264
func (l *loader) loadFunc(pos token.Pos, path string) *build.Instance {
+61
-11
cue/load/instances.go
+61
-11
cue/load/instances.go
···
17
17
import (
18
18
"context"
19
19
"fmt"
20
+
"sort"
21
+
"strconv"
20
22
21
23
"cuelang.org/go/cue/ast"
22
24
"cuelang.org/go/cue/build"
···
66
68
if err != nil {
67
69
return []*build.Instance{c.newErrInstance(err)}
68
70
}
71
+
for _, f := range otherFiles {
72
+
if err := setFileSource(c, f); err != nil {
73
+
return []*build.Instance{c.newErrInstance(err)}
74
+
}
75
+
}
69
76
synCache := newSyntaxCache(c)
70
-
71
77
// Pass all arguments that look like packages to loadPackages
72
78
// so that they'll be available when looking up the packages
73
79
// that are specified on the command line.
74
80
// Relative import paths create a package with an associated
75
81
// error but it turns out that's actually OK because the cue/load
76
82
// logic resolves such paths without consulting pkgs.
77
-
pkgs, err := loadPackages(ctx, c, pkgArgs)
83
+
pkgs, err := loadPackages(ctx, c, synCache, pkgArgs, otherFiles)
78
84
if err != nil {
79
-
return []*build.Instance{c.newErrInstance(err)}
85
+
return []*build.Instance{c.newErrInstance(fmt.Errorf("xxx: %v", err))}
80
86
}
81
87
tg := newTagger(c)
82
88
l := newLoader(c, tg, synCache, pkgs)
···
140
146
return a
141
147
}
142
148
143
-
func loadPackages(ctx context.Context, cfg *Config, extraPkgs []string) (*modpkgload.Packages, error) {
149
+
// loadPackages returns packages loaded from the given package list and also
150
+
// including imports from the given build files.
151
+
func loadPackages(ctx context.Context, cfg *Config, synCache *syntaxCache, extraPkgs []string, otherFiles []*build.File) (*modpkgload.Packages, error) {
144
152
if cfg.Registry == nil || cfg.modFile == nil || cfg.modFile.Module == "" {
145
153
return nil, nil
146
154
}
···
154
162
FS: cfg.fileSystem.ioFS(cfg.ModuleRoot),
155
163
Dir: ".",
156
164
}
157
-
allImports, err := modimports.AllImports(modimports.AllModuleFiles(mainModLoc.FS, mainModLoc.Dir))
158
-
if err != nil {
159
-
return nil, fmt.Errorf("cannot enumerate all module imports: %v", err)
165
+
pkgPaths := make(map[string]bool)
166
+
// Add any packages specified directly on the command line.
167
+
for _, pkg := range extraPkgs {
168
+
pkgPaths[pkg] = true
160
169
}
161
-
// Add any packages specified on the command line so they're always
162
-
// available.
163
-
allImports = append(allImports, extraPkgs...)
170
+
if len(otherFiles) == 0 || len(extraPkgs) > 0 {
171
+
// Resolve all the imports in the current module. We specifically
172
+
// avoid doing this when files are specified on the command line
173
+
// but no packages because we don't want to fail because of
174
+
// invalid files in the module when we're just evaluating files.
175
+
// TODO this means that if files _and_ packages are specified,
176
+
// we can still error when there are problems with packages that
177
+
// aren't used by anything explicitly specified on the command line;
178
+
// a proper solution would involve pushing the pattern evaluation
179
+
// down into the module loading code, but that implies a significant
180
+
// refactoring of the modules code, so this will do for now.
181
+
modImports, err := modimports.AllImports(modimports.AllModuleFiles(mainModLoc.FS, mainModLoc.Dir))
182
+
if err != nil {
183
+
return nil, fmt.Errorf("cannot enumerate all module imports: %v", err)
184
+
}
185
+
for _, pkg := range modImports {
186
+
pkgPaths[pkg] = true
187
+
}
188
+
}
189
+
// Add any imports found in other files.
190
+
for _, f := range otherFiles {
191
+
syntaxes, err := synCache.getSyntax(f)
192
+
if err != nil {
193
+
return nil, fmt.Errorf("cannot get syntax for %q: %v", f.Filename, err)
194
+
}
195
+
for _, syntax := range syntaxes {
196
+
for _, imp := range syntax.Imports {
197
+
pkgPath, err := strconv.Unquote(imp.Path.Value)
198
+
if err != nil {
199
+
// Should never happen.
200
+
return nil, fmt.Errorf("invalid import path %q in %s", imp.Path.Value, f.Filename)
201
+
}
202
+
// Canonicalize the path.
203
+
pkgPath = module.ParseImportPath(pkgPath).Canonical().String()
204
+
pkgPaths[pkgPath] = true
205
+
}
206
+
}
207
+
}
208
+
// TODO use maps.Keys when we can.
209
+
pkgPathSlice := make([]string, 0, len(pkgPaths))
210
+
for p := range pkgPaths {
211
+
pkgPathSlice = append(pkgPathSlice, p)
212
+
}
213
+
sort.Strings(pkgPathSlice)
164
214
return modpkgload.LoadPackages(
165
215
ctx,
166
216
cfg.Module,
167
217
mainModLoc,
168
218
reqs,
169
219
cfg.Registry,
170
-
allImports,
220
+
pkgPathSlice,
171
221
), nil
172
222
}
+4
-1
cue/load/loader_common.go
+4
-1
cue/load/loader_common.go
···
170
170
if !filepath.IsAbs(fullPath) {
171
171
fullPath = filepath.Join(root, fullPath)
172
172
}
173
+
file.Filename = fullPath
173
174
}
174
-
file.Filename = fullPath
175
175
176
176
base := filepath.Base(fullPath)
177
177
···
184
184
file.ExcludeReason = fp.err
185
185
p.InvalidFiles = append(p.InvalidFiles, file)
186
186
return true
187
+
}
188
+
if err := setFileSource(fp.c, file); err != nil {
189
+
return badFile(errors.Promote(err, ""))
187
190
}
188
191
189
192
match, data, err := matchFile(fp.c, file, true, fp.allTags, mode)
+3
-17
cue/load/match.go
+3
-17
cue/load/match.go
···
15
15
package load
16
16
17
17
import (
18
-
"io"
19
18
"path/filepath"
20
19
"strings"
21
20
···
50
49
// If allTags is non-nil, matchFile records any encountered build tag
51
50
// by setting allTags[tag] = true.
52
51
func matchFile(cfg *Config, file *build.File, returnImports bool, allTags map[string]bool, mode importMode) (match bool, data []byte, err errors.Error) {
53
-
if fi := cfg.fileSystem.getOverlay(file.Filename); fi != nil {
54
-
if fi.file != nil {
55
-
file.Source = fi.file
56
-
} else {
57
-
file.Source = fi.contents
58
-
}
59
-
}
60
-
52
+
// Note: file.Source should already have been set by setFileSource just
53
+
// after the build.File value was created.
61
54
if file.Encoding != build.CUE {
62
55
return false, nil, nil // not a CUE file, don't record.
63
56
}
64
-
65
57
if file.Filename == "-" {
66
-
b, err2 := io.ReadAll(cfg.stdin())
67
-
if err2 != nil {
68
-
err = errors.Newf(token.NoPos, "read stdin: %v", err)
69
-
return
70
-
}
71
-
file.Source = b
72
-
return true, b, nil // don't check shouldBuild for stdin
58
+
return true, file.Source.([]byte), nil // don't check shouldBuild for stdin
73
59
}
74
60
75
61
name := filepath.Base(file.Filename)