Skip to content

Commit 39e518a

Browse files
yongxiuYongxiu Cui
yongxiu
authored and
Yongxiu Cui
committed
Fix controller-tools doesn't support single files as input
#837
1 parent 881ffb4 commit 39e518a

File tree

2 files changed

+71
-69
lines changed

2 files changed

+71
-69
lines changed

‎pkg/loader/loader.go

+6-14
Original file line numberDiff line numberDiff line change
@@ -389,12 +389,6 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
389389
}
390390
}()
391391

392-
// uniquePkgIDs is used to keep track of the discovered packages to be nice
393-
// and try and prevent packages from showing up twice when nested module
394-
// support is enabled. there is not harm that comes from this per se, but
395-
// it makes testing easier when a known number of modules can be asserted
396-
uniquePkgIDs := sets.String{}
397-
398392
// loadPackages returns the Go packages for the provided roots
399393
//
400394
// if validatePkgFn is nil, a package will be returned in the slice,
@@ -412,10 +406,7 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
412406
var pkgs []*Package
413407
for _, rp := range rawPkgs {
414408
p := l.packageFor(rp)
415-
if !uniquePkgIDs.Has(p.ID) {
416-
pkgs = append(pkgs, p)
417-
uniquePkgIDs.Insert(p.ID)
418-
}
409+
pkgs = append(pkgs, p)
419410
}
420411
return pkgs, nil
421412
}
@@ -568,13 +559,14 @@ func LoadRootsWithConfig(cfg *packages.Config, roots ...string) ([]*Package, err
568559
for _, r := range fspRoots {
569560
b, d := filepath.Base(r), filepath.Dir(r)
570561

571-
// we want the base part of the path to be either "..." or ".", except
572-
// Go's filepath utilities clean paths during manipulation, removing the
573-
// ".". thus, if not "...", let's update the path components so that:
562+
// we want the base part of the path to be either "..." or ".", except Go's
563+
// filepath utilities clean paths during manipulation or go file path,
564+
// removing the ".". thus, if not "..." or go file, let's update the path
565+
// components so that:
574566
//
575567
// d = r
576568
// b = "."
577-
if b != "..." {
569+
if b != "..." && filepath.Ext(b) != ".go" {
578570
d = r
579571
b = "."
580572
}

‎pkg/loader/loader_test.go

+65-55
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,16 @@ var _ = Describe("Loader parsing root module", func() {
3333
testmodPkg = loaderPkg + "/testmod"
3434
)
3535

36-
var indexOfPackage = func(pkgID string, pkgs []*loader.Package) int {
37-
for i := range pkgs {
38-
if pkgs[i].ID == pkgID {
39-
return i
40-
}
41-
}
42-
return -1
36+
var assertPkgExists = func(pkgID string, pkgs map[string]struct{}) {
37+
Expect(pkgs).Should(HaveKey(pkgID))
4338
}
4439

45-
var assertPkgExists = func(pkgID string, pkgs []*loader.Package) {
46-
ExpectWithOffset(1, indexOfPackage(pkgID, pkgs)).Should(BeNumerically(">", -1))
40+
var dedupPkgs = func(pkgs []*loader.Package) map[string]struct{} {
41+
uniquePkgs := make(map[string]struct{})
42+
for _, p := range pkgs {
43+
uniquePkgs[p.ID] = struct{}{}
44+
}
45+
return uniquePkgs
4746
}
4847

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

7575
Context("with roots=[sigs.k8s.io/controller-tools/pkg/loader/testmod/...]", func() {
7676
It("should load six packages", func() {
7777
pkgs, err := loader.LoadRoots("sigs.k8s.io/controller-tools/pkg/loader/testmod/...")
7878
Expect(err).ToNot(HaveOccurred())
79-
Expect(pkgs).To(HaveLen(6))
80-
assertPkgExists(testmodPkg, pkgs)
81-
assertPkgExists(testmodPkg+"/subdir1", pkgs)
82-
assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
83-
assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
84-
assertPkgExists(testmodPkg+"/submod1", pkgs)
85-
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
79+
uniquePkgs := dedupPkgs(pkgs)
80+
Expect(uniquePkgs).To(HaveLen(6))
81+
assertPkgExists(testmodPkg, uniquePkgs)
82+
assertPkgExists(testmodPkg+"/subdir1", uniquePkgs)
83+
assertPkgExists(testmodPkg+"/subdir1/subdir1", uniquePkgs)
84+
assertPkgExists(testmodPkg+"/subdir1/subdir2", uniquePkgs)
85+
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
86+
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
8687
})
8788
})
8889

8990
Context("with roots=[sigs.k8s.io/controller-tools/pkg/loader/testmod/..., ./...]", func() {
9091
It("should load seven packages", func() {
9192
pkgs, err := loader.LoadRoots("sigs.k8s.io/controller-tools/pkg/loader/testmod/...", "./...")
9293
Expect(err).ToNot(HaveOccurred())
93-
Expect(pkgs).To(HaveLen(7))
94-
assertPkgExists(testmodPkg, pkgs)
95-
assertPkgExists(testmodPkg+"/subdir1", pkgs)
96-
assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
97-
assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
98-
assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
99-
assertPkgExists(testmodPkg+"/submod1", pkgs)
100-
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
94+
uniquePkgs := dedupPkgs(pkgs)
95+
Expect(uniquePkgs).To(HaveLen(7))
96+
assertPkgExists(testmodPkg, uniquePkgs)
97+
assertPkgExists(testmodPkg+"/subdir1", uniquePkgs)
98+
assertPkgExists(testmodPkg+"/subdir1/subdir1", uniquePkgs)
99+
assertPkgExists(testmodPkg+"/subdir1/subdir2", uniquePkgs)
100+
assertPkgExists(testmodPkg+"/subdir1/submod1", uniquePkgs)
101+
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
102+
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
101103
})
102104
})
103105
})
@@ -106,26 +108,29 @@ var _ = Describe("Loader parsing root module", func() {
106108
It("should load one package", func() {
107109
pkgs, err := loader.LoadRoots("../crd/.")
108110
Expect(err).ToNot(HaveOccurred())
109-
Expect(pkgs).To(HaveLen(1))
110-
assertPkgExists(pkgPkg+"/crd", pkgs)
111+
uniquePkgs := dedupPkgs(pkgs)
112+
Expect(uniquePkgs).To(HaveLen(1))
113+
assertPkgExists(pkgPkg+"/crd", uniquePkgs)
111114
})
112115
})
113116

114117
Context("with roots=[./]", func() {
115118
It("should load one package", func() {
116119
pkgs, err := loader.LoadRoots("./")
117120
Expect(err).ToNot(HaveOccurred())
118-
Expect(pkgs).To(HaveLen(1))
119-
assertPkgExists(loaderPkg, pkgs)
121+
uniquePkgs := dedupPkgs(pkgs)
122+
Expect(uniquePkgs).To(HaveLen(1))
123+
assertPkgExists(loaderPkg, uniquePkgs)
120124
})
121125
})
122126

123127
Context("with roots=[../../pkg/loader]", func() {
124128
It("should load one package", func() {
125129
pkgs, err := loader.LoadRoots("../../pkg/loader")
126130
Expect(err).ToNot(HaveOccurred())
127-
Expect(pkgs).To(HaveLen(1))
128-
assertPkgExists(loaderPkg, pkgs)
131+
uniquePkgs := dedupPkgs(pkgs)
132+
Expect(uniquePkgs).To(HaveLen(1))
133+
assertPkgExists(loaderPkg, uniquePkgs)
129134
})
130135
})
131136

@@ -135,57 +140,62 @@ var _ = Describe("Loader parsing root module", func() {
135140
"../../pkg/loader/../loader/testmod/...",
136141
"./testmod/./../testmod//.")
137142
Expect(err).ToNot(HaveOccurred())
138-
Expect(pkgs).To(HaveLen(7))
139-
assertPkgExists(testmodPkg, pkgs)
140-
assertPkgExists(testmodPkg+"/subdir1", pkgs)
141-
assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
142-
assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
143-
assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
144-
assertPkgExists(testmodPkg+"/submod1", pkgs)
145-
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
143+
uniquePkgs := dedupPkgs(pkgs)
144+
Expect(uniquePkgs).To(HaveLen(7))
145+
assertPkgExists(testmodPkg, uniquePkgs)
146+
assertPkgExists(testmodPkg+"/subdir1", uniquePkgs)
147+
assertPkgExists(testmodPkg+"/subdir1/subdir1", uniquePkgs)
148+
assertPkgExists(testmodPkg+"/subdir1/subdir2", uniquePkgs)
149+
assertPkgExists(testmodPkg+"/subdir1/submod1", uniquePkgs)
150+
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
151+
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
146152
})
147153
})
148154

149155
Context("with roots=[./testmod/...]", func() {
150156
It("should load seven packages", func() {
151157
pkgs, err := loader.LoadRoots("./testmod/...")
152158
Expect(err).ToNot(HaveOccurred())
153-
Expect(pkgs).To(HaveLen(7))
154-
assertPkgExists(testmodPkg, pkgs)
155-
assertPkgExists(testmodPkg+"/subdir1", pkgs)
156-
assertPkgExists(testmodPkg+"/subdir1/subdir1", pkgs)
157-
assertPkgExists(testmodPkg+"/subdir1/subdir2", pkgs)
158-
assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
159-
assertPkgExists(testmodPkg+"/submod1", pkgs)
160-
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
159+
uniquePkgs := dedupPkgs(pkgs)
160+
Expect(uniquePkgs).To(HaveLen(7))
161+
assertPkgExists(testmodPkg, uniquePkgs)
162+
assertPkgExists(testmodPkg+"/subdir1", uniquePkgs)
163+
assertPkgExists(testmodPkg+"/subdir1/subdir1", uniquePkgs)
164+
assertPkgExists(testmodPkg+"/subdir1/subdir2", uniquePkgs)
165+
assertPkgExists(testmodPkg+"/subdir1/submod1", uniquePkgs)
166+
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
167+
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
161168
})
162169
})
163170

164171
Context("with roots=[./testmod/subdir1/submod1/...]", func() {
165172
It("should load one package", func() {
166173
pkgs, err := loader.LoadRoots("./testmod/subdir1/submod1/...")
167174
Expect(err).ToNot(HaveOccurred())
168-
Expect(pkgs).To(HaveLen(1))
169-
assertPkgExists(testmodPkg+"/subdir1/submod1", pkgs)
175+
uniquePkgs := dedupPkgs(pkgs)
176+
Expect(uniquePkgs).To(HaveLen(1))
177+
assertPkgExists(testmodPkg+"/subdir1/submod1", uniquePkgs)
170178
})
171179
})
172180

173181
Context("with roots=[./testmod, ./testmod/submod1]", func() {
174182
It("should load two packages", func() {
175183
pkgs, err := loader.LoadRoots("./testmod", "./testmod/submod1")
176184
Expect(err).ToNot(HaveOccurred())
177-
Expect(pkgs).To(HaveLen(2))
178-
assertPkgExists(testmodPkg, pkgs)
179-
assertPkgExists(testmodPkg+"/submod1", pkgs)
185+
uniquePkgs := dedupPkgs(pkgs)
186+
Expect(uniquePkgs).To(HaveLen(2))
187+
assertPkgExists(testmodPkg, uniquePkgs)
188+
assertPkgExists(testmodPkg+"/submod1", uniquePkgs)
180189
})
181190
})
182191

183192
Context("with roots=[./testmod/submod1/subdir1/]", func() {
184193
It("should load one package", func() {
185194
pkgs, err := loader.LoadRoots("./testmod/submod1/subdir1/")
186195
Expect(err).ToNot(HaveOccurred())
187-
Expect(pkgs).To(HaveLen(1))
188-
assertPkgExists(testmodPkg+"/submod1/subdir1", pkgs)
196+
uniquePkgs := dedupPkgs(pkgs)
197+
Expect(uniquePkgs).To(HaveLen(1))
198+
assertPkgExists(testmodPkg+"/submod1/subdir1", uniquePkgs)
189199
})
190200
})
191201
})

0 commit comments

Comments
 (0)