Phase 3: Add comprehensive tests for gitignore functionality

kinetic/utils/packager.py

@@ -9,6 +9,8 @@
 import fnmatch
 import json
 import os
+import posixpath
+import subprocess
 import sys
 import zipfile
 from collections import defaultdict
@@ -50,6 +52,77 @@
 # Reserved archive path carrying the client's packaging plan.
 _PLAN_ARCHIVE_NAME = ".kinetic/plan.json"
 
+
+def _list_git_files(base_dir: str) -> list[str] | None:
+  """List tracked and non-ignored untracked files under ``base_dir``."""
+  try:
+    result = subprocess.run(
+      [
+        "git",
+        "-C",
+        base_dir,
+        "ls-files",
+        "--cached",
+        "--others",
+        "--exclude-standard",
+        "-z",
+        "--",
+        ".",
+      ],
+      check=True,
+      stdout=subprocess.PIPE,
+      stderr=subprocess.DEVNULL,
+    )
+  except (OSError, subprocess.CalledProcessError):
+    return None
+
+  return [os.fsdecode(path) for path in result.stdout.split(b"\0") if path]
+
+
+def _path_is_excluded(path: str, exclude_paths: set[str]) -> bool:
+  """Check if a path should be excluded from archiving."""
+  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 _write_git_files(
+  zipf: zipfile.ZipFile,
+  base_dir: str,
+  git_files: list[str],
+  exclude_paths: set[str],
+  archive_prefix: str = "",
+) -> None:
+  """Write files from git ls-files to ZIP, respecting exclusions."""
+  for relative_path in git_files:
+    file_path = os.path.join(base_dir, relative_path)
+    if _path_is_excluded(file_path, exclude_paths) or not os.path.lexists(
+      file_path
+    ):
+      continue
+
+    archive_name = posixpath.join(archive_prefix, relative_path)
+    if os.path.isdir(file_path) and not os.path.islink(file_path):
+      nested_files = _list_git_files(file_path)
+      if nested_files is not None:
+        _write_git_files(
+          zipf,
+          file_path,
+          nested_files,
+          exclude_paths,
+          archive_prefix=archive_name,
+        )
+      continue
+    try:
+      zipf.write(file_path, archive_name)
+    except OSError as e:
+      logging.warning("Could not archive %s: %s", file_path, e)
+
+
 _MB = 1024 * 1024
 _DEFAULT_CONTEXT_SIZE_WARN_MB = 100.0
 _DEFAULT_PAYLOAD_SIZE_WARN_MB = 50.0
@@ -144,6 +217,10 @@ def zip_working_dir(
 ) -> None:
   """Zip a directory into a ZIP archive, excluding common non-source files.
 
+  When in a git repository, respects ``.gitignore`` and uses git ls-files
+  to determine which files to include. Falls back to directory traversal
+  with ``.kineticignore`` patterns when not in a git repo.
+
   Symlinked directories are followed (with a cycle guard), empty
   directories are preserved, and files that cannot be archived (broken
   symlinks, unreadable files) are skipped with a warning instead of
@@ -181,6 +258,51 @@ def zip_working_dir(
   with zipfile.ZipFile(
     output_path, "w", zipfile.ZIP_DEFLATED, strict_timestamps=False
   ) as zipf:
+    # Try git ls-files first if in a git repository
+    git_files = _list_git_files(base_dir)
+    if git_files is not None:
+      for relative_path in git_files:
+        file_path = os.path.join(base_dir, relative_path)
+        if _path_is_excluded(file_path, normalized_excludes) or not os.path.lexists(
+          file_path
+        ):
+          continue
+
+        archive_name = relative_path
+        if os.path.isdir(file_path) and not os.path.islink(file_path):
+          # Empty directories are preserved in git mode
+          rel_dir = archive_name.replace(os.sep, "/")
+          info = zipfile.ZipInfo(rel_dir + "/")
+          info.external_attr = (0o40755 << 16) | 0x10
+          zipf.writestr(info, b"")
+          continue
+
+        try:
+          size = os.path.getsize(file_path)
+          zipf.write(file_path, archive_name)
+          archived.append((size, archive_name))
+          if _is_secret_name(os.path.basename(file_path)):
+            secrets.append(archive_name)
+        except (OSError, ValueError, UnicodeEncodeError) as e:
+          logging.warning("Skipping %s: %s", file_path, e)
+
+      if plan_json is not None:
+        zipf.writestr(
+          _PLAN_ARCHIVE_NAME, json.dumps(plan_json, indent=2, default=str)
+        )
+      _report_context_size(output_path, archived)
+      if secrets:
+        logging.warning(
+          "Credential-shaped files are being uploaded with your code: %s. They "
+          "will be stored in the job's Cloud Storage bucket. Add them to a "
+          "%s file at %s to keep them out of the archive.",
+          ", ".join(sorted(secrets)),
+          _KINETICIGNORE,
+          base_dir,
+        )
+      return
+
+    # Fall back to directory traversal with os.walk
     for root, dirs, files in os.walk(base_dir, followlinks=True):
       kept_dirs = []
       for name in dirs:

kinetic/utils/packager_test.py

@@ -1093,5 +1093,76 @@ def test_replaced_structure_pickles(self):
     self.assertIsInstance(restored_kwargs["items"], ListSubclass)
 
 
+class TestGitIntegration(absltest.TestCase):
+  """Tests for git ls-files integration."""
+
+  def _make_temp_path(self):
+    td = tempfile.TemporaryDirectory()
+    self.addCleanup(td.cleanup)
+    return pathlib.Path(td.name)
+
+  def test_list_git_files_in_repo(self):
+    """Test _list_git_files works in a git repository."""
+    import subprocess
+
+    from kinetic.utils.packager import _list_git_files
+
+    tmp_path = self._make_temp_path()
+    src = tmp_path / "repo"
+    src.mkdir()
+
+    # Initialize git repo
+    subprocess.run(["git", "-C", str(src), "init"], check=True, capture_output=True)
+    subprocess.run(
+      ["git", "-C", str(src), "config", "user.email", "test@example.com"],
+      check=True,
+      capture_output=True,
+    )
+    subprocess.run(
+      ["git", "-C", str(src), "config", "user.name", "Test User"],
+      check=True,
+      capture_output=True,
+    )
+
+    # Add files
+    (src / "file1.py").write_text("code")
+    (src / "file2.txt").write_text("data")
+    (src / ".gitignore").write_text("*.pyc\n")
+
+    subprocess.run(
+      ["git", "-C", str(src), "add", "."], check=True, capture_output=True
+    )
+
+    files = _list_git_files(str(src))
+    self.assertIsNotNone(files)
+    self.assertIn("file1.py", files)
+    self.assertIn("file2.txt", files)
+    self.assertIn(".gitignore", files)
+
+  def test_list_git_files_not_in_repo(self):
+    """Test _list_git_files returns None when not in a git repository."""
+    from kinetic.utils.packager import _list_git_files
+
+    tmp_path = self._make_temp_path()
+    src = tmp_path / "not_repo"
+    src.mkdir()
+    (src / "file.py").write_text("code")
+
+    files = _list_git_files(str(src))
+    self.assertIsNone(files)
+
+  def test_path_is_excluded(self):
+    """Test _path_is_excluded checks exclude paths correctly."""
+    from kinetic.utils.packager import _path_is_excluded
+
+    exclude_paths = {"/tmp/data", "/tmp/cache"}
+
+    self.assertTrue(_path_is_excluded("/tmp/data/file.txt", exclude_paths))
+    self.assertTrue(_path_is_excluded("/tmp/data", exclude_paths))
+    self.assertTrue(_path_is_excluded("/tmp/cache/subdir/file.txt", exclude_paths))
+    self.assertFalse(_path_is_excluded("/tmp/other/file.txt", exclude_paths))
+    self.assertFalse(_path_is_excluded("/tmp/datafile.txt", exclude_paths))
+
+
 if __name__ == "__main__":
   absltest.main()