this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

cue/load: do not load non-CUE directories as instances

Currently if we use a pattern like `./...:foo` and there's a `package
foo` file in the root directory, every single directory counts as an
instance of package foo even if it doesn't have any `package foo` files.

We fix that by only traversing to a directory's parents if there's at
least one file that might be part of the package. It's OK if that file
is excluded (a package that unifies with an ancestor directory is still
valid even if build constraints currently exclude all the files in that
directory itself).

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: If8450b8fee821af553fb2c66815ed687bc2cd359
Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1230082
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>

+28 -31
+10 -3
cue/load/import.go
··· 142 142 // See https://cuelang.org/docs/concept/modules-packages-instances/#instances. 143 143 for _, d := range dirs { 144 144 dir := filepath.Clean(d[1]) 145 + // firstDir keeps track of whether we're still looking at the initial 146 + // directory rather than one of its parents. If there are no CUE files 147 + // in the initial directory, we shouldn't walk to its parents because 148 + // the initial directory isn't itself a CUE package. 149 + firstDir := true 145 150 for { 146 151 sd, ok := l.dirCachedBuildFiles[dir] 147 152 if !ok { ··· 154 159 } 155 160 return retErr(errors.Wrapf(err, token.NoPos, "import failed reading dir %v", dir)) 156 161 } 162 + added := false 157 163 for _, name := range sd.filenames { 158 164 file, err := filetypes.ParseFileAndType(name, "", filetypes.Input) 159 165 if err != nil { ··· 161 167 Filename: name, 162 168 ExcludeReason: errors.Newf(token.NoPos, "unknown filetype"), 163 169 }) 164 - } else { 165 - fp.add(dir, file, 0) 170 + } else if fp.add(dir, file, 0) { 171 + added = true 166 172 } 167 173 } 168 - if p.PkgName == "" || !inModule || l.cfg.isModRoot(dir) || dir == d[0] { 174 + if p.PkgName == "" || !inModule || l.cfg.isModRoot(dir) || dir == d[0] || (firstDir && !added) { 169 175 break 170 176 } 171 177 ··· 180 186 break 181 187 } 182 188 dir = parent 189 + firstDir = false 183 190 } 184 191 } 185 192
+16 -11
cue/load/loader_common.go
··· 153 153 } 154 154 155 155 // add adds the given file to the appropriate package in fp. 156 - func (fp *fileProcessor) add(root string, file *build.File, mode importMode) { 156 + // It reports whether the file might be considered part of the 157 + // package being loaded, even if it ends up not added to 158 + // the build files, for example because of an @if constraint or 159 + // it's a tool file. 160 + func (fp *fileProcessor) add(root string, file *build.File, mode importMode) bool { 157 161 fullPath := file.Filename 158 162 if fullPath != "-" { 159 163 if !filepath.IsAbs(fullPath) { ··· 183 187 } 184 188 if err := setFileSource(fp.c, file); err != nil { 185 189 badFile(errors.Promote(err, "")) 186 - return 190 + return false 187 191 } 188 192 189 193 if file.Encoding != build.CUE { ··· 191 195 if sameDir { 192 196 p.OrphanedFiles = append(p.OrphanedFiles, file) 193 197 } 194 - return 198 + return false 195 199 } 196 200 if (mode & allowExcludedFiles) == 0 { 197 201 var badPrefix string ··· 202 206 } 203 207 if badPrefix != "" { 204 208 if !sameDir { 205 - return 209 + return false 206 210 } 207 211 file.ExcludeReason = errors.Newf(token.NoPos, "filename starts with a '%s'", badPrefix) 208 212 if file.Interpretation == "" { ··· 210 214 } else { 211 215 p.OrphanedFiles = append(p.OrphanedFiles, file) 212 216 } 213 - return 217 + return false 214 218 } 215 219 } 216 220 // Note: when path is "-" (stdin), it will already have ··· 219 223 pf, perr := fp.c.fileSystem.getCUESyntax(file, fp.c.parserConfig) 220 224 if perr != nil { 221 225 badFile(errors.Promote(perr, "add failed")) 222 - return 226 + return false 223 227 } 224 228 225 229 pkg := pf.PackageName() ··· 239 243 if q == nil && !sameDir { 240 244 // It's a file in a parent directory that doesn't correspond 241 245 // to a package in the original directory. 242 - return 246 + return false 243 247 } 244 248 if q == nil { 245 249 q = fp.c.Context.NewInstance(p.Dir, nil) ··· 262 266 file.ExcludeReason = excludeError{errors.Newf(pos, "no package name")} 263 267 p.IgnoredFiles = append(p.IgnoredFiles, file) 264 268 } 265 - return 269 + return false 266 270 } 267 271 268 272 if !fp.c.AllCUEFiles { ··· 282 286 } 283 287 file.ExcludeReason = err 284 288 p.IgnoredFiles = append(p.IgnoredFiles, file) 285 - return 289 + return true 286 290 } 287 291 } 288 292 ··· 295 299 file.ExcludeReason = excludeError{errors.Newf(pos, 296 300 "package is %s, want %s", pkg, p.PkgName)} 297 301 p.IgnoredFiles = append(p.IgnoredFiles, file) 298 - return 302 + return false 299 303 } 300 304 if !fp.allPackages { 301 305 badFile(&MultiplePackageError{ ··· 303 307 Packages: []string{p.PkgName, pkg}, 304 308 Files: []string{fp.firstFile, base}, 305 309 }) 306 - return 310 + return false 307 311 } 308 312 } 309 313 } ··· 344 348 default: 345 349 p.BuildFiles = append(p.BuildFiles, file) 346 350 } 351 + return true 347 352 } 348 353 349 354 // isLocalImport reports whether the import path is
+2 -17
cue/load/loader_test.go
··· 112 112 files: 113 113 $CWD/testdata/testmod/noncuedirectories/root.cue 114 114 115 - path: mod.test/test/noncuedirectories/a@v0:foo 116 - module: mod.test/test@v0 117 - root: $CWD/testdata/testmod 118 - dir: $CWD/testdata/testmod/noncuedirectories/a 119 - display:./noncuedirectories/a 120 - files: 121 - $CWD/testdata/testmod/noncuedirectories/root.cue 122 - 123 - path: mod.test/test/noncuedirectories/b@v0:foo 124 - module: mod.test/test@v0 125 - root: $CWD/testdata/testmod 126 - dir: $CWD/testdata/testmod/noncuedirectories/b 127 - display:./noncuedirectories/b 128 - files: 129 - $CWD/testdata/testmod/noncuedirectories/root.cue 130 - 131 115 path: mod.test/test/noncuedirectories/b/c@v0:foo 132 116 module: mod.test/test@v0 133 117 root: $CWD/testdata/testmod ··· 151 135 dir: $CWD/testdata/testmod/noncuedirectories/tool 152 136 display:./noncuedirectories/tool 153 137 files: 154 - $CWD/testdata/testmod/noncuedirectories/root.cue`}, { 138 + $CWD/testdata/testmod/noncuedirectories/root.cue`, 139 + }, { 155 140 name: "DefaultPackageWithExplicitDotArgument", 156 141 // Even though the directory is called testdata, the last path in 157 142 // the module is test. So "package test" is correctly the default