Skip to content

Follow symlinked source directories - #116

Open
alganet wants to merge 1 commit into
swoole:masterfrom
alganet:fix/follow-symlinked-source-dirs
Open

alganet wants to merge 1 commit into
swoole:masterfrom
alganet:fix/follow-symlinked-source-dirs

Conversation

@alganet

@alganet alganet commented Sep 17, 2026

Copy link
Copy Markdown

Follow symlinked source directories

A source directory reached through a symlink contributed no files at all, and
the build still reported success. The missing classes only surfaced at runtime.

How to reproduce

This is the ordinary Composer layout for developing a library alongside an app.
The app requires the library through a path repository:

{
    "repositories": [{ "type": "path", "url": "../mylib" }],
    "require": { "vendor/mylib": "@dev" }
}

Composer installs a path repository by symlinking it, which is what makes
edits to the library show up in the app immediately:

vendor/vendor/mylib -> ../../../mylib

A compiled binary has no autoloader, so every class it uses has to be compiled
in, and the project file says so:

sources:
  - app
  - vendor

Compiling then succeeds:

Successfully compiled 8 files

but nothing under vendor/vendor/mylib was compiled, and the binary dies on the
first request that reaches the library:

PHP Fatal error: Uncaught Error: class 'MyLib\Greeting' is undefined

Cause

FileScanner::scan() builds a RecursiveDirectoryIterator with SKIP_DOTS
only. RecursiveIteratorIterator descends by calling hasChildren(), whose
$allowLinks argument defaults to false, so symlinked directories are never
entered. Passing a symlink as the source root works, because that path is
resolved directly; only symlinks found during a scan are skipped.

default:                                  1 file
with FilesystemIterator::FOLLOW_SYMLINKS: 2 files

FOLLOW_SYMLINKS does not appear anywhere in the tree, and nothing in the docs
or tests describes source scanning as excluding symlinks, so this looks like an
inherited default rather than a decision. For contrast, PrecompiledHeaderManager
checks isLink() deliberately, twice, where it matters for cache cleanup.

Change

Scan with FilesystemIterator::FOLLOW_SYMLINKS, and visit each real directory
at most once, so a link pointing at one of its own ancestors cannot recurse
forever.

Opting out, for a tree that deliberately links to something that must not be
compiled:

follow-symlinks: false
tpc.php project.yml --no-follow-symlinks

Both are read before sources are scanned, since that is what consumes them.
The command line wins over the project file, matching php-version.

Tests

phpunit/src/Build/FileScannerTest.php covers the three behaviours: a symlinked
directory is scanned by default, is skipped when disabled, and a symlink cycle
is visited once rather than recursing.

Full suite before and after this branch fails the same 7 pre-existing tests
(ExitCodeTest, PythonModuleTest), 2289 → 2292 tests with the three added.
completions/tpc.bash is regenerated for the new flag.

Additional notes

Chinese documentation and code comments were created with the help of AI, I'm not able to verify if their quality is good enough. Those texts might be rough or need polishing, let me know if that's the case and I'll happily fix it.

Composer path repositories install a package as a symlink, so a source
directory reached through one contributed no files at all: PHP's
RecursiveDirectoryIterator does not descend into symlinked directories
unless asked, because RecursiveIteratorIterator calls hasChildren() and
its $allowLinks argument defaults to false.

The build still reported success, and the missing classes only surfaced
at runtime as `class 'X' is undefined`.

Scan with FilesystemIterator::FOLLOW_SYMLINKS, visiting each real
directory at most once so a link pointing at an ancestor cannot recurse
forever. Add `follow-symlinks: false` and --no-follow-symlinks for trees
that deliberately link to something that must not be compiled.

Both are read before sources are scanned, since that is what consumes
them, with the command line taking precedence over the project file.

@matyhtf matyhtf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for tracking down the missing Composer path-repository sources. Following linked source directories is the behavior we want, but I am requesting changes before merging this PR.

  1. Please treat symlinks to files and directories as ordinary source entries, without adding a follow-symlinks configuration setting or --no-follow-symlinks CLI option. Source selection should remain controlled by the existing sources and ignore settings. Please remove the option, its completion entry, and the related documentation and tests.

  2. Please fix ignore for paths reached through a symlink. The scanner returns a path such as project/vendor/pkg/src/Foo.php, while parseProjectYaml() resolves ignore: vendor/pkg to the target's real path. shouldIgnoreFile() compares those strings, so the ignored package is still compiled. Match exclusions against the path the user specified and the scanner traversed; keep the real path separately for identity and cycle detection.

  3. Please avoid a global visited-directory set before filtering. If two links point to the same directory and the first path is excluded by FileScanner::addExcludePattern(), the current callback marks the real directory visited and the second, allowed path is never scanned. Detect cycles against the real directories on the current traversal branch, then deduplicate accepted source files by real path after exclusions. Keep the resulting file order deterministic.

Please add regression tests for a linked directory excluded by YAML ignore, two aliases to one directory with only one excluded, a link cycle, and a symlinked source file. I reproduced both exclusion failures with the scanner from this PR's head commit.

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.

2 participants