Don't create long paths during build to avoid failures on Windows#30248
Don't create long paths during build to avoid failures on Windows#30248alespergl wants to merge 1 commit intofacebook:mainfrom
Conversation
|
Hi @alespergl! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Base commit: 36b0f7d |
|
@cortinico @mganandraj wouldn't this address the Windows+Android issue? (pending resolving the conflicts, ofc) |
So those files have been migrated to CMake. This PR might be relevant just for 0.68.x, but it won't anyway provide any benefit for 0.69 and future versions. I'm unsure if this would work and maybe @mganandraj can tell more about it |
|
This might help as shortening each file paths will reduce the object file names and jence the command length.. I actually tried explicitly listing file names in one project and it made a difference .. but this approach of replacing the prefix after enumerating the files through wild card is neater and scales better.. |
|
@alespergl This change will won't have any effect on main branch which has switched to cmake .. but it would be very useful change in 0.68-stable branch.. We have issues especially with newarchitecuture enabled, as the fabric code hierarchy is even deeper.. I would appreciate if you could test building on Windows with newarchitecuture enabled in 0.68-stable branch, which might require fixing a lot more makefiles !! |
|
so just to be clear, it'd be probably better if Ales rebased this PR on top of the 0.68-stable branch and re-targetted this PR to it right? |
|
Yes, even better build with newArchitecuture turned on. I'm testing it out on the 0.68-stable branch now .. |
|
I extended this change to add all the new modules and created a PR against 0.68-stable branch : #33707 The change is verified on a ~10 character long host applications path .. And it allows us to revert the ndk bump. Actually ndk23 is frequently crashing ! which i'll continue investigating .. |
|
@mganandraj Thanks for taking this and porting it to the 0.68 branch. Normally I would suggest to just update this PR but since you already created a new one I'll wait for that to be completed and then this one can be withdrawn. |
|
Thanks for sending this over @alespergl. We anyway need to backport that PR to main as well, as we'll have to include this fix inside |
|
Closing as the fixes for long paths on Windows already landed and are available on 0.69+ which are the supported versions of React Native at the time of writing. |
Summary
When building Android C++ targets on Windows the generated output paths can be over the MAX_PATH limit of 260 characters causing build errors. This is due to the fact that when
$(wildcard)is used to generate a list of input files it produces absolute paths. Those are then processed to replace special characters, and appended to the build target's output root which creates an unnecessarily long path for each output file.This change converts the absolute input paths to relative paths by stripping the
LOCAL_PATHportion used to generate them.Changelog
[Android] [Fixed] - Improved build on Windows by avoiding failures due to long output paths
Test Plan
n/a