Skip to content

Commit

Permalink
Fix controller-tools doesn't support single files as input
Browse files Browse the repository at this point in the history 
kubernetes-sigs#837
  • Loading branch information
@yongxiu
yongxiu authored and Yongxiu Cui committed Nov 30, 2023
1 parent 881ffb4 commit 39e518a
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 69 deletions.
20 changes: 6 additions & 14 deletions pkg/loader/loader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,12 +389,6 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
}
}()

// 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
// it makes testing easier when a known number of modules can be asserted
uniquePkgIDs := sets.String{}

// loadPackages returns the Go packages for the provided roots
//
// if validatePkgFn is nil, a package will be returned in the slice,
Expand All @@ -412,10 +406,7 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
var pkgs []*Package
for _, rp := range rawPkgs {
p := l.packageFor(rp)
if !uniquePkgIDs.Has(p.ID) {
pkgs = append(pkgs, p)
uniquePkgIDs.Insert(p.ID)
}
pkgs = append(pkgs, p)
}
return pkgs, nil
}
Expand Down Expand Up @@ -568,13 +559,14 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
for _, r := range fspRoots {
b, d := filepath.Base(r), filepath.Dir(r)

// we want the base part of the path to be either "..." or ".", except
// Go's filepath utilities clean paths during manipulation, removing the
// ".". thus, if not "...", let's update the path components so that:
// we want the base part of the path to be either "..." or ".", except Go's
// filepath utilities clean paths during manipulation or go file path,
// removing the ".". thus, if not "..." or go file, let's update the path
// components so that:
//
// d = r
// b = "."
if b != "..." {
if b != "..." && filepath.Ext(b) != ".go" {
d = r
b = "."
}
Expand Down
120 changes: 65 additions & 55 deletions pkg/loader/loader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,16 @@ var _ = Describe("Loader parsing root module", func() {
testmodPkg = loaderPkg + "/testmod"
)

var indexOfPackage = func(pkgID string, pkgs []*loader.Package) int {
for i := range pkgs {
if pkgs[i].ID == pkgID {
return i
}
}
return -1
var assertPkgExists = func(pkgID string, pkgs map[string]struct{}) {
Expect(pkgs).Should(HaveKey(pkgID))
}

var assertPkgExists = func(pkgID string, pkgs []*loader.Package) {
ExpectWithOffset(1, indexOfPackage(pkgID, pkgs)).Should(BeNumerically(">", -1))
var dedupPkgs = func(pkgs []*loader.Package) map[string]struct{} {
uniquePkgs := make(map[string]struct{})
for _, p := range pkgs {
uniquePkgs[p.ID] = struct{}{}
}
return uniquePkgs
}

Context("with named packages/modules", func() {
Expand All @@ -67,37 +66,40 @@ var _ = Describe("Loader parsing root module", func() {
It("should load one package", func() {
pkgs, err := loader.LoadRoots("sigs.k8s.io/controller-tools/pkg/loader/testmod/submod1")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(1))
assertPkgExists(testmodPkg+"/submod1", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(1))
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
})
})

Context("with roots=[sigs.k8s.io/controller-tools/pkg/loader/testmod/...]", func() {
It("should load six packages", func() {
pkgs, err := loader.LoadRoots("sigs.k8s.io/controller-tools/pkg/loader/testmod/...")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(6))
assertPkgExists(testmodPkg, pkgs)
assertPkgExists(testmodPkg+"/subdir1", pkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
assertPkgExists(testmodPkg+"/submod1", pkgs)
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(6))
assertPkgExists(testmodPkg, uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir1", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir2", uniquePkgs)
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
})
})

Context("with roots=[sigs.k8s.io/controller-tools/pkg/loader/testmod/..., ./...]", func() {
It("should load seven packages", func() {
pkgs, err := loader.LoadRoots("sigs.k8s.io/controller-tools/pkg/loader/testmod/...", "./...")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(7))
assertPkgExists(testmodPkg, pkgs)
assertPkgExists(testmodPkg+"/subdir1", pkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
assertPkgExists(testmodPkg+"/submod1", pkgs)
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(7))
assertPkgExists(testmodPkg, uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir1", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir2", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/submod1", uniquePkgs)
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
})
})
})
Expand All @@ -106,26 +108,29 @@ var _ = Describe("Loader parsing root module", func() {
It("should load one package", func() {
pkgs, err := loader.LoadRoots("../crd/.")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(1))
assertPkgExists(pkgPkg+"/crd", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(1))
assertPkgExists(pkgPkg+"/crd", uniquePkgs)
})
})

Context("with roots=[./]", func() {
It("should load one package", func() {
pkgs, err := loader.LoadRoots("./")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(1))
assertPkgExists(loaderPkg, pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(1))
assertPkgExists(loaderPkg, uniquePkgs)
})
})

Context("with roots=[../../pkg/loader]", func() {
It("should load one package", func() {
pkgs, err := loader.LoadRoots("../../pkg/loader")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(1))
assertPkgExists(loaderPkg, pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(1))
assertPkgExists(loaderPkg, uniquePkgs)
})
})

Expand All @@ -135,57 +140,62 @@ var _ = Describe("Loader parsing root module", func() {
"../../pkg/loader/../loader/testmod/...",
"./testmod/./../testmod//.")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(7))
assertPkgExists(testmodPkg, pkgs)
assertPkgExists(testmodPkg+"/subdir1", pkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
assertPkgExists(testmodPkg+"/submod1", pkgs)
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(7))
assertPkgExists(testmodPkg, uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir1", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir2", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/submod1", uniquePkgs)
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
})
})

Context("with roots=[./testmod/...]", func() {
It("should load seven packages", func() {
pkgs, err := loader.LoadRoots("./testmod/...")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(7))
assertPkgExists(testmodPkg, pkgs)
assertPkgExists(testmodPkg+"/subdir1", pkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
assertPkgExists(testmodPkg+"/submod1", pkgs)
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(7))
assertPkgExists(testmodPkg, uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir1", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/subdir2", uniquePkgs)
assertPkgExists(testmodPkg+"/subdir1/submod1", uniquePkgs)
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
})
})

Context("with roots=[./testmod/subdir1/submod1/...]", func() {
It("should load one package", func() {
pkgs, err := loader.LoadRoots("./testmod/subdir1/submod1/...")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(1))
assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(1))
assertPkgExists(testmodPkg+"/subdir1/submod1", uniquePkgs)
})
})

Context("with roots=[./testmod, ./testmod/submod1]", func() {
It("should load two packages", func() {
pkgs, err := loader.LoadRoots("./testmod", "./testmod/submod1")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(2))
assertPkgExists(testmodPkg, pkgs)
assertPkgExists(testmodPkg+"/submod1", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(2))
assertPkgExists(testmodPkg, uniquePkgs)
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
})
})

Context("with roots=[./testmod/submod1/subdir1/]", func() {
It("should load one package", func() {
pkgs, err := loader.LoadRoots("./testmod/submod1/subdir1/")
Expect(err).ToNot(HaveOccurred())
Expect(pkgs).To(HaveLen(1))
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
uniquePkgs := dedupPkgs(pkgs)
Expect(uniquePkgs).To(HaveLen(1))
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
})
})
})

0 comments on commit 39e518a

Please sign in to comment.