fix: exclude OS-specific files from kit init Kitfile generation#1099
fix: exclude OS-specific files from kit init Kitfile generation#1099rishi-jat wants to merge 6 commits intokitops-ml:mainfrom
Conversation
|
/cc @akagami-harsh |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where kit init was including OS-generated metadata files (like .DS_Store, Thumbs.db) in generated Kitfiles, often resulting in unnecessary catch-all code layers (path: .). The solution filters out these OS-specific files during directory scanning.
Changes:
- Added
osSpecificFileslist containing 9 known OS-generated filenames from macOS and Windows - Added
shouldIgnoreFile()helper function with case-insensitive filename matching - Integrated filtering into
genDirListingFromPath()to exclude OS files before file classification
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // shouldIgnoreFile returns true if the file should be ignored during Kitfile generation. | ||
| // This filters out OS-specific metadata files that users did not intentionally create. | ||
| func shouldIgnoreFile(name string) bool { | ||
| for _, osFile := range osSpecificFiles { | ||
| if strings.EqualFold(name, osFile) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
The new shouldIgnoreFile function lacks test coverage. The codebase has comprehensive automated testing for this package (see generate_test.go and integration tests in testing/generate-kitfile_test.go). Consider adding unit tests to verify case-insensitive matching of OS-specific filenames, including edge cases like different case variations (e.g., ".ds_store", "THUMBS.DB") and non-matching files.
| var osSpecificFiles = []string{ | ||
| ".DS_Store", // macOS | ||
| ".AppleDouble", // macOS | ||
| ".LSOverride", // macOS | ||
| ".Spotlight-V100", // macOS | ||
| ".Trashes", // macOS | ||
| "Thumbs.db", // Windows | ||
| "desktop.ini", // Windows | ||
| "$RECYCLE.BIN", // Windows | ||
| "ehthumbs.db", // Windows | ||
| } |
There was a problem hiding this comment.
Consider whether the HuggingFace integration (pkg/lib/hf/list.go:106-107) should also filter OS-specific files for consistency. Currently it only filters .gitignore and .gitattributes, but downloaded HuggingFace repositories might also contain OS metadata files. This could be addressed in a follow-up PR by having the HF integration also call shouldIgnoreFile (which would need to be exported or moved to a shared location).
|
The OS files that are now filtered from kit init will still be packed by Have you considered ways to leverage the existing |
|
@gorkem Thanks for the feedback. The scan logic now ignores hidden files and OS metadata files during Kitfile generation. This prevents the catch-all code layer ( The ignore helper is also exported as |
Filter out OS-generated metadata files (.DS_Store, Thumbs.db, etc.) during directory scanning in kit init to prevent spurious catch-all code layers being added to the generated Kitfile. Signed-off-by: Rishi Jat <rishijat098@gmail.com>
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
e5b282f to
53e2659
Compare
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
|
@amisevsk please have a look. |
Description
Running
kit initpreviously included OS-generated metadata files (.DS_Store,Thumbs.db, etc.) in the generated Kitfile. This often resulted in unnecessary catch-all code layers (path: .) being added solely to include these hidden files.Changes
osSpecificFileslist containing known OS-generated filenames (macOS and Windows)shouldIgnoreFile()helper function with case-insensitive matchinggenDirListingFromPath()to skip OS files before classificationFiles Ignored
.DS_Store.AppleDouble.LSOverride.Spotlight-V100.TrashesThumbs.dbdesktop.ini$RECYCLE.BINehthumbs.dbTesting
kit initwith only OS files → no code layer generatedkit initwith model + OS files → only model included, no catch-allkit initwith normal files → behavior unchangedkit packunaffectedgo test ./pkg/...)Linked issues
Fixes #1081