Add React Native Source Patching Infrastructure #3871
Conversation
As part of the efforts to move off of microsoft/react-native, we're aiming to consume facebook/react-native directly and then apply a set of Windows specific patches. This change gives us a mechanism to do this patching. We opt to fully modify React Native sources instead of manipulating files/includes with build logic. This helps with header resolution and keeps the rest of the build mostly unaware of the patching. Patching is done in the projects intermediate output directory. This has a number of benefits but has the unfortunate property that we will do patching separately for each vcxproj, and have copies of patched react native for each. Some of this overhead is mitigated by only copying C++ source files. Avoiding this would likely mean outputting to a non-build directory or making dramatic changes to build structure. We move jsireact header logic here as well, so we can remove the root WorkingHeaders directory. Validated that files in DeforkingPatches will overwrite source and that we still compile files including jsireact after removing an existing WorkingHeaders directory. Tested that changing patches leads to a correct imcremental rebuild.
NickGerleman
requested a review
from microsoft/react-native-windows-write
as a
code owner
| <!-- | ||
| Visual Studio has its own incremental build logic for simple cases that | ||
| falls over here. Force use of MSBuild logic here. This has a minimal impact | ||
| on build time (~10s total to build a noop change on a six core machine). |
NickGerleman
Jan 14, 2020
Author
Contributor
I kind of hate this, but it feels good to enable this anyway since we could have silently been getting flaky incremental builds before. Given the time to compile actual changes (especially without a PCH in most projects), this isn't too much overhead.
I kind of hate this, but it feels good to enable this anyway since we could have silently been getting flaky incremental builds before. Given the time to compile actual changes (especially without a PCH in most projects), this isn't too much overhead.
| <Warning Text="Folly required to build without microsoft/react-native. - Downloading folly..." Condition="!Exists('$(FollyDir)..\.follyzip\folly-2019.09.30.00.zip')" /> | ||
| <DownloadFile Condition="!Exists('$(FollyDir)..\.follyzip\folly-2019.09.30.00.zip')" SourceUrl="https://github.com/facebook/folly/archive/v2019.09.30.00.zip" DestinationFolder="$(FollyDir)..\.follyzip" /> | ||
| </Target> | ||
| <Target Name="UnzipFollyIfNotUseMicrosoftReactNative" BeforeTargets="PrepareForBuild" DependsOnTargets="DownloadFollyIfNotUseMicrosoftReactNative"> |
NickGerleman
Jan 14, 2020
•
Author
Contributor
Updating this since from what I can tell we never populate folly from npm, even when using microsoft/react-native.
Updating this since from what I can tell we never populate folly from npm, even when using microsoft/react-native.
…nput since we enumerate before other patching.
|
@acoates-ms I'm interested in some of the comments you made earlier about potential issues around developers consuming RNW and how we publish the non-Nuget version of the project. Is there an area I should look, or any immediate concerns that come to mind? |
| <ReactSourceDestinationFiles Include="@(ReactSourceFiles->'$(ReactNativeDir)\%(RecursiveDir)%(Filename)%(Extension)')" /> | ||
|
|
||
| <DeforkingPatchFiles Include="$(ReactNativeWindowsDir)\DeforkingPatches\**" /> | ||
| <DeforkingPatchDesintationFiles Include="@(DeforkingPatchFiles->'$(ReactNativeDir)\%(RecursiveDir)%(Filename)%(Extension)')" /> |
NickGerleman
Jan 14, 2020
Author
Contributor
nit: spelling
nit: spelling
|
Hello @NickGerleman! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
@msftbot require 2 sign offs. |
|
Hello @NickGerleman! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
|
|
msftbot
bot
merged commit 026b176
into
microsoft:master
As part of the efforts to move off of microsoft/react-native, we're aiming to
consume facebook/react-native directly and then apply a set of Windows specific
patches. This change gives us a mechanism to do this patching.
We opt to fully modify React Native sources instead of manipulating
files/includes with build logic. This helps with header resolution and keeps
the rest of the build mostly unaware of the patching. Patching is done in the
projects intermediate output directory. This has a number of benefits but has
the unfortunate property that we will do patching separately for each vcxproj,
and have copies of patched react native for each. Some of this overhead is
mitigated by only copying C++ source files. Avoiding this would likely mean
outputting to a non-build directory or making larger changes to build
structure.
We move jsireact header logic here as well, so we can remove the root
WorkingHeaders directory.
Validated that files in DeforkingPatches will overwrite source and that we
still compile files including jsireact after removing an existing
WorkingHeaders directory. Tested that changing patches leads to a correct
imcremental rebuild.
Microsoft Reviewers: Open in CodeFlow