Skip to content

Commit

Permalink
cue/load: obey "all packages" mode when loading files
Browse files Browse the repository at this point in the history
When load.Config.Package is "*", we document the behavior where
cue/load will load all files no matter what package they belong to.
This was obeyed in importPkg entrypoint when dealing with package args,
but not in cueFilesPackage when loading a list of CUE files directly.
Be consistent between the two by setting allPackages and PkgName.

Note that cueFilesPackage is different than importPkg
in that it wants all files to be part of a single build.Instance,
even when they have different package names.

To properly obey allPackages as part of a single build.Instance,
tweak fileProcessor.add to obey allPackages and not error on mismatches,
and tweak Instance.AddSyntax to ignore mismatches when User is set,
which it is when the package was created from individual files.
Note that the above requires setting User before calling addFiles.

This fixes two issues, both of which had the same root cause
due to the mismatching package names causing some files
to either be ignored or result in incorrect mismatch errors.

While here, remove the allFiles field and parameter,
neither of which was ever used. Perhaps a duplicate of allPackages.

Also stop setting ignoreOther in the "cue fmt" scenario.
When loading all packages, we explicitly do not want to ignore files
which belong to other packages. The true boolean appeared pointless,
but it still confused me for a bit, so remove it.

Fixes #1938.
Fixes #2523.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I3d742404711d7c91895448d827234a62427fba68
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1185356
TryBot-Result: CUEcueckoo <cueckoo@gmail.com>
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mvdan committed Mar 20, 2024
1 parent 7fcae93 commit 623b6a8
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 12 deletions.
26 changes: 26 additions & 0 deletions cmd/cue/cmd/testdata/script/fmt_err.txtar
Expand Up @@ -11,6 +11,12 @@ exec cue fmt ./...
cmp x.cue out/x_cue
cd ..

cd issue1938
exec cue fmt x.cue y.cue
cmp x.cue out/x_cue
cmp y.cue out/y_cue
cd ..

-- cue.mod/module.cue --
module: "mod.test/x"
-- emptypkg/.empty --
Expand Down Expand Up @@ -40,3 +46,23 @@ package x
import "mod.test/x/emptypkg"

x: 5
-- issue1938/x.cue --
package x

import "mod.test/x/nosuchpkg"

x: 5
-- issue1938/out/x_cue --
package x

import "mod.test/x/nosuchpkg"

x: 5
-- issue1938/y.cue --
package y

y: 6
-- issue1938/out/y_cue --
package y

y: 6
13 changes: 13 additions & 0 deletions cmd/cue/cmd/testdata/script/fmt_multi.txtar
@@ -1,8 +1,21 @@
# TODO(mvdan): once we have a "list" or "diff" fmt flag,
# we can reuse the same files without worrying about modifying in-place.
mkdir originals
cp x.cue star.cue y.cue yb.cue originals/

exec cue fmt .
cmp x.cue out/x.cue
cmp star.cue out/star-cue
cmp y.cue out/y-cue
cmp yb.cue out/yb-cue

cp originals/x.cue originals/star.cue originals/y.cue originals/yb.cue .

exec cue fmt x.cue star.cue y.cue yb.cue
cmp x.cue out/x.cue
cmp star.cue out/star-cue
cmp y.cue out/y-cue
cmp yb.cue out/yb-cue
-- x.cue --
// header

Expand Down
2 changes: 1 addition & 1 deletion cue/build/instance.go
Expand Up @@ -240,7 +240,7 @@ func (inst *Instance) AddSyntax(file *ast.File) errors.Error {
inst.Err = errors.Append(inst.Err, errors.Newf(pos, msg, args...))
})
_, pkg, pos := internal.PackageInfo(file)
if pkg != "" && pkg != "_" && !inst.setPkg(pkg) && pkg != inst.PkgName {
if pkg != "" && pkg != "_" && !inst.User && !inst.setPkg(pkg) && pkg != inst.PkgName {
err := errors.Newf(pos,
"package name %q conflicts with previous package name %q",
pkg, inst.PkgName)
Expand Down
1 change: 0 additions & 1 deletion cue/load/import.go
Expand Up @@ -80,7 +80,6 @@ func (l *loader) importPkg(pos token.Pos, p *build.Instance) []*build.Instance {

if p.PkgName == "" {
if l.cfg.Package == "*" {
fp.ignoreOther = true
fp.allPackages = true
p.PkgName = "_"
} else {
Expand Down
7 changes: 5 additions & 2 deletions cue/load/loader.go
Expand Up @@ -99,6 +99,10 @@ func (l *loader) cueFilesPackage(files []*build.File) *build.Instance {
}

fp := newFileProcessor(cfg, pkg, l.tagger)
if l.cfg.Package == "*" {
fp.allPackages = true
pkg.PkgName = "_"
}
for _, file := range files {
fp.add(cfg.Dir, file, allowAnonymous)
}
Expand All @@ -119,13 +123,12 @@ func (l *loader) cueFilesPackage(files []*build.File) *build.Instance {
// bp.ImportPath = ModDirImportPath(dir)
// }

pkg.User = true
l.addFiles(cfg.Dir, pkg)

pkg.User = true
l.stk.Push("user")
_ = pkg.Complete()
l.stk.Pop()
pkg.User = true
//pkg.LocalPrefix = dirToImportPath(dir)
pkg.DisplayPath = "command-line-arguments"

Expand Down
15 changes: 8 additions & 7 deletions cue/load/loader_common.go
Expand Up @@ -102,7 +102,6 @@ type fileProcessor struct {
firstCommentFile string
imported map[string][]token.Pos
allTags map[string]bool
allFiles bool
ignoreOther bool // ignore files from other packages
allPackages bool

Expand Down Expand Up @@ -183,7 +182,7 @@ func (fp *fileProcessor) add(root string, file *build.File, mode importMode) (ad
return true
}

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

Expand Down Expand Up @@ -264,11 +263,13 @@ func (fp *fileProcessor) add(root string, file *build.File, mode importMode) (ad
p.IgnoredFiles = append(p.IgnoredFiles, file)
return false
}
return badFile(&MultiplePackageError{
Dir: p.Dir,
Packages: []string{p.PkgName, pkg},
Files: []string{fp.firstFile, base},
})
if !fp.allPackages {
return badFile(&MultiplePackageError{
Dir: p.Dir,
Packages: []string{p.PkgName, pkg},
Files: []string{fp.firstFile, base},
})
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion cue/load/match.go
Expand Up @@ -49,7 +49,7 @@ func (e excludeError) Is(err error) bool { return err == errExclude }
// considers text until the first non-comment.
// If allTags is non-nil, matchFile records any encountered build tag
// by setting allTags[tag] = true.
func matchFile(cfg *Config, file *build.File, returnImports, allFiles bool, allTags map[string]bool) (match bool, data []byte, err errors.Error) {
func matchFile(cfg *Config, file *build.File, returnImports bool, allTags map[string]bool) (match bool, data []byte, err errors.Error) {
if fi := cfg.fileSystem.getOverlay(file.Filename); fi != nil {
if fi.file != nil {
file.Source = fi.file
Expand Down

0 comments on commit 623b6a8

Please sign in to comment.