scikit-build-core icon indicating copy to clipboard operation
scikit-build-core copied to clipboard

Prevent infinite loops in build_sdist

Open oplik0 opened this issue 6 months ago • 2 comments

As a result of #362 build_sdist now follows symlinks - however, due to the way each_unignored_file is written it will iterate through every subdirectory with no way to prevent it - as without mutating dirnames or any additional way of ending the loop, os.walk will iterate through everything no matter what patterns were excluded.

This becomes a problem when it encounters a circular pattern with a symlink, e.g.: something like this:

my_package/
├─ some_subfolder/
│  ├─ my_package -> ../my_package

will result in an infinite loop and prevent build_sdist from ever finishing without any warning or even debug log beyond the phase starting. No sdist.exclude can help here, as even the most general pattern will end up just ignoring files endlessly. You could exclude ** and still be stuck in an infinite loop.

To solve this each_unignored_file could actually ignore the directories that match exclusions, e.g.

for dirstr, dirnames, filenames in os.walk(str(starting_path), followlinks=True):
    dirpath = Path(dirstr)
    for i, dirname in enumerate(dirnames):
        # Always include something included
        if include_spec.match_file(p):
            continue

        # Always exclude something excluded
        if user_exclude_spec.match_file(p):
            del dirnames[i]

        # Ignore from global ignore
        if global_exclude_spec.match_file(p):
            del dirnames[i]

        # Ignore built-in patterns
        if builtin_exclude_spec.match_file(p):
            del dirnames[i]

This would lead to a slight issue with including a file within excluded directories, though prioritizing include here too would allow for a workaround at least (include the dir, exclude a glob for its children,, include the child you want)

oplik0 avatar Jun 06 '25 14:06 oplik0

This is https://github.com/scikit-build/scikit-build-core/pull/1048, which we need to gate behind the version requirement, since it changes behavior.

henryiii avatar Jun 06 '25 14:06 henryiii

Thanks, I was looking at issues and didn't see anything, so I didn't think to check PRs

oplik0 avatar Jun 09 '25 13:06 oplik0