this repo has no description
cue/load: minor tweaks
This CL makes some mostly cosmetic changes that shouldn't influence
observable behaviour in practice.
- ensure that Config.ModuleRoot is always an absolute path - various
other places in the code already assume that it is (by checking that
it's a prefix of other absolute paths, for example, or by returning it
where an absolute path is expected).
- Close the `cue.mod` file after opening it.
- Factor out the sizable code block that sets Config.Module into its own
function so it's clearer what it's doing (and avoiding the need for the
slightly sleazy switch, used only so we can `break` out of it.
- Remove places that needlessly check that Config.Dir is absolute.
- Make the contract for Config.newRelInstance clear, and change the only
place (a test) that calls it with an empty path, because it's clear by
inspection of all the call sites that it can never be called with an
empty path in practice.
Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: Id48c981b8fa2ebae56d88032c4b36de15888264f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/548987
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
+84
-86
+74
-68
cue/load/config.go
+74
-68
cue/load/config.go
···
15
15
package load
16
16
17
17
import (
18
+
"fmt"
18
19
"io"
19
20
"os"
20
21
pathpkg "path"
···
35
36
const (
36
37
cueSuffix = ".cue"
37
38
modDir = "cue.mod"
38
-
configFile = "module.cue"
39
+
moduleFile = "module.cue"
39
40
pkgDir = "pkg"
40
41
)
41
42
···
303
304
return i
304
305
}
305
306
307
+
// newRelInstance returns a build instance from the given
308
+
// relative import path.
306
309
func (c *Config) newRelInstance(pos token.Pos, path, pkgName string) *build.Instance {
310
+
if !isLocalImport(path) {
311
+
panic(fmt.Errorf("non-relative import path %q passed to newRelInstance", path))
312
+
}
307
313
fs := c.fileSystem
308
314
309
315
var err errors.Error
···
316
322
p.Root = c.ModuleRoot
317
323
p.Module = c.Module
318
324
319
-
if isLocalImport(path) {
320
-
if c.Dir == "" {
321
-
err = errors.Append(err, errors.Newf(pos, "cwd unknown"))
322
-
}
323
-
dir = filepath.Join(c.Dir, filepath.FromSlash(path))
324
-
}
325
+
dir = filepath.Join(c.Dir, filepath.FromSlash(path))
325
326
326
-
if path == "" {
327
-
err = errors.Append(err, errors.Newf(pos,
328
-
"import %q: invalid import path", path))
329
-
} else if path != cleanImport(path) {
327
+
if path != cleanImport(path) {
330
328
err = errors.Append(err, c.loader.errPkgf(nil,
331
329
"non-canonical import path: %q should be %q", path, pathpkg.Clean(path)))
332
330
}
···
480
478
481
479
// Complete updates the configuration information. After calling complete,
482
480
// the following invariants hold:
483
-
// - c.ModuleRoot != ""
481
+
// - c.Dir is an absolute path.
482
+
// - c.ModuleRoot is an absolute path
484
483
// - c.Module is set to the module import prefix if there is a cue.mod file
485
484
// with the module property.
486
485
// - c.loader != nil
···
517
516
if root := c.findRoot(c.Dir); root != "" {
518
517
c.ModuleRoot = root
519
518
}
519
+
} else if !filepath.IsAbs(c.ModuleRoot) {
520
+
c.ModuleRoot = filepath.Join(c.Dir, c.ModuleRoot)
520
521
}
521
-
522
+
if err := c.completeModule(); err != nil {
523
+
return nil, err
524
+
}
522
525
c.loader = &loader{
523
526
cfg: &c,
524
527
buildTags: make(map[string]bool),
525
528
}
526
-
527
-
// TODO: also make this work if run from outside the module?
528
-
switch {
529
-
case true:
530
-
mod := filepath.Join(c.ModuleRoot, modDir)
531
-
info, cerr := c.fileSystem.stat(mod)
532
-
if cerr != nil {
533
-
break
534
-
}
535
-
if info.IsDir() {
536
-
mod = filepath.Join(mod, configFile)
537
-
}
538
-
f, cerr := c.fileSystem.openFile(mod)
539
-
if cerr != nil {
540
-
break
541
-
}
542
-
543
-
// TODO: move to full build again
544
-
file, err := parser.ParseFile("load", f)
545
-
if err != nil {
546
-
return nil, errors.Wrapf(err, token.NoPos, "invalid cue.mod file")
547
-
}
548
-
549
-
r := runtime.New()
550
-
v, err := compile.Files(nil, r, "_", file)
551
-
if err != nil {
552
-
return nil, errors.Wrapf(err, token.NoPos, "invalid cue.mod file")
553
-
}
554
-
ctx := eval.NewContext(r, v)
555
-
v.Finalize(ctx)
556
-
prefix := v.Lookup(ctx.StringLabel("module"))
557
-
if prefix != nil {
558
-
name := ctx.StringValue(prefix.Value())
559
-
if err := ctx.Err(); err != nil {
560
-
return &c, err.Err
561
-
}
562
-
pos := token.NoPos
563
-
src := prefix.Value().Source()
564
-
if src != nil {
565
-
pos = src.Pos()
566
-
}
567
-
if c.Module != "" && c.Module != name {
568
-
return &c, errors.Newf(pos, "inconsistent modules: got %q, want %q", name, c.Module)
569
-
}
570
-
c.Module = name
571
-
}
572
-
}
573
-
574
529
c.loadFunc = c.loader.loadFunc()
575
530
576
531
if c.Context == nil {
···
579
534
build.ParseFile(c.loader.cfg.ParseFile),
580
535
)
581
536
}
537
+
return &c, nil
538
+
}
582
539
583
-
return &c, nil
540
+
// completeModule fills out c.Module if it's empty or checks it for
541
+
// consistency with the module file otherwise.
542
+
func (c *Config) completeModule() error {
543
+
// TODO: also make this work if run from outside the module?
544
+
mod := filepath.Join(c.ModuleRoot, modDir)
545
+
info, cerr := c.fileSystem.stat(mod)
546
+
if cerr != nil {
547
+
return nil
548
+
}
549
+
// TODO remove support for legacy non-directory module.cue file
550
+
// by returning an error if info.IsDir is false.
551
+
if info.IsDir() {
552
+
mod = filepath.Join(mod, moduleFile)
553
+
}
554
+
f, cerr := c.fileSystem.openFile(mod)
555
+
if cerr != nil {
556
+
return nil
557
+
}
558
+
defer f.Close()
559
+
560
+
// TODO: move to full build again
561
+
file, err := parser.ParseFile("load", f)
562
+
if err != nil {
563
+
return errors.Wrapf(err, token.NoPos, "invalid cue.mod file")
564
+
}
565
+
566
+
r := runtime.New()
567
+
v, err := compile.Files(nil, r, "_", file)
568
+
if err != nil {
569
+
return errors.Wrapf(err, token.NoPos, "invalid cue.mod file")
570
+
}
571
+
ctx := eval.NewContext(r, v)
572
+
v.Finalize(ctx)
573
+
prefix := v.Lookup(ctx.StringLabel("module"))
574
+
if prefix == nil {
575
+
return nil
576
+
}
577
+
name := ctx.StringValue(prefix.Value())
578
+
if err := ctx.Err(); err != nil {
579
+
return err.Err
580
+
}
581
+
if c.Module == "" {
582
+
c.Module = name
583
+
return nil
584
+
}
585
+
if c.Module == name {
586
+
return nil
587
+
}
588
+
pos := token.NoPos
589
+
if src := prefix.Value().Source(); src != nil {
590
+
pos = src.Pos()
591
+
}
592
+
return errors.Newf(pos, "inconsistent modules: got %q, want %q", name, c.Module)
584
593
}
585
594
586
595
func (c Config) isRoot(dir string) bool {
···
590
599
return err == nil
591
600
}
592
601
593
-
// findRoot returns the module root or "" if none was found.
594
-
func (c Config) findRoot(dir string) string {
602
+
// findRoot returns the module root that's ancestor
603
+
// of the given absolute directory path, or "" if none was found.
604
+
func (c Config) findRoot(absDir string) string {
595
605
fs := &c.fileSystem
596
606
597
-
absDir, err := filepath.Abs(dir)
598
-
if err != nil {
599
-
return ""
600
-
}
601
607
abs := absDir
602
608
for {
603
609
if c.isRoot(abs) {
+1
-5
cue/load/import.go
+1
-5
cue/load/import.go
···
52
52
)
53
53
54
54
// importPkg returns details about the CUE package named by the import path,
55
-
// interpreting local import paths relative to the srcDir directory.
55
+
// interpreting local import paths relative to l.cfg.Dir.
56
56
// If the path is a local import path naming a package that can be imported
57
57
// using a standard import path, the returned package will set p.ImportPath
58
58
// to that path.
···
111
111
if p.PkgName != "" {
112
112
// If we have an explicit package name, we can ignore other packages.
113
113
fp.ignoreOther = true
114
-
}
115
-
116
-
if !strings.HasPrefix(p.Dir, cfg.ModuleRoot) {
117
-
panic("")
118
114
}
119
115
120
116
var dirs [][2]string
+5
-2
cue/load/import_test.go
+5
-2
cue/load/import_test.go
···
34
34
}
35
35
36
36
func TestEmptyImport(t *testing.T) {
37
-
p, err := getInst("", "")
38
-
if err == nil {
37
+
c, _ := (&Config{}).complete()
38
+
l := loader{cfg: c}
39
+
inst := c.newInstance(token.NoPos, "")
40
+
p := l.importPkg(token.NoPos, inst)[0]
41
+
if p.Err == nil {
39
42
t.Fatal(`Import("") returned nil error.`)
40
43
}
41
44
if p == nil {
-6
cue/load/loader.go
-6
cue/load/loader.go
···
156
156
// ModInit() // TODO: support modules
157
157
pkg := l.cfg.Context.NewInstance(cfg.Dir, l.loadFunc())
158
158
159
-
_, err := filepath.Abs(cfg.Dir)
160
-
if err != nil {
161
-
return cfg.newErrInstance(pos, toImportPath(cfg.Dir),
162
-
errors.Wrapf(err, pos, "could not convert '%s' to absolute path", cfg.Dir))
163
-
}
164
-
165
159
for _, bf := range files {
166
160
f := bf.Filename
167
161
if f == "-" {
+4
-5
cue/load/search.go
+4
-5
cue/load/search.go
···
197
197
// silently skipped as not matching the pattern.
198
198
// Do not take root, as we want to stay relative
199
199
// to one dir only.
200
-
dir, e := filepath.Rel(c.Dir, path)
200
+
relPath, e := filepath.Rel(c.Dir, path)
201
201
if e != nil {
202
-
panic(err)
203
-
} else {
204
-
dir = "./" + dir
202
+
panic(err) // Should never happen because c.Dir is absolute.
205
203
}
204
+
relPath = "./" + filepath.ToSlash(relPath)
206
205
// TODO: consider not doing these checks here.
207
-
inst := c.newRelInstance(token.NoPos, dir, pkgName)
206
+
inst := c.newRelInstance(token.NoPos, relPath, pkgName)
208
207
pkgs := l.importPkg(token.NoPos, inst)
209
208
for _, p := range pkgs {
210
209
if err := p.Err; err != nil && (p == nil || len(p.InvalidFiles) == 0) {