feat(android): port iOS rsync optimisations to Android build pipeline#31
feat(android): port iOS rsync optimisations to Android build pipeline#31benjam-es wants to merge 7 commits intoNativePHP:mainfrom
Conversation
|
This ports the same optimisations introduced in the iOS build in #30 — shared The Android pipeline is a bit more involved since it needs to work across macOS, Linux, and Windows (robocopy vs rsync, different nativephp/ exclude logic per platform), so this might need more testing on those platforms to make sure nothing slips through. The core exclude list is identical to iOS, but the platform-conditional branches are worth a closer look. |
5ce4ddc to
ad955fd
Compare
Manual profiling: kitchen-sink-mobile-vueTested on real device build ( Build step timing
What the 15.8 MB reduction is made ofBreakdown of the 1,794 files (34 MB pre-compression) now excluded from the bundle:
Profiling script results (3-run average)
|
Safety of removed filesAll excluded content is non-runtime:
The cleanup step ( |
ad955fd to
105385a
Compare
shanerbaner82
left a comment
There was a problem hiding this comment.
Nice work — the refactor into BundleFileManager / BundleExclusions is clean and the benchmarks are compelling (10x memory reduction, 25% faster builds).
After merging main (which now includes #33's test fixes), 10 tests still fail — all the same root cause: tests calling $this->platformOptimizedCopy() which was removed from PlatformFileOperations. These tests weren't caught earlier because they only break when combined with this PR's removal of the method.
Failing tests:
CrossPlatformFileOperationsTest(5) — calls$this->platformOptimizedCopy()EdgeCasesAndErrorHandlingTest(1) — circular symlinks test calls itInstallsAndroidTest(2) —createAndroidStudioProjectcalls it via the traitPlatformFileOperationsTest(2) — directly tests the removed method
These need to either be updated to use BundleFileManager::copy() or removed since the functionality moved. Everything else passes — the new tests (BundleFileManagerTest, AndroidBundleCopyTest, IosBuildCopyTest) all look good.
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
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.
- Add shared getExcludedPaths() with comprehensive exclude list matching iOS - Add loadVendorExportIgnorePatterns() for .gitattributes export-ignore support - Fix vendor glob pattern from vendor/*/vendor to vendor/*/*/vendor - Replace platform-specific match block with unified getExcludedPaths() call - Stream ZIP iterator directly instead of materialising via iterator_to_array() - Remove redundant addDirectoryToZip() excludes now handled by rsync
…ileManager Remove getExcludedPaths(), loadVendorExportIgnorePatterns(), and platformOptimizedCopy() that duplicated the extracted BundleExclusions and BundleFileManager classes. Android bundle prep now uses the same copy/cleanup pipeline as iOS.
Add copyRaw() to BundleFileManager for plain directory copies without bundle exclusions. Update InstallsAndroid to use it for template and binary copies where applying exclusions would strip essential files like AndroidManifest.xml and resource XMLs. Update v3.1 tests to use Process::fake() and assert rsync commands rather than relying on the removed platformOptimizedCopy() method. Signed-off-by: Ben James <in@benjam.es>
Replace File::copyDirectory() with BundleFileManager::copyRaw() for Xcode template, PHP library, and bridge file copies during iOS installation. Consistent with the Android install path. Signed-off-by: Ben James <in@benjam.es>
Move excludes resolution into copy() and pass the pre-built array to copyWithRsync/copyWithRobocopy. These private methods now default to an empty excludes array, allowing copyRaw to delegate directly without duplicating rsync/robocopy logic. Signed-off-by: Ben James <in@benjam.es>
105385a to
07c1463
Compare
|
Android consistency + copyRaw() After rebasing onto v3.1, I ported the same rsync approach to the Android build pipeline for consistency — both platforms now use BundleFileManager for all file operations. During that work, I noticed that template and binary copies (Xcode project, Android Studio project, PHP libraries) were using a mix of File::copyDirectory() and the old platformOptimizedCopy() trait method. These copies should never have exclusion patterns applied — running them through BundleFileManager::copy() would strip files like AndroidManifest.xml because of the /*.xml exclusion pattern. So I added BundleFileManager::copyRaw() — a plain copy with no exclusions, intended specifically for templates and binary artifacts. Under the hood it shares the same copyWithRsync/copyWithRobocopy private methods, just without passing any excludes. All template/binary copies across both platforms now use copyRaw(), while Laravel bundle copies use copy() with exclusions. If merged this closes #30 at the same time |
Problem
The Android build pipeline in
PreparesBuild.phpandPlatformFileOperations.phphas the same issues that were fixed for iOS in #30:Minimal exclude rules — Only 4–6 hardcoded excludes (
.git,node_modules,nativephp/*,vendor/nativephp/mobile/resources), so rsync copies thousands of non-runtime files (tests, docs, dotfiles, YAML configs, LICENSE files) into the temp directory.Wrong vendor glob —
vendor/*/vendoronly matches one level deep, missing the actual Composer layoutvendor/namespace/package/vendor. The separatevendor/nativephp/mobile/vendorline was a workaround for this bug.No
.gitattributessupport — Vendor packages that declareexport-ignorepatterns are fully copied, including their test suites and CI configs.Memory spike in ZIP step —
addDirectoryToZip()callsiterator_to_array()to materialise every file into a PHP array before iterating, consuming ~20 MB unnecessarily.Redundant hardcoded excludes —
addDirectoryToZip()maintains its own list of 14 hardcoded excludes that partially overlap with rsync's excludes, making the filtering logic harder to maintain.Solution
Port the iOS optimisations from #30 to the Android build:
getExcludedPaths()— shared method with comprehensive exclude list (dotfiles, tests, docs, vendor non-runtime files, project-level build artifacts), with platform-awarenativephp/handlingloadVendorExportIgnorePatterns()— reads.gitattributesexport-ignorefrom vendor packagesprepareLaravelBundle()— replaces thematch (PHP_OS_FAMILY)block withgetExcludedPaths()+loadVendorExportIgnorePatterns()platformOptimizedCopy()— fixesvendor/*/vendor→vendor/*/*/vendor, removes hardcoded appends (now provided by caller)addDirectoryToZip()— streams iterator directly instead of materialising array; removes excludes now guaranteed absent after rsync (keeps safety-net excludes for files created between rsync and zip)Benchmarks
Profiled on kitchen-sink-mobile-vue (Android build flow):
Rsync copy
ZIP creation
Total
Key takeaways:
iterator_to_arrayis the big winaddDirectoryToZip()hardcoded excludes cleanup was safeTest plan
composer installstep still runs correctly in temp directorygetExcludedPaths()