feat(ios): replace PHP iterator with rsync for app copy step#30
Open
benjam-es wants to merge 2 commits intoNativePHP:mainfrom
Open
feat(ios): replace PHP iterator with rsync for app copy step#30benjam-es wants to merge 2 commits intoNativePHP:mainfrom
benjam-es wants to merge 2 commits intoNativePHP:mainfrom
Conversation
Replace RecursiveIteratorIterator with rsync -a --copy-links in copyLaravelAppIntoIosApp() to eliminate unnecessary file copying and prevent OOM crashes with symlinked local packages. The old approach collected all files into a PHP array before copying, consuming 60+ MB on standard projects and exceeding the 128 MB memory limit when Composer path repositories introduced nested vendor dirs. It also copied ~19,000 files (notably node_modules) that removeUnnecessaryFiles() immediately deleted. - Add getExcludedPaths() as single source of truth for rsync excludes - Add loadVendorExportIgnorePatterns() to respect .gitattributes - Add glob support in removeUnnecessaryFiles for wildcard dir patterns - Exclude non-runtime files (*.md, LICENSE*, docs, CI configs) - Fix vendor nested dir pattern (vendor/*/*/vendor vs vendor/*/vendor) - Wrap copy step in components->task() for progress feedback
5 tasks
Author
|
I spent so long working on profiling scripts to prove out the performance improvements I totally forgot to add any tests. Will add them now |
Move hardcoded exclusion lists, copy logic, and cleanup logic out of BuildIosAppCommand into BundleExclusions (data) and BundleFileManager (behaviour). This makes the lists testable, maintainable, and reusable across platforms.
Author
Benchmarks (kitchen-sink-vue)Profiled against
Key benefits
Tests41 tests across two suites:
|
Author
|
@simonhamp this restructure also addresses #2 I believe as we aim to centralise files and folder lists in a way they are reusable between copy exclusions as well as removals whilst bringing in user config) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
BuildIosAppCommand::copyLaravelAppIntoIosApp()uses aRecursiveIteratorIteratorwithFOLLOW_SYMLINKSto copy the Laravel app into the iOS build directory. This approach has three issues:Memory — Every file is collected into a PHP array before copying. On a standard Inertia project this consumes ~60 MB of memory. With local Composer path repositories (symlinked packages), the iterator follows symlinks into nested
vendor/directories, easily exceeding the default 128 MB memory limit and causing a silent exit code 255 with no error output.Wasted I/O — The iterator copies files that
removeUnnecessaryFiles()immediately deletes afterwards (notablynode_modules). On a real project this means ~19,000 files and ~240 MB of unnecessary disk writes every build.Speed — The two-pass approach (collect all files into memory, then iterate and copy) is slower than a native file copy tool.
Solution
Replace the PHP iterator with a single
rsync -a --copy-linkscall using exclude patterns. A newgetExcludedPaths()method provides a single source of truth for paths that should never enter the iOS bundle. Vendor package.gitattributesexport-ignorepatterns are also respected when present (relevant for locally symlinked packages during development).removeUnnecessaryFiles()is unchanged — it continues to clean up aftercomposer installwhich may recreate excluded directories.Benchmarks
Profiled on kitchen-sink-mobile-vue (Vue/Inertia, 14 NativePHP plugins, stock mobile ^3.0, no symlinks):
With locally symlinked packages (3 Composer path repos), the original iterator follows into each package's own
node_modules/andvendor/directories:What changed
copyLaravelAppIntoIosApp()— replacedRecursiveIteratorIterator+ manual copy loop withrsync -a --copy-linksand exclude flagsgetExcludedPaths()— new method, single source of truth for excluded paths (used by rsync)loadVendorExportIgnorePatterns()— new method, reads.gitattributesexport-ignorefrom vendor packagesremoveUnnecessaryFiles()— added glob support for wildcard patterns in$directoriesToRemove, addedvendor/*/*/vendorcleanup as safety net$this->components->task()Test plan
removeUnnecessaryFiles()still cleans up aftercomposer install