Skip to content

Phase 1: Add git ls-files integration to packager - #293

Closed
GargiGupta-io wants to merge 1 commit into
keras-team:mainfrom
GargiGupta-io:phase-1-git-integration
Closed

Phase 1: Add git ls-files integration to packager#293
GargiGupta-io wants to merge 1 commit into
keras-team:mainfrom
GargiGupta-io:phase-1-git-integration

Conversation

@GargiGupta-io

Copy link
Copy Markdown

Adds helper functions to support git-aware packaging:

  • _list_git_files(): Lists tracked and non-ignored untracked files
  • _path_is_excluded(): Checks if a path should be excluded
  • _write_git_files(): Recursively writes files to ZIP respecting exclusions

Part of #288 (Phase 1 of 4). Merge this first, then #290, #291, #292 in order.

Adds _list_git_files(), _path_is_excluded(), and _write_git_files() functions to support git-aware packaging. These helpers enable respecting .gitignore when zipping working directories.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces helper functions (_list_git_files, _path_is_excluded, and _write_git_files) to package Git-tracked and non-ignored files into a zip archive. The review feedback correctly identifies an issue where comparing relative file paths against absolute exclusion paths will cause exclusions to be silently ignored, and provides a robust solution using os.path.abspath.

Comment thread kinetic/utils/packager.py
Comment on lines +49 to +56
def _path_is_excluded(path: str, exclude_paths: set[str]) -> bool:
if not exclude_paths:
return False
normalized_path = os.path.normpath(path)
return any(
normalized_path == excluded or normalized_path.startswith(excluded + os.sep)
for excluded in exclude_paths
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If base_dir is passed as a relative path (e.g., . or ./src), file_path will be constructed as a relative path. However, exclude_paths are absolute paths (as specified in the zip_working_dir docstring). Comparing a relative path with an absolute path using os.path.normpath will fail to match, causing exclusions to be silently ignored.

Converting path to an absolute path using os.path.abspath before performing the comparison ensures that exclusions are correctly respected regardless of whether base_dir is relative or absolute.

Suggested change
def _path_is_excluded(path: str, exclude_paths: set[str]) -> bool:
if not exclude_paths:
return False
normalized_path = os.path.normpath(path)
return any(
normalized_path == excluded or normalized_path.startswith(excluded + os.sep)
for excluded in exclude_paths
)
def _path_is_excluded(path: str, exclude_paths: set[str]) -> bool:
if not exclude_paths:
return False
abs_path = os.path.abspath(path)
return any(
abs_path == excluded or abs_path.startswith(excluded + os.sep)
for excluded in exclude_paths
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant