Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure that third party libraries include Fabric code #41669

Closed
wants to merge 1 commit into from

Conversation

cipolleschi
Copy link
Contributor

Summary:
In some previous changes (a607692 and 6b53205) we make sure to always include all the pods (including Fabric) and we unify codegen to run in the same way on both architectures.
While doing so, we enabled codegen to run on libraries tat already migrated to Fabric.
These makes those libraries to fail when building as they were not including the Fabric code when the New Architecture is disabled.

This change will make sue that the code is always included, thus the library should always build, and it also make sure that we can control the New/Old Architecture at build time.

Changelog

[iOS][Changed] - Make sure that libraries always include Fabric code also in the old architecture

Differential Revision: D51617542

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Nov 28, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D51617542

@analysis-bot
Copy link

analysis-bot commented Nov 28, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,701,319 -7
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,092,546 +6
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 667d85b
Branch: main

cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request Nov 29, 2023
)

Summary:

In some previous changes ([a607692](facebook@a607692) and [6b53205](facebook@6b53205)) we make sure to always include all the pods (including Fabric) and we unify codegen to run in the same way on both architectures.
While doing so, we enabled codegen to run on libraries tat already migrated to Fabric.
These makes those libraries to fail when building as they were not including the Fabric code when the New Architecture is disabled.

This change will make sue that the code is always included, thus the library should always build, and it also make sure that we can control the New/Old Architecture at build time.

## Changelog
[iOS][Changed] - Make sure that libraries always include Fabric code also in the old architecture

Reviewed By: dmytrorykun

Differential Revision: D51617542
)

Summary:

In some previous changes ([a607692](facebook@a607692) and [6b53205](facebook@6b53205)) we make sure to always include all the pods (including Fabric) and we unify codegen to run in the same way on both architectures.
While doing so, we enabled codegen to run on libraries tat already migrated to Fabric.
These makes those libraries to fail when building as they were not including the Fabric code when the New Architecture is disabled.

This change will make sue that the code is always included, thus the library should always build, and it also make sure that we can control the New/Old Architecture at build time.

## Changelog
[iOS][Changed] - Make sure that libraries always include Fabric code also in the old architecture

Reviewed By: dmytrorykun

Differential Revision: D51617542
cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request Nov 29, 2023
)

Summary:

In some previous changes ([a607692](facebook@a607692) and [6b53205](facebook@6b53205)) we make sure to always include all the pods (including Fabric) and we unify codegen to run in the same way on both architectures.
While doing so, we enabled codegen to run on libraries tat already migrated to Fabric.
These makes those libraries to fail when building as they were not including the Fabric code when the New Architecture is disabled.

This change will make sue that the code is always included, thus the library should always build, and it also make sure that we can control the New/Old Architecture at build time.

## Changelog
[iOS][Changed] - Make sure that libraries always include Fabric code also in the old architecture

Reviewed By: dmytrorykun

Differential Revision: D51617542
cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request Nov 29, 2023
)

Summary:

In some previous changes ([a607692](facebook@a607692) and [6b53205](facebook@6b53205)) we make sure to always include all the pods (including Fabric) and we unify codegen to run in the same way on both architectures.
While doing so, we enabled codegen to run on libraries tat already migrated to Fabric.
These makes those libraries to fail when building as they were not including the Fabric code when the New Architecture is disabled.

This change will make sue that the code is always included, thus the library should always build, and it also make sure that we can control the New/Old Architecture at build time.

## Changelog
[iOS][Changed] - Make sure that libraries always include Fabric code also in the old architecture

Reviewed By: dmytrorykun

Differential Revision: D51617542
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D51617542

cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request Nov 29, 2023
)

Summary:

In some previous changes ([a607692](facebook@a607692) and [6b53205](facebook@6b53205)) we make sure to always include all the pods (including Fabric) and we unify codegen to run in the same way on both architectures.
While doing so, we enabled codegen to run on libraries tat already migrated to Fabric.
These makes those libraries to fail when building as they were not including the Fabric code when the New Architecture is disabled.

This change will make sue that the code is always included, thus the library should always build, and it also make sure that we can control the New/Old Architecture at build time.

## Changelog
[iOS][Changed] - Make sure that libraries always include Fabric code also in the old architecture

Reviewed By: dmytrorykun

Differential Revision: D51617542
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D51617542

Copy link

This pull request was successfully merged by @cipolleschi in 4a35f25.

When will my fix make it into a release? | Upcoming Releases

@github-actions github-actions bot added the Merged This PR has been merged. label Nov 29, 2023
Othinn pushed a commit to Othinn/react-native that referenced this pull request Jan 9, 2024
)

Summary:
Pull Request resolved: facebook#41669

In some previous changes ([a607692](facebook@a607692) and [6b53205](facebook@6b53205)) we make sure to always include all the pods (including Fabric) and we unify codegen to run in the same way on both architectures.
While doing so, we enabled codegen to run on libraries tat already migrated to Fabric.
These makes those libraries to fail when building as they were not including the Fabric code when the New Architecture is disabled.

This change will make sue that the code is always included, thus the library should always build, and it also make sure that we can control the New/Old Architecture at build time.

## Changelog
[iOS][Changed] - Make sure that libraries always include Fabric code also in the old architecture

Reviewed By: dmytrorykun

Differential Revision: D51617542

fbshipit-source-id: 883d1e258c341feb0405ad389bb8af34d64b59b8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants