Skip to content

Commit cdbb47c

Browse files
jjbustamanteclaude
andcommitted
Fix project.toml directory exclusion behavior (#2402)
This change fixes unexpected behavior when excluding directories with trailing slash patterns in project.toml. Previously, patterns like "foo/" would exclude files within the directory but still create empty directory entries in the final image. Changes: - Enhanced getFileFilter() to properly exclude directories when patterns end with "/" - Modified WriteDirToTar() to skip entire directory trees using filepath.SkipDir - Fixed isNil() function in testhelpers to properly handle nil function pointers - Added comprehensive tests for directory exclusion scenarios The fix ensures that when users specify exclude = ["foo/"] in project.toml, the directory "foo" and all its contents are completely excluded from the build process, eliminating the unexpected empty directory artifacts. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 57cf5b9 commit cdbb47c

5 files changed

Lines changed: 118 additions & 3 deletions

File tree

pkg/archive/archive.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,10 @@ func WriteDirToTar(tw TarWriter, srcDir, basePath string, uid, gid int, mode int
179179
return err
180180
}
181181
if !fileFilter(relPath) {
182+
// If this is a directory that doesn't match the filter, skip the entire directory tree
183+
if fi != nil && fi.IsDir() {
184+
return filepath.SkipDir
185+
}
182186
return nil
183187
}
184188
}

pkg/archive/archive_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,32 @@ func testArchive(t *testing.T, when spec.G, it spec.S) {
312312
}
313313
})
314314

315+
it("excludes directories and their contents when directory is filtered", func() {
316+
tarFile := filepath.Join(tmpDir, "some.tar")
317+
fh, err := os.Create(tarFile)
318+
h.AssertNil(t, err)
319+
320+
tw := tar.NewWriter(fh)
321+
322+
err = archive.WriteDirToTar(tw, src, "/nested/dir/dir-in-archive", 1234, 2345, 0777, true, false, func(path string) bool {
323+
// Exclude the sub-dir directory entirely
324+
return path != "sub-dir"
325+
})
326+
h.AssertNil(t, err)
327+
h.AssertNil(t, tw.Close())
328+
h.AssertNil(t, fh.Close())
329+
330+
file, err := os.Open(filepath.Join(tmpDir, "some.tar"))
331+
h.AssertNil(t, err)
332+
defer file.Close()
333+
334+
tr := tar.NewReader(file)
335+
336+
verify := h.NewTarVerifier(t, tr, 1234, 2345)
337+
verify.NextFile("/nested/dir/dir-in-archive/some-file.txt", "some-content", int64(os.ModePerm))
338+
verify.NoMoreFilesExist()
339+
})
340+
315341
it("filter is only handed relevant section of the filepath", func() {
316342
tarFile := filepath.Join(tmpDir, "some.tar")
317343
fh, err := os.Create(tarFile)

pkg/client/build.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -894,12 +894,31 @@ func getFileFilter(descriptor projectTypes.Descriptor) (func(string) bool, error
894894
if len(descriptor.Build.Exclude) > 0 {
895895
excludes := ignore.CompileIgnoreLines(descriptor.Build.Exclude...)
896896
return func(fileName string) bool {
897-
return !excludes.MatchesPath(fileName)
897+
// Check if the file/directory matches directly
898+
if excludes.MatchesPath(fileName) {
899+
return false
900+
}
901+
// For directories, also check if adding a trailing slash would match
902+
// This handles patterns like "foo/" that should exclude the "foo" directory
903+
if excludes.MatchesPath(fileName + "/") {
904+
return false
905+
}
906+
return true
898907
}, nil
899908
}
900909
if len(descriptor.Build.Include) > 0 {
901910
includes := ignore.CompileIgnoreLines(descriptor.Build.Include...)
902-
return includes.MatchesPath, nil
911+
return func(fileName string) bool {
912+
// Check if the file/directory matches directly
913+
if includes.MatchesPath(fileName) {
914+
return true
915+
}
916+
// For directories, also check if adding a trailing slash would match
917+
if includes.MatchesPath(fileName + "/") {
918+
return true
919+
}
920+
return false
921+
}, nil
903922
}
904923

905924
return nil, nil

pkg/client/build_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,65 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
156156
h.AssertNilE(t, fakeLifecycleImage.Cleanup())
157157
})
158158

159+
when("#getFileFilter", func() {
160+
when("exclude patterns", func() {
161+
it("creates filter that excludes files matching patterns", func() {
162+
descriptor := projectTypes.Descriptor{
163+
Build: projectTypes.Build{
164+
Exclude: []string{"*.log", "tmp/"},
165+
},
166+
}
167+
168+
filter, err := getFileFilter(descriptor)
169+
h.AssertNil(t, err)
170+
h.AssertNotNil(t, filter)
171+
172+
// Files matching patterns should be excluded (filter returns false)
173+
h.AssertEq(t, filter("app.log"), false)
174+
h.AssertEq(t, filter("tmp/cache.dat"), false)
175+
h.AssertEq(t, filter("tmp"), false) // Directory "tmp" should be excluded due to "tmp/" pattern
176+
177+
// Files not matching patterns should be included (filter returns true)
178+
h.AssertEq(t, filter("app.js"), true)
179+
h.AssertEq(t, filter("src/main.go"), true)
180+
})
181+
})
182+
183+
when("include patterns", func() {
184+
it("creates filter that includes only files matching patterns", func() {
185+
descriptor := projectTypes.Descriptor{
186+
Build: projectTypes.Build{
187+
Include: []string{"src/", "*.md"},
188+
},
189+
}
190+
191+
filter, err := getFileFilter(descriptor)
192+
h.AssertNil(t, err)
193+
h.AssertNotNil(t, filter)
194+
195+
// Files matching patterns should be included (filter returns true)
196+
h.AssertEq(t, filter("src/main.go"), true)
197+
h.AssertEq(t, filter("README.md"), true)
198+
199+
// Files not matching patterns should be excluded (filter returns false)
200+
h.AssertEq(t, filter("app.log"), false)
201+
h.AssertEq(t, filter("tmp/cache.dat"), false)
202+
})
203+
})
204+
205+
when("both exclude and include are empty", func() {
206+
it("returns nil filter", func() {
207+
descriptor := projectTypes.Descriptor{
208+
Build: projectTypes.Build{},
209+
}
210+
211+
filter, err := getFileFilter(descriptor)
212+
h.AssertNil(t, err)
213+
h.AssertNil(t, filter)
214+
})
215+
})
216+
})
217+
159218
when("#Build", func() {
160219
when("ephemeral builder is not needed", func() {
161220
it("does not create one", func() {

testhelpers/testhelpers.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,14 @@ func AssertTarball(t *testing.T, path string) {
327327
}
328328

329329
func isNil(value interface{}) bool {
330-
return value == nil || (reflect.TypeOf(value).Kind() == reflect.Ptr && reflect.ValueOf(value).IsNil())
330+
if value == nil {
331+
return true
332+
}
333+
rv := reflect.ValueOf(value)
334+
kind := rv.Kind()
335+
// Check for types that can be nil
336+
return (kind == reflect.Ptr || kind == reflect.Func || kind == reflect.Interface ||
337+
kind == reflect.Chan || kind == reflect.Map || kind == reflect.Slice) && rv.IsNil()
331338
}
332339

333340
func hasMatches(actual, exp string) bool {

0 commit comments

Comments
 (0)