Skip to content

Commit 734a8a3

Browse files
akutzakutz
committed
Cross-module support for filesystem paths
This patch updates the controller-gen package loader to support loading packages across Go module boundaries for filesystem paths. What does this actually mean? Imagine a project with the following structure: my-project/ | |-- go.mod |-- go.sum | |-- apis/ | |-- v1alpha1/ | |-- go.mod |-- go.sum Prior to this patch, the following command, executed from the root of "my-project," would *not* include the package my-project/apis/v1alpha1: controller-gen crd \ paths=./apis/... \ crd:trivialVersions=true \ output:crd:dir=./config/crd/bases The reason the above command would not parse the types from the package my-project/apis/v1alpha1 is because it is a distinct Go module from the root module, where the command was executed. In fact, while the above command would silently fail to include those types, the following command will fail with an error: controller-gen crd \ paths=./apis/v1alpha1 \ crd:trivialVersions=true \ output:crd:dir=./config/crd/bases The above command fails because the underlying logic to load packages cannot load a path from one package when executed from the working directory of another package. With this patch, it is now possible for the first command to successfully traverse Go module boundaries for roots that are file- system paths ending with "..." with the following, high-level logic: 1. If no roots are provided then load the working directory and return early. 2. Otherwise sort the provided roots into two, distinct buckets: a. package/module names b. filesystem paths A filesystem path is distinguished from a Go package/module name by the same rules as followed by the "go" command. At a high level, a root is a filesystem path IFF it meets ANY of the following criteria: * is absolute * begins with . * begins with .. For more information please refer to the output of the command "go help packages". 3. Load the package/module roots as a single call to packages.Load. If there are no filesystem path roots then return early. 4. For filesystem path roots ending with "...", check to see if its descendants include any nested, Go modules. If so, add the directory that contains the nested Go module to the filesystem path roots. 5. Load the filesystem path roots and return the load packages for the package/module roots AND the filesystem path roots.
1 parent cb13ac5 commit 734a8a3

File tree

13 files changed

+590
-6
lines changed

13 files changed

+590
-6
lines changed

‎pkg/loader/loader.go

Lines changed: 238 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2019 The Kubernetes Authors.
2+
Copyright 2019-2022 The Kubernetes Authors.
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
@@ -25,9 +25,12 @@ import (
2525
"go/types"
2626
"io/ioutil"
2727
"os"
28+
"path/filepath"
29+
"regexp"
2830
"sync"
2931

3032
"golang.org/x/tools/go/packages"
33+
"k8s.io/apimachinery/pkg/util/sets"
3134
)
3235

3336
// Much of this is strongly inspired by the contents of go/packages,
@@ -329,6 +332,40 @@ func LoadRoots(roots ...string) ([]*Package, error) {
329332
//
330333
// This is generally only useful for use in testing when you need to modify
331334
// loading settings to load from a fake location.
335+
//
336+
// This function will traverse Go module boundaries for roots that are file-
337+
// system paths and end with "...". Please note this feature currently only
338+
// supports roots that are filesystem paths. For more information, please
339+
// refer to the high-level outline of this function's logic:
340+
//
341+
// 1. If no roots are provided then load the working directory and return
342+
// early.
343+
//
344+
// 2. Otherwise sort the provided roots into two, distinct buckets:
345+
//
346+
// a. package/module names
347+
// b. filesystem paths
348+
//
349+
// A filesystem path is distinguished from a Go package/module name by
350+
// the same rules as followed by the "go" command. At a high level, a
351+
// root is a filesystem path IFF it meets ANY of the following criteria:
352+
//
353+
// * is absolute
354+
// * begins with .
355+
// * begins with ..
356+
//
357+
// For more information please refer to the output of the command
358+
// "go help packages".
359+
//
360+
// 3. Load the package/module roots as a single call to packages.Load. If
361+
// there are no filesystem path roots then return early.
362+
//
363+
// 4. For filesystem path roots ending with "...", check to see if its
364+
// descendants include any nested, Go modules. If so, add the directory
365+
// that contains the nested Go module to the filesystem path roots.
366+
//
367+
// 5. Load the filesystem path roots and return the load packages for the
368+
// package/module roots AND the filesystem path roots.
332369
func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, error) {
333370
l := &loader{
334371
cfg: cfg,
@@ -341,13 +378,208 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
341378
// put our build flags first so that callers can override them
342379
l.cfg.BuildFlags = append([]string{"-tags", "ignore_autogenerated"}, l.cfg.BuildFlags...)
343380

344-
rawPkgs, err := packages.Load(l.cfg, roots...)
345-
if err != nil {
346-
return nil, err
381+
// uniquePkgIDs is used to keep track of the discovered packages to be nice
382+
// and try and prevent packages from showing up twice when nested module
383+
// support is enabled. there is not harm that comes from this per se, but
384+
// it makes testing easier when a known number of modules can be asserted
385+
uniquePkgIDs := sets.String{}
386+
387+
// loadPackages returns the Go packages for the provided roots
388+
//
389+
// if validatePkgFn is nil, a package will be returned in the slice,
390+
// otherwise the package is only returned if the result of
391+
// validatePkgFn(pkg.ID) is truthy
392+
loadPackages := func(roots ...string) ([]*Package, error) {
393+
rawPkgs, err := packages.Load(l.cfg, roots...)
394+
if err != nil {
395+
return nil, err
396+
}
397+
var pkgs []*Package
398+
for _, rp := range rawPkgs {
399+
p := l.packageFor(rp)
400+
if !uniquePkgIDs.Has(p.ID) {
401+
pkgs = append(pkgs, p)
402+
uniquePkgIDs.Insert(p.ID)
403+
}
404+
}
405+
return pkgs, nil
406+
}
407+
408+
// if no roots were provided then load the current package and return early
409+
if len(roots) == 0 {
410+
pkgs, err := loadPackages()
411+
if err != nil {
412+
return nil, err
413+
}
414+
l.Roots = append(l.Roots, pkgs...)
415+
return l.Roots, nil
416+
}
417+
418+
// pkgRoots is a slice of roots that are package/modules and fspRoots
419+
// is a slice of roots that are local filesystem paths.
420+
//
421+
// please refer to this function's godoc comments for more information on
422+
// how these two types of roots are distinguished from one another
423+
var (
424+
pkgRoots []string
425+
fspRoots []string
426+
fspRootRx = regexp.MustCompile(`^\.{1,2}`)
427+
)
428+
for _, r := range roots {
429+
if filepath.IsAbs(r) || fspRootRx.MatchString(r) {
430+
fspRoots = append(fspRoots, r)
431+
} else {
432+
pkgRoots = append(pkgRoots, r)
433+
}
434+
}
435+
436+
// handle the package roots by sending them into the packages.Load function
437+
// all at once. this is more efficient, but cannot be used for the file-
438+
// system path roots due to them needing a custom, calculated value for the
439+
// cfg.Dir field
440+
if len(pkgRoots) > 0 {
441+
pkgs, err := loadPackages(pkgRoots...)
442+
if err != nil {
443+
return nil, err
444+
}
445+
l.Roots = append(l.Roots, pkgs...)
446+
}
447+
448+
// if there are no filesystem path roots then go ahead and return early
449+
if len(fspRoots) == 0 {
450+
return l.Roots, nil
451+
}
452+
453+
//
454+
// at this point we are handling filesystem path roots
455+
//
456+
457+
// ensure the cfg.Dir field is reset to its original value upon
458+
// returning from this function. it should honestly be fine if it is
459+
// not given most callers will not send in the cfg parameter directly,
460+
// as it's largely for testing, but still, let's be good stewards.
461+
defer func(d string) {
462+
cfg.Dir = d
463+
}(cfg.Dir)
464+
465+
// store the value of cfg.Dir so we can use it later if it is non-empty.
466+
// we need to store it now as the value of cfg.Dir will be updated by
467+
// a loop below
468+
cfgDir := cfg.Dir
469+
470+
// addNestedGoModulesToRoots is given to filepath.WalkDir and adds the
471+
// directory part of p to the list of filesystem path roots IFF p is the
472+
// path to a file named "go.mod"
473+
addNestedGoModulesToRoots := func(
474+
p string,
475+
d os.DirEntry,
476+
e error) error {
477+
478+
if e != nil {
479+
return e
480+
}
481+
if !d.IsDir() && filepath.Base(p) == "go.mod" {
482+
fspRoots = append(fspRoots, filepath.Join(filepath.Dir(p), "..."))
483+
}
484+
return nil
347485
}
348486

349-
for _, rawPkg := range rawPkgs {
350-
l.Roots = append(l.Roots, l.packageFor(rawPkg))
487+
// in the first pass over the filesystem path roots we:
488+
//
489+
// 1. make the root into an absolute path
490+
//
491+
// 2. check to see if a root uses the nested path syntax, ex. ...
492+
//
493+
// 3. if so, walk the root's descendants, searching for any nested Go
494+
// modules
495+
//
496+
// 4. if found then the directory containing the Go module is added to
497+
// the list of the filesystem path roots
498+
for i := range fspRoots {
499+
r := fspRoots[i]
500+
501+
// clean up the root
502+
r = filepath.Clean(r)
503+
504+
// get the absolute path of the root
505+
if !filepath.IsAbs(r) {
506+
507+
// if the initial value of cfg.Dir was non-empty then use it when
508+
// building the absolute path to this root. otherwise use the
509+
// filepath.Abs function to get the absolute path of the root based
510+
// on the working directory
511+
if cfgDir != "" {
512+
r = filepath.Join(cfgDir, r)
513+
} else {
514+
ar, err := filepath.Abs(r)
515+
if err != nil {
516+
return nil, err
517+
}
518+
r = ar
519+
}
520+
}
521+
522+
// update the root to be an absolute path
523+
fspRoots[i] = r
524+
525+
b, d := filepath.Base(r), filepath.Dir(r)
526+
527+
// if the base element is "..." then it means nested traversal is
528+
// activated. this can be passed directly to the loader. however, if
529+
// specified we also want to traverse the path manually to determine if
530+
// there are any nested Go modules we want to add to the list of file-
531+
// system path roots to process
532+
if b == "..." {
533+
if err := filepath.WalkDir(
534+
d,
535+
addNestedGoModulesToRoots); err != nil {
536+
537+
return nil, err
538+
}
539+
}
540+
}
541+
542+
// in the second pass over the filesystem path roots we:
543+
//
544+
// 1. determine the directory from which to execute the loader
545+
//
546+
// 2. update the loader config's Dir property to be the directory from
547+
// step one
548+
//
549+
// 3. determine whether the root passed to the loader should be "./."
550+
// or "./..."
551+
//
552+
// 4. execute the loader with the value from step three
553+
for _, r := range fspRoots {
554+
b, d := filepath.Base(r), filepath.Dir(r)
555+
556+
// we want the base part of the path to be either "..." or ".", except
557+
// Go's filepath utilities clean paths during manipulation, removing the
558+
// ".". thus, if not "...", let's update the path components so that:
559+
//
560+
// d = r
561+
// b = "."
562+
if b != "..." {
563+
d = r
564+
b = "."
565+
}
566+
567+
// update the loader configuration's Dir field to the directory part of
568+
// the root
569+
l.cfg.Dir = d
570+
571+
// update the root to be "./..." or "./."
572+
// (with OS-specific filepath separator). please note filepath.Join
573+
// would clean up the trailing "." character that we want preserved,
574+
// hence the more manual path concatenation logic
575+
r = fmt.Sprintf(".%s%s", string(filepath.Separator), b)
576+
577+
// load the packages from the roots
578+
pkgs, err := loadPackages(r)
579+
if err != nil {
580+
return nil, err
581+
}
582+
l.Roots = append(l.Roots, pkgs...)
351583
}
352584

353585
return l.Roots, nil

‎pkg/loader/loader_suite_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
Copyright 2022 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package loader_test
18+
19+
import (
20+
"testing"
21+
22+
. "github.com/onsi/ginkgo"
23+
. "github.com/onsi/gomega"
24+
)
25+
26+
func TestLoader(t *testing.T) {
27+
RegisterFailHandler(Fail)
28+
RunSpecs(t, "Loader Patching Suite")
29+
}

0 commit comments

Comments
 (0)