this repo has no description
cue/load: do not create packages unnecessarily for files outside package directory
The current logic creates instances for files outside a package's
directory even when those have nothing to do with the package in
question.
Change the logic so that when a file in a parent directory is added, an
instance is only created when that file's package matches one of the
packages in the original directory.
Note: this is only an issue when `PkgName` is set to `*` to load all
packages.
Note also: this fixes some of the existing behavior, as witness the
other test result changes that mention `anon.cue` where that file is
actually irrelevant and should not be included as part of the instance
(packageless CUE files never include packageless CUE files from parent
directories).
Fixes #3306
Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I7f158b70f5d64068aa031b063aaca5680d738132
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198686
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
+46
-60
+11
-1
cue/load/import.go
+11
-1
cue/load/import.go
···
53
53
// is present.
54
54
// _ anonymous files (which may be marked with _)
55
55
// * all packages
56
-
func (l *loader) importPkg(pos token.Pos, p *build.Instance) (_ret []*build.Instance) {
56
+
func (l *loader) importPkg(pos token.Pos, p *build.Instance) []*build.Instance {
57
57
retErr := func(errs errors.Error) []*build.Instance {
58
58
// XXX: move this loop to ReportError
59
59
for _, err := range errors.Errors(errs) {
···
192
192
p.ReportError(errors.Promote(err, ""))
193
193
}
194
194
195
+
if len(p.BuildFiles) == 0 &&
196
+
len(p.IgnoredFiles) == 0 &&
197
+
len(p.OrphanedFiles) == 0 &&
198
+
len(p.InvalidFiles) == 0 &&
199
+
len(p.UnknownFiles) == 0 {
200
+
// The package has no files in it. This can happen
201
+
// when the default package added in newFileProcessor
202
+
// doesn't have any associated files.
203
+
continue
204
+
}
195
205
all = append(all, p)
196
206
rewriteFiles(p, cfg.ModuleRoot, false)
197
207
if errs := fp.finalize(p); errs != nil {
+31
-5
cue/load/loader_common.go
+31
-5
cue/load/loader_common.go
···
167
167
// special * and _
168
168
p := fp.pkg // default package
169
169
170
+
// sameDir holds whether the file should be considered to be
171
+
// part of the same directory as the default package. This is
172
+
// true when the file is part of the original package directory
173
+
// or when allowExcludedFiles is specified, signifying that the
174
+
// file is part of an explicit set of files provided on the
175
+
// command line.
176
+
sameDir := filepath.Dir(fullPath) == p.Dir || (mode&allowExcludedFiles) != 0
177
+
170
178
// badFile := func(p *build.Instance, err errors.Error) bool {
171
179
badFile := func(err errors.Error) {
172
180
fp.err = errors.Append(fp.err, err)
···
181
189
182
190
if file.Encoding != build.CUE {
183
191
// Not a CUE file.
184
-
p.OrphanedFiles = append(p.OrphanedFiles, file)
192
+
if sameDir {
193
+
p.OrphanedFiles = append(p.OrphanedFiles, file)
194
+
}
185
195
return
186
196
}
187
197
if (mode & allowExcludedFiles) == 0 {
···
192
202
}
193
203
}
194
204
if badPrefix != "" {
205
+
if !sameDir {
206
+
return
207
+
}
195
208
file.ExcludeReason = errors.Newf(token.NoPos, "filename starts with a '%s'", badPrefix)
196
209
if file.Interpretation == "" {
197
210
p.IgnoredFiles = append(p.IgnoredFiles, file)
···
217
230
pos := pf.Pos()
218
231
219
232
switch {
220
-
case pkg == p.PkgName, mode&allowAnonymous != 0:
233
+
case pkg == p.PkgName && (sameDir || pkg != "_"):
234
+
// We've got the exact package that's being looked for.
235
+
// It will already be present in fp.pkgs.
236
+
case mode&allowAnonymous != 0 && sameDir:
237
+
// It's an anonymous file that's not in a parent directory.
221
238
case fp.allPackages && pkg != "_":
222
239
q := fp.pkgs[pkg]
240
+
if q == nil && !sameDir {
241
+
// It's a file in a parent directory that doesn't correspond
242
+
// to a package in the original directory.
243
+
return
244
+
}
223
245
if q == nil {
224
246
q = &build.Instance{
225
247
PkgName: pkg,
···
235
257
p = q
236
258
237
259
case pkg != "_":
238
-
260
+
// We're loading a single package and we either haven't matched
261
+
// the earlier selected package or we haven't selected a package
262
+
// yet. In either case, the default package is the one we want to use.
239
263
default:
240
-
file.ExcludeReason = excludeError{errors.Newf(pos, "no package name")}
241
-
p.IgnoredFiles = append(p.IgnoredFiles, file)
264
+
if sameDir {
265
+
file.ExcludeReason = excludeError{errors.Newf(pos, "no package name")}
266
+
p.IgnoredFiles = append(p.IgnoredFiles, file)
267
+
}
242
268
return
243
269
}
244
270
+4
-54
cue/load/loader_test.go
+4
-54
cue/load/loader_test.go
···
307
307
},
308
308
args: []string{"./toolonly"},
309
309
want: `err: build constraints exclude all CUE files in ./toolonly:
310
-
anon.cue: no package name
311
310
test.cue: package is test, want foo
312
311
toolonly/foo_tool.cue: _tool.cue files excluded in non-cmd mode
313
312
path: mod.test/test/toolonly@v0:foo
···
499
498
dir: $CWD/testdata/testmod/multi4
500
499
display:.
501
500
files:
502
-
$CWD/testdata/testmod/anon.cue
503
501
$CWD/testdata/testmod/multi4/nopackage1.cue
504
502
$CWD/testdata/testmod/multi4/nopackage2.cue`}, {
505
503
// Test what happens when there's a single CUE file
···
516
514
dir: $CWD/testdata/testmod/multi5
517
515
display:.
518
516
files:
519
-
$CWD/testdata/testmod/anon.cue
520
517
$CWD/testdata/testmod/multi5/nopackage.cue`}, {
521
518
// Check that imports are only considered from files
522
519
// that match the build paths.
···
570
567
// in the result of Instances.
571
568
name: "Issue3306",
572
569
cfg: &Config{
573
-
Dir: testdataDir,
574
-
Package: "*",
570
+
Dir: testdataDir,
571
+
Package: "*",
572
+
SkipImports: true,
575
573
},
576
574
args: []string{"./issue3306/..."},
577
-
want: `path: mod.test/test/issue3306@v0:_
578
-
module: mod.test/test@v0
579
-
root: $CWD/testdata/testmod
580
-
dir: $CWD/testdata/testmod/issue3306
581
-
display:./issue3306
582
-
files:
583
-
$CWD/testdata/testmod/anon.cue
584
-
585
-
path: mod.test/test/issue3306@v0:test
586
-
module: mod.test/test@v0
587
-
root: $CWD/testdata/testmod
588
-
dir: $CWD/testdata/testmod/issue3306
589
-
display:./issue3306
590
-
files:
591
-
$CWD/testdata/testmod/test.cue
592
-
593
-
path: mod.test/test/issue3306@v0:x
575
+
want: `path: mod.test/test/issue3306@v0:x
594
576
module: mod.test/test@v0
595
577
root: $CWD/testdata/testmod
596
578
dir: $CWD/testdata/testmod/issue3306
···
598
580
files:
599
581
$CWD/testdata/testmod/issue3306/x.cue
600
582
601
-
path: mod.test/test/issue3306/a@v0:_
602
-
module: mod.test/test@v0
603
-
root: $CWD/testdata/testmod
604
-
dir: $CWD/testdata/testmod/issue3306/a
605
-
display:./issue3306/a
606
-
files:
607
-
$CWD/testdata/testmod/anon.cue
608
-
609
583
path: mod.test/test/issue3306/a@v0:a
610
584
module: mod.test/test@v0
611
585
root: $CWD/testdata/testmod
···
622
596
files:
623
597
$CWD/testdata/testmod/issue3306/a/b.cue
624
598
625
-
path: mod.test/test/issue3306/a@v0:test
626
-
module: mod.test/test@v0
627
-
root: $CWD/testdata/testmod
628
-
dir: $CWD/testdata/testmod/issue3306/a
629
-
display:./issue3306/a
630
-
files:
631
-
$CWD/testdata/testmod/test.cue
632
-
633
599
path: mod.test/test/issue3306/a@v0:x
634
600
module: mod.test/test@v0
635
601
root: $CWD/testdata/testmod
···
638
604
files:
639
605
$CWD/testdata/testmod/issue3306/x.cue
640
606
$CWD/testdata/testmod/issue3306/a/x.cue
641
-
642
-
path: mod.test/test/issue3306/x@v0:_
643
-
module: mod.test/test@v0
644
-
root: $CWD/testdata/testmod
645
-
dir: $CWD/testdata/testmod/issue3306/x
646
-
display:./issue3306/x
647
-
files:
648
-
$CWD/testdata/testmod/anon.cue
649
-
650
-
path: mod.test/test/issue3306/x@v0:test
651
-
module: mod.test/test@v0
652
-
root: $CWD/testdata/testmod
653
-
dir: $CWD/testdata/testmod/issue3306/x
654
-
display:./issue3306/x
655
-
files:
656
-
$CWD/testdata/testmod/test.cue
657
607
658
608
path: mod.test/test/issue3306/x@v0:x
659
609
module: mod.test/test@v0