Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🏃 Simplify the LoadRootsWithConfig logic #687

Merged
merged 1 commit into from Jun 3, 2022
Merged

🏃 Simplify the LoadRootsWithConfig logic #687

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
106 changes: 42 additions & 64 deletions pkg/loader/loader.go
View file
Open in desktop
Expand Up @@ -323,7 +323,7 @@ func (l *loader) parseFile(filename string, src []byte) (*ast.File, error) {
// populated. Additional information, like ASTs and type-checking information,
// can be accessed via methods on individual packages.
func LoadRoots(roots ...string) ([]*Package, error) {
return LoadRootsWithConfig(nil, roots...)
return LoadRootsWithConfig(&packages.Config{}, roots...)
}

// LoadRootsWithConfig functions like LoadRoots, except that it allows passing
Expand Down Expand Up @@ -366,41 +366,30 @@ func LoadRoots(roots ...string) ([]*Package, error) {
//
// 5. Load the filesystem path roots and return the load packages for the
// package/module roots AND the filesystem path roots.
//
//nolint:gocyclo
func LoadRootsWithConfig(cfg *packages.Config, roots ...string) (result []*Package, retErr error) {
func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, error) {
l := &loader{
cfg: cfg,
packages: make(map[*packages.Package]*Package),
}
l.cfg.Mode |= packages.LoadImports | packages.NeedTypesSizes
if l.cfg.Fset == nil {
l.cfg.Fset = token.NewFileSet()
}
// put our build flags first so that callers can override them
l.cfg.BuildFlags = append([]string{"-tags", "ignore_autogenerated"}, l.cfg.BuildFlags...)

// Visit the import graphs of the loaded, root packages. If an imported
// package refers to another loaded, root package, then replace the
// instance of the imported package with a reference to the loaded, root
// package. This is required to make kubebuilder markers work correctly
// when multiple root paths are loaded and types from one path reference
// types from another root path.
defer func() {
if retErr != nil {
return
}
for i := range result {
visitImports(result, result[i], nil)
for i := range l.Roots {
visitImports(l.Roots, l.Roots[i], nil)
}
}()

newLoader := func(cfg *packages.Config, cfgDir string) *loader {
if cfg == nil {
cfg = &packages.Config{
Dir: cfgDir,
}
}
l := &loader{
cfg: cfg,
packages: map[*packages.Package]*Package{},
}
l.cfg.Mode |= packages.LoadImports | packages.NeedTypesSizes
if l.cfg.Fset == nil {
l.cfg.Fset = token.NewFileSet()
}
// put our build flags first so that callers can override them
l.cfg.BuildFlags = append([]string{"-tags", "ignore_autogenerated"}, l.cfg.BuildFlags...)

return l
}

// initialize the default loader
l := newLoader(cfg, "")

// uniquePkgIDs is used to keep track of the discovered packages to be nice
// and try and prevent packages from showing up twice when nested module
// support is enabled. there is not harm that comes from this per se, but
Expand All @@ -412,7 +401,7 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) (result []*Packa
// if validatePkgFn is nil, a package will be returned in the slice,
// otherwise the package is only returned if the result of
// validatePkgFn(pkg.ID) is truthy
loadPackages := func(l *loader, roots ...string) ([]*Package, error) {
loadPackages := func(roots ...string) ([]*Package, error) {
rawPkgs, err := packages.Load(l.cfg, roots...)
if err != nil {
return nil, err
Expand All @@ -430,11 +419,12 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) (result []*Packa

// if no roots were provided then load the current package and return early
if len(roots) == 0 {
pkgs, err := loadPackages(l)
pkgs, err := loadPackages()
if err != nil {
return nil, err
}
return pkgs, nil
l.Roots = append(l.Roots, pkgs...)
return l.Roots, nil
}

// pkgRoots is a slice of roots that are package/modules and fspRoots
Expand All @@ -460,39 +450,34 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) (result []*Packa
// system path roots due to them needing a custom, calculated value for the
// cfg.Dir field
if len(pkgRoots) > 0 {
pkgs, err := loadPackages(l, pkgRoots...)
pkgs, err := loadPackages(pkgRoots...)
if err != nil {
return nil, err
}
result = append(result, pkgs...)
l.Roots = append(l.Roots, pkgs...)
}

// if there are no filesystem path roots then go ahead and return early
if len(fspRoots) == 0 {
return result, nil
return l.Roots, nil
}

//
// at this point we are handling filesystem path roots
//

// store the value of cfg.Dir so we can use it later if it is non-empty.
// we need to store it now as the value of cfg.Dir will be updated by
// a loop below
var cfgDir string
if cfg != nil {
cfgDir = cfg.Dir
}

// ensure the cfg.Dir field is reset to its original value upon
// returning from this function. it should honestly be fine if it is
// not given most callers will not send in the cfg parameter directly,
// as it's largely for testing, but still, let's be good stewards.
defer func(d string) {
if cfg != nil {
cfg.Dir = d
}
}(cfgDir)
cfg.Dir = d
}(cfg.Dir)

// store the value of cfg.Dir so we can use it later if it is non-empty.
// we need to store it now as the value of cfg.Dir will be updated by
// a loop below
cfgDir := cfg.Dir

// addNestedGoModulesToRoots is given to filepath.WalkDir and adds the
// directory part of p to the list of filesystem path roots IFF p is the
Expand Down Expand Up @@ -591,16 +576,9 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) (result []*Packa
b = "."
}

// create the loader for this filesystem path. this is required in order
// to properly load the files as AST later in various workflows,
// including CRD generation
var fspLoader *loader
if cfg == nil {
fspLoader = newLoader(nil, d)
} else {
fspLoader = l
fspLoader.cfg.Dir = d
}
// update the loader configuration's Dir field to the directory part of
// the root
l.cfg.Dir = d

// update the root to be "./..." or "./."
// (with OS-specific filepath separator). please note filepath.Join
Expand All @@ -609,14 +587,14 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) (result []*Packa
r = fmt.Sprintf(".%s%s", string(filepath.Separator), b)

// load the packages from the roots
pkgs, err := loadPackages(fspLoader, r)
pkgs, err := loadPackages(r)
if err != nil {
return nil, err
}
result = append(result, pkgs...)
l.Roots = append(l.Roots, pkgs...)
}

return result, nil
return l.Roots, nil
}

// visitImports walks a dependency graph, replacing imported package
Expand All @@ -634,8 +612,8 @@ func visitImports(rootPkgs []*Package, pkg *Package, seen sets.String) {
}
}
if !seen.Has(importedPkgID) {
visitImports(rootPkgs, importedPkg, seen)
seen.Insert(importedPkgID)
visitImports(rootPkgs, importedPkg, seen)
}
}
}
Expand Down