Conversation
league/flysystem v1 has been abandoned since 2022. Migrate to v3 API: - Replace Adapter\Local with Local\LocalFilesystemAdapter - Replace League\Flysystem\Exception with FilesystemException - Remove readStream false-check (v3 throws instead of returning false) - Remove outdated @throws FileNotFoundException docblocks - Update integration test expectations for v3 path traversal message - Upstream phpdocumentor/guides 1.9.5 already supports flysystem v3
Summary of ChangesHello @CybotTM, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a critical dependency upgrade, moving the project from the abandoned Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully upgrades league/flysystem from v1 to v3, along with other dependency updates in composer.json and composer.lock. The code changes reflect the API migration, specifically in CopyResources.php for LocalFilesystemAdapter and TwigExtension.php for FilesystemException. Docblock updates were also made in IncludeDirective.php and SiteSetSettingsDirective.php to remove references to FileNotFoundException which is no longer thrown by Flysystem v3. The integration test logs were updated to reflect the new error message format. The changes are well-aligned with the goal of upgrading the library and migrating to its new API.
There was a problem hiding this comment.
Pull request overview
This PR upgrades the league/flysystem dependency from the abandoned v1 (end-of-life since 2022) to the current v3 version. The migration involves updating the code to use Flysystem v3's API changes, including the new LocalFilesystemAdapter class and FilesystemException for error handling. The phpdocumentor/guides upstream dependency (v1.9.5) already supports Flysystem v3, enabling this upgrade.
Changes:
- Updated league/flysystem constraint from ^1.1.10 to ^3.29 with corresponding composer.lock updates
- Migrated from deprecated
League\Flysystem\Adapter\LocaltoLeague\Flysystem\Local\LocalFilesystemAdapter - Updated exception handling from
League\Flysystem\ExceptiontoLeague\Flysystem\FilesystemException - Removed obsolete
@throws \League\Flysystem\FileNotFoundExceptiondocblock annotations - Updated integration test expectations to match Flysystem v3 error message format
- Added PHP 8.5 to CI test matrix
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| composer.json | Updated flysystem constraint to ^3.29 |
| composer.lock | Flysystem v3.31.0 and transitive dependency updates (doctrine/deprecations, league/tactician, league/uri, masterminds/html5, phpdocumentor/flyfinder 2.0.0, phpdocumentor/guides 1.9.5, symfony packages, twig 3.23.0, webmozart/assert) |
| packages/typo3-docs-theme/src/EventListeners/CopyResources.php | Migrated to LocalFilesystemAdapter, removed false-check error handling (v3 throws exceptions) |
| packages/typo3-docs-theme/src/Twig/TwigExtension.php | Updated exception import to FilesystemException |
| packages/typo3-docs-theme/src/Directives/IncludeDirective.php | Removed obsolete FileNotFoundException docblock annotations |
| packages/typo3-docs-theme/src/Directives/SiteSetSettingsDirective.php | Removed obsolete FileNotFoundException docblock annotation |
| tests/Integration/tests/site-set-failure/expected/logs/*.log | Updated expected error message from "Path is outside of the defined root" to "Path traversal detected" with new log format |
| .github/workflows/main.yaml | Added PHP 8.5 to CI matrix |
Comments suppressed due to low confidence (1)
packages/typo3-docs-theme/src/EventListeners/CopyResources.php:56
- In Flysystem v3, the
readStream()andputStream()methods throwFilesystemExceptioninstead of returning false on errors. The removed error handling for false returns should be replaced with try-catch block to handle potential exceptions. Without this, exceptions from file operations will propagate uncaught, potentially causing the entire resource copying process to fail on a single problematic file.
Consider wrapping the file operations in a try-catch block similar to the pattern used in TwigExtension.php (line 620), logging errors and continuing with the next file rather than halting the entire process.
foreach ($finder as $file) {
$stream = $source->readStream($file->getRelativePathname());
$destinationPath = sprintf(
'%s/%s%s',
self::DESTINATION_PATH,
$file->getRelativePath() !== '' ? $file->getRelativePath() . '/' : '',
$file->getFilename()
);
$destination->putStream($destinationPath, $stream);
is_resource($stream) && fclose($stream);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
Summary
Files changed
composer.json/composer.lock- constraint update + transitive upgradespackages/typo3-docs-theme/src/EventListeners/CopyResources.php- v3 API migrationpackages/typo3-docs-theme/src/Twig/TwigExtension.php- exception class updatepackages/typo3-docs-theme/src/Directives/IncludeDirective.php- docblock updatepackages/typo3-docs-theme/src/Directives/SiteSetSettingsDirective.php- docblock updateTest plan