fix(sources): skip non-regular files in scan_compiled_extensions

[pmattsso]

pathlib.Path.walk() lists symlinks to directories as filenames rather than dirnames. This causes scan_compiled_extensions to attempt to open() them, resulting in IsADirectoryError. Add an is_file() check to skip non-regular files before attempting to read their contents.

Closes: #1001

Closes: #1004

Pull Request Description

What

Why

• PR follows CONTRIBUTING.md guidelines

[coderabbitai]

📝 Walkthrough

Walkthrough

The scan_compiled_extensions() function in src/fromager/sources.py was changed to skip non-regular files before attempting to open them. After checking suffixes, it now calls if not filepath.is_file(): continue to avoid calling filepath.open("rb") on directory symlinks or other non-file entries. The suffix and magic-header detection logic is unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: adding an is_file() check to skip non-regular files in scan_compiled_extensions to prevent IsADirectoryError.
Linked Issues check	✅ Passed	The code change directly addresses issue #1001 by adding an is_file() check to skip non-regular files before attempting to open them, preventing IsADirectoryError on symlinked directories.
Out of Scope Changes check	✅ Passed	All changes are scoped to the scan_compiled_extensions() function in sources.py to address the symlink handling issue; no unrelated modifications detected.
Description check	✅ Passed	The pull request description directly addresses the changeset: it explains the symlink directory issue, the IsADirectoryError problem, and the solution (is_file() check) that matches the implementation in the code changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/fromager/sources.py (1)

868-871: Fix looks correct; consider adding test coverage for symlink edge case.

The is_file() check correctly handles symlinks to directories (returns False) while still processing symlinks to regular files (follows symlink, returns True). Broken symlinks are also safely skipped.

The existing test in tests/test_sources.py:304-314 doesn't cover this scenario. A regression test with a symlinked directory would prevent this from breaking again.

🧪 Suggested test addition

def test_scan_compiled_extensions_skips_symlinked_directory(tmp_path: pathlib.Path) -> None:
    """Ensure symlinks to directories don't cause IsADirectoryError."""
    real_dir = tmp_path / "real_dir"
    real_dir.mkdir()
    (real_dir / "file.c").write_bytes(b"/* source */")
    
    symlink_dir = tmp_path / "symlink_dir"
    symlink_dir.symlink_to(real_dir)
    
    # Should not raise IsADirectoryError
    matches = sources.scan_compiled_extensions(tmp_path)
    assert pathlib.Path("symlink_dir") not in matches

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/fromager/sources.py` around lines 868 - 871, The scan_compiled_extensions
loop currently guards against symlinked directories by checking
filepath.is_file(), but there's no test ensuring this behavior; add a regression
test for scan_compiled_extensions that creates a real directory with a file,
creates a symlink to that directory, calls sources.scan_compiled_extensions on
the parent tmp_path, and asserts that the symlinked directory name is not
included in the returned matches (and that no IsADirectoryError is raised);
reference the scan_compiled_extensions function and the filepath.is_file() check
when locating where this behavior is handled.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/fromager/sources.py`:
- Around line 868-871: The scan_compiled_extensions loop currently guards
against symlinked directories by checking filepath.is_file(), but there's no
test ensuring this behavior; add a regression test for scan_compiled_extensions
that creates a real directory with a file, creates a symlink to that directory,
calls sources.scan_compiled_extensions on the parent tmp_path, and asserts that
the symlinked directory name is not included in the returned matches (and that
no IsADirectoryError is raised); reference the scan_compiled_extensions function
and the filepath.is_file() check when locating where this behavior is handled.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5ff9a666-d708-4f26-adb8-add78418103c

📥 Commits

Reviewing files that changed from the base of the PR and between aec9c9c and a2969b2.

📒 Files selected for processing (1)

• src/fromager/sources.py

[tiran]

LGTM

[tiran]

@Mergifyio queue

[mergify]

queue

⚠️ Configuration not compatible with a branch protection setting

Details

The branch protection setting Require branches to be up to date before merging is not compatible with draft PR checks. To keep this branch protection enabled, update your Mergify configuration to enable in-place checks: set merge_queue.max_parallel_checks: 1, set every queue rule batch_size: 1, and avoid two-step CI (make merge_conditions identical to queue_conditions). Otherwise, disable this branch protection.

[tiran]

@Mergifyio rebase

[mergify]

rebase

☑️ Nothing to do, the required conditions are not met

Details

• any of:
  • #commits-behind > 0 [📌 rebase requirement]
  • -linear-history [📌 rebase requirement]
• -closed [📌 rebase requirement]
• -conflict [📌 rebase requirement]
• queue-position = -1 [📌 rebase requirement]

[tiran]

Mergify rules are broken, see #1005. I'm going to approve and merge this PR manually.