Skip to content

Commit d878411

Browse files
committed
Enhance temporary package directory handling and debugging in BuildManager and SubfolderBuildConfig
This commit adds detailed debug logging to the BuildManager and SubfolderBuildConfig classes, improving visibility into the temporary package directory creation and file copying processes. It introduces checks for Python files in the temporary directory, including recursive verification, and implements exclude pattern handling during directory copying. Additionally, a new test is added to ensure exclude patterns do not incorrectly match pytest temporary directories, addressing a regression issue. These changes enhance the robustness and traceability of the build process.
1 parent d05b366 commit d878411

3 files changed

Lines changed: 303 additions & 4 deletions

File tree

src/python_package_folder/manager.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,12 +1153,24 @@ def build():
11531153
f"Temporary package directory does not exist: {temp_dir}. "
11541154
"This should have been created during prepare_build()."
11551155
)
1156+
# Debug: List all files in temp directory
1157+
all_files = list(temp_dir.rglob("*"))
1158+
all_files = [f for f in all_files if f.is_file()]
1159+
print(
1160+
f"DEBUG: Temp package directory {temp_dir} contains {len(all_files)} files: "
1161+
f"{[str(f.relative_to(temp_dir)) for f in all_files[:10]]}",
1162+
file=sys.stderr,
1163+
)
11561164
# Verify it contains Python files
11571165
py_files = list(temp_dir.glob("*.py"))
11581166
if not py_files:
1159-
raise RuntimeError(
1160-
f"Temporary package directory exists but contains no Python files: {temp_dir}"
1161-
)
1167+
# Also check recursively
1168+
py_files = list(temp_dir.rglob("*.py"))
1169+
if not py_files:
1170+
raise RuntimeError(
1171+
f"Temporary package directory exists but contains no Python files: {temp_dir}. "
1172+
f"Total files found: {len(all_files)}"
1173+
)
11621174
# Verify __init__.py exists
11631175
if not (temp_dir / "__init__.py").exists():
11641176
raise RuntimeError(

src/python_package_folder/subfolder_build.py

Lines changed: 209 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ def _create_temp_package_directory(self) -> None:
134134
This way, hatchling will install it with the correct name without needing force-include.
135135
"""
136136
if not self.package_name:
137+
print("DEBUG: No package_name provided, skipping temp package directory creation", file=sys.stderr)
137138
return
138139

139140
# Convert package name (with hyphens) to import name (with underscores)
@@ -144,13 +145,21 @@ def _create_temp_package_directory(self) -> None:
144145
# This way, hatchling will install it with the correct name
145146
import_name_dir = self.project_root / import_name
146147

148+
print(
149+
f"DEBUG: Creating temporary package directory: {import_name_dir} "
150+
f"(from src_dir: {self.src_dir}, import name: {import_name})",
151+
file=sys.stderr,
152+
)
153+
147154
# Check if the directory already exists and is the correct one
148155
if import_name_dir.exists() and import_name_dir == self._temp_package_dir:
149156
# Directory already exists and is the correct one, no need to recreate
157+
print(f"DEBUG: Temporary package directory already exists: {import_name_dir}", file=sys.stderr)
150158
return
151159

152160
# Remove if it already exists (from a previous build)
153161
if import_name_dir.exists():
162+
print(f"DEBUG: Removing existing temporary package directory: {import_name_dir}", file=sys.stderr)
154163
shutil.rmtree(import_name_dir)
155164

156165
# Copy the entire source directory contents directly to the import name directory
@@ -171,9 +180,38 @@ def _create_temp_package_directory(self) -> None:
171180
self._temp_package_dir = None
172181
return
173182

183+
# Get exclude patterns from parent pyproject.toml
184+
exclude_patterns = []
185+
original_pyproject = self.project_root / "pyproject.toml"
186+
if original_pyproject.exists():
187+
exclude_patterns = read_exclude_patterns(original_pyproject)
188+
print(
189+
f"DEBUG: Using exclude patterns for temp directory copy: {exclude_patterns}",
190+
file=sys.stderr,
191+
)
192+
193+
# Check if src_dir has any files before copying
194+
src_files = list(self.src_dir.rglob("*"))
195+
src_files = [f for f in src_files if f.is_file()]
196+
print(
197+
f"DEBUG: Source directory {self.src_dir} contains {len(src_files)} files before copy",
198+
file=sys.stderr,
199+
)
200+
201+
# Use a copy method that respects exclude patterns and handles missing directories
174202
try:
175-
shutil.copytree(self.src_dir, import_name_dir)
203+
print(f"DEBUG: Starting copy from {self.src_dir} to {import_name_dir}", file=sys.stderr)
204+
self._copytree_excluding_patterns(self.src_dir, import_name_dir, exclude_patterns)
176205
self._temp_package_dir = import_name_dir
206+
207+
# Verify files were copied
208+
copied_files = list(import_name_dir.rglob("*"))
209+
copied_files = [f for f in copied_files if f.is_file()]
210+
print(
211+
f"DEBUG: After copy, temp directory {import_name_dir} contains {len(copied_files)} files",
212+
file=sys.stderr,
213+
)
214+
177215
print(
178216
f"Created temporary package directory: {import_name_dir} "
179217
f"(import name: {import_name})"
@@ -183,8 +221,141 @@ def _create_temp_package_directory(self) -> None:
183221
f"Warning: Could not create temporary package directory: {e}",
184222
file=sys.stderr,
185223
)
224+
import traceback
225+
print(f"DEBUG: Traceback: {traceback.format_exc()}", file=sys.stderr)
186226
# Fall back to using src_dir directly
187227
self._temp_package_dir = None
228+
229+
def _copytree_excluding_patterns(self, src: Path, dst: Path, exclude_patterns: list[str]) -> None:
230+
"""
231+
Copy a directory tree, excluding certain patterns and handling missing directories gracefully.
232+
233+
This is similar to BuildManager._copytree_excluding but works without needing
234+
the BuildManager instance. It respects exclude patterns and skips missing directories
235+
(e.g., broken symlinks or already-excluded directories).
236+
237+
Args:
238+
src: Source directory
239+
dst: Destination directory
240+
exclude_patterns: List of patterns to exclude (e.g., ['_SS', '__SS', '.*_test.*'])
241+
"""
242+
default_patterns = [
243+
"_SS",
244+
"__SS",
245+
"_sandbox",
246+
"__sandbox",
247+
"_skip",
248+
"__skip",
249+
"_test",
250+
"__test__",
251+
]
252+
all_exclude_patterns = default_patterns + exclude_patterns
253+
254+
def should_exclude(path: Path) -> bool:
255+
"""Check if a path should be excluded."""
256+
import re
257+
# Only check parts of the path relative to src_dir, not the entire absolute path
258+
# This prevents matching test directory names or other parts outside the source
259+
try:
260+
rel_path = path.relative_to(src)
261+
# Check each component of the relative path
262+
for part in rel_path.parts:
263+
# Check if any part matches an exclusion pattern
264+
for pattern in all_exclude_patterns:
265+
# Determine if pattern is a regex (contains regex special characters)
266+
is_regex = any(c in pattern for c in ['.', '*', '+', '?', '^', '$', '[', ']', '(', ')', '{', '}', '|', '\\'])
267+
268+
if is_regex:
269+
# Use regex matching for patterns like '.*_test.*'
270+
try:
271+
if re.search(pattern, part):
272+
print(f"DEBUG: Excluding {path} (part '{part}' matches regex pattern '{pattern}')", file=sys.stderr)
273+
return True
274+
except re.error:
275+
# Invalid regex, fall back to simple string matching
276+
if part == pattern or part.startswith(pattern):
277+
print(f"DEBUG: Excluding {path} (part '{part}' matches pattern '{pattern}')", file=sys.stderr)
278+
return True
279+
else:
280+
# Simple string matching for patterns like '_SS'
281+
if part == pattern or part.startswith(pattern):
282+
print(f"DEBUG: Excluding {path} (part '{part}' matches pattern '{pattern}')", file=sys.stderr)
283+
return True
284+
except ValueError:
285+
# Path is not relative to src, check the name only
286+
for pattern in all_exclude_patterns:
287+
is_regex = any(c in pattern for c in ['.', '*', '+', '?', '^', '$', '[', ']', '(', ')', '{', '}', '|', '\\'])
288+
if is_regex:
289+
try:
290+
if re.search(pattern, path.name):
291+
return True
292+
except re.error:
293+
if path.name == pattern or path.name.startswith(pattern):
294+
return True
295+
else:
296+
if path.name == pattern or path.name.startswith(pattern):
297+
return True
298+
return False
299+
300+
# Create destination directory
301+
dst.mkdir(parents=True, exist_ok=True)
302+
303+
# Copy files and subdirectories, excluding patterns
304+
copied_count = 0
305+
excluded_count = 0
306+
skipped_count = 0
307+
try:
308+
items = list(src.iterdir())
309+
print(f"DEBUG: Copying from {src} to {dst}, found {len(items)} items", file=sys.stderr)
310+
for item in items:
311+
if should_exclude(item):
312+
print(f"DEBUG: Excluding {item} from temp package directory copy", file=sys.stderr)
313+
excluded_count += 1
314+
continue
315+
316+
src_item = src / item.name
317+
dst_item = dst / item.name
318+
319+
# Skip if source doesn't exist (broken symlink, already deleted, etc.)
320+
if not src_item.exists():
321+
print(
322+
f"DEBUG: Skipping non-existent item: {src_item}",
323+
file=sys.stderr,
324+
)
325+
skipped_count += 1
326+
continue
327+
328+
if src_item.is_file():
329+
try:
330+
shutil.copy2(src_item, dst_item)
331+
copied_count += 1
332+
print(f"DEBUG: Copied file {src_item} -> {dst_item}", file=sys.stderr)
333+
except (OSError, IOError) as e:
334+
print(
335+
f"DEBUG: Could not copy file {src_item}: {e}, skipping",
336+
file=sys.stderr,
337+
)
338+
skipped_count += 1
339+
continue
340+
elif src_item.is_dir():
341+
try:
342+
self._copytree_excluding_patterns(src_item, dst_item, exclude_patterns)
343+
copied_count += 1
344+
print(f"DEBUG: Copied directory {src_item} -> {dst_item}", file=sys.stderr)
345+
except (OSError, IOError) as e:
346+
print(
347+
f"DEBUG: Could not copy directory {src_item}: {e}, skipping",
348+
file=sys.stderr,
349+
)
350+
skipped_count += 1
351+
continue
352+
print(
353+
f"DEBUG: Copy summary: {copied_count} items copied, {excluded_count} excluded, {skipped_count} skipped",
354+
file=sys.stderr,
355+
)
356+
except (OSError, IOError) as e:
357+
# If we can't even iterate the source directory, that's a problem
358+
raise RuntimeError(f"Cannot iterate source directory {src}: {e}") from e
188359

189360
def _get_package_structure(self) -> tuple[str, list[str]]:
190361
"""
@@ -198,6 +369,13 @@ def _get_package_structure(self) -> tuple[str, list[str]]:
198369
# Use temporary package directory if it exists, otherwise use src_dir
199370
package_dir = self._temp_package_dir if self._temp_package_dir and self._temp_package_dir.exists() else self.src_dir
200371

372+
print(
373+
f"DEBUG: _get_package_structure: temp_package_dir={self._temp_package_dir}, "
374+
f"exists={self._temp_package_dir.exists() if self._temp_package_dir else False}, "
375+
f"using package_dir={package_dir}",
376+
file=sys.stderr,
377+
)
378+
201379
# Check if package_dir itself is a package (has __init__.py)
202380
has_init = (package_dir / "__init__.py").exists()
203381

@@ -211,6 +389,16 @@ def _get_package_structure(self) -> tuple[str, list[str]]:
211389
packages_path = str(rel_path).replace("\\", "/")
212390
except ValueError:
213391
packages_path = None
392+
print(
393+
f"DEBUG: Could not calculate relative path from {self.project_root} to {package_dir}",
394+
file=sys.stderr,
395+
)
396+
397+
print(
398+
f"DEBUG: _get_package_structure returning: packages_path={packages_path}, "
399+
f"has_init={has_init}, has_py_files={has_py_files}",
400+
file=sys.stderr,
401+
)
214402

215403
# If package_dir has Python files but no __init__.py, we need to make it a package
216404
# or include it as a module directory
@@ -518,8 +706,28 @@ def create_temp_pyproject(self) -> Path | None:
518706
# This will copy the __init__.py we just created (if any)
519707
self._create_temp_package_directory()
520708

709+
# Log the result of temp directory creation
710+
if self._temp_package_dir and self._temp_package_dir.exists():
711+
py_files = list(self._temp_package_dir.glob("*.py"))
712+
print(
713+
f"DEBUG: Temp package directory created successfully: {self._temp_package_dir}, "
714+
f"contains {len(py_files)} Python files",
715+
file=sys.stderr,
716+
)
717+
else:
718+
print(
719+
f"WARNING: Temp package directory was NOT created. "
720+
f"Will fall back to using src_dir: {self.src_dir}",
721+
file=sys.stderr,
722+
)
723+
521724
# Determine which directory to use (temp package dir or src_dir)
522725
package_dir = self._temp_package_dir if self._temp_package_dir and self._temp_package_dir.exists() else self.src_dir
726+
print(
727+
f"DEBUG: Using package_dir for build: {package_dir} "
728+
f"(temp_dir={self._temp_package_dir}, src_dir={self.src_dir})",
729+
file=sys.stderr,
730+
)
523731

524732
# Read the original pyproject.toml
525733
original_pyproject = self.project_root / "pyproject.toml"

tests/test_subfolder_build.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,85 @@ def test_temp_package_directory_handles_existing_directory(
11301130

11311131
config.restore()
11321132

1133+
def test_temp_package_directory_respects_exclude_patterns_without_matching_test_dirs(
1134+
self, test_project_with_pyproject: Path
1135+
) -> None:
1136+
"""
1137+
Test that exclude patterns don't incorrectly match pytest temp directory names.
1138+
1139+
This test ensures that exclude patterns like '.*test_.*' only match files/directories
1140+
within the source directory, not the pytest temp directory name (e.g.,
1141+
'test_real_world_ml_drawing_assistant_data_scenario').
1142+
1143+
This is a regression test for the bug where all files were excluded because the
1144+
exclude pattern matching checked the entire absolute path instead of just the
1145+
relative path within src_dir.
1146+
"""
1147+
project_root = test_project_with_pyproject
1148+
subfolder = project_root / "subfolder"
1149+
1150+
# Create files that should NOT be excluded
1151+
(subfolder / "__init__.py").write_text("# Package init")
1152+
(subfolder / "module.py").write_text("def func(): pass")
1153+
(subfolder / "utils.py").write_text("def util(): pass")
1154+
1155+
# Create a file that SHOULD be excluded (matches pattern)
1156+
(subfolder / "test_helper.py").write_text("def test(): pass")
1157+
(subfolder / "_SS").mkdir()
1158+
(subfolder / "_SS" / "excluded.py").write_text("# Should be excluded")
1159+
1160+
# Update pyproject.toml with exclude patterns that could match test directory names
1161+
pyproject_path = project_root / "pyproject.toml"
1162+
pyproject_content = pyproject_path.read_text()
1163+
# Add exclude patterns including ones that could match pytest temp dirs
1164+
if "[tool.python-package-folder]" not in pyproject_content:
1165+
pyproject_content += "\n[tool.python-package-folder]\n"
1166+
pyproject_content += 'exclude-patterns = ["_SS", "__SS", ".*_test.*", ".*test_.*", "sandbox"]\n'
1167+
pyproject_path.write_text(pyproject_content)
1168+
1169+
config = SubfolderBuildConfig(
1170+
project_root=project_root,
1171+
src_dir=subfolder,
1172+
version="1.0.0",
1173+
package_name="my-package",
1174+
)
1175+
1176+
# Create temp pyproject (which creates temp package directory with exclude patterns)
1177+
config.create_temp_pyproject()
1178+
1179+
temp_package_dir = config._temp_package_dir
1180+
assert temp_package_dir is not None
1181+
assert temp_package_dir.exists()
1182+
1183+
# Files that should NOT be excluded should be copied
1184+
assert (temp_package_dir / "__init__.py").exists(), (
1185+
"__init__.py should be copied (doesn't match exclude patterns)"
1186+
)
1187+
assert (temp_package_dir / "module.py").exists(), (
1188+
"module.py should be copied (doesn't match exclude patterns)"
1189+
)
1190+
assert (temp_package_dir / "utils.py").exists(), (
1191+
"utils.py should be copied (doesn't match exclude patterns)"
1192+
)
1193+
1194+
# Files that SHOULD be excluded should NOT be copied
1195+
assert not (temp_package_dir / "test_helper.py").exists(), (
1196+
"test_helper.py should be excluded (matches '.*test_.*' pattern)"
1197+
)
1198+
assert not (temp_package_dir / "_SS").exists(), (
1199+
"_SS directory should be excluded (matches '_SS' pattern)"
1200+
)
1201+
1202+
# Verify at least some files were copied (this would fail if all files were excluded)
1203+
all_files = list(temp_package_dir.rglob("*"))
1204+
all_files = [f for f in all_files if f.is_file()]
1205+
assert len(all_files) >= 3, (
1206+
f"Expected at least 3 files to be copied, but only found {len(all_files)}. "
1207+
f"This suggests exclude patterns are incorrectly matching test directory names."
1208+
)
1209+
1210+
config.restore()
1211+
11331212

11341213
class TestWheelPackaging:
11351214
"""Tests to verify that wheels are correctly packaged with the right directory structure."""

0 commit comments

Comments
 (0)