Remove broken CircleCI loops #940
Closed
Conversation
This file contains 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
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Apr 25, 2023
Summary: X-link: facebook/litho#940 X-link: facebook/yoga#1252 Pull Request resolved: facebook#36993 Fabric relies on the private C++ internals of Yoga. This creates a conundrum in the open source build due to how header creation in Cocoapods works. 1. The default mechanism of specifying public headers needs to include the private headers for them to be made usable by fabric (by default) 2. Cocoapods will roll up all of the public headers when importing a module facebook#33381 fixed the Fabric Cocoapods build which ran into this. React Native relies on FlipperKit which relies on YogaKit, which in turn finally imports the Yoga podspec. Because YogaKit may use Swift, we can only expose the public Yoga C ABI. The first solution in that PR was to allow RN to access Yoga private headers, but this was changed to instead make all Yoga headers public, and to add ifdefs to all of them to no-op when included outside of a C++ environment. Talking to Kudo, we should be able to change back to the earlier approach in the PR, to instead expose the private headers to only RN. This lets us avoid exposing headers that we ideally wouldn't be, and lets us avoid the messy ifdefs in every Yoga header. Changelog: [Internal] Differential Revision: D45139075 fbshipit-source-id: b80bcb5b2a20ec5b2936f6c4d84361f084e8f4c3
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Apr 26, 2023
Summary: X-link: facebook/litho#940 X-link: facebook/yoga#1252 Pull Request resolved: facebook#36993 Fabric relies on the private C++ internals of Yoga. This creates a conundrum in the open source build due to how header creation in Cocoapods works. 1. The default mechanism of specifying public headers needs to include the private headers for them to be made usable by fabric (by default) 2. Cocoapods will roll up all of the public headers when importing a module facebook#33381 fixed the Fabric Cocoapods build which ran into this. React Native relies on FlipperKit which relies on YogaKit, which in turn finally imports the Yoga podspec. Because YogaKit may use Swift, we can only expose the public Yoga C ABI. The first solution in that PR was to allow RN to access Yoga private headers, but this was changed to instead make all Yoga headers public, and to add ifdefs to all of them to no-op when included outside of a C++ environment. Talking to Kudo, we should be able to change back to the earlier approach in the PR, to instead expose the private headers to only RN. This lets us avoid exposing headers that we ideally wouldn't be, and lets us avoid the messy ifdefs in every Yoga header. Changelog: [Internal] Reviewed By: rshest Differential Revision: D45139075 fbshipit-source-id: ad6c0b5b5a7aa5fbf849c3ca62f3fe330601b19a
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Apr 26, 2023
Summary: X-link: facebook/litho#940 X-link: facebook/yoga#1252 Pull Request resolved: facebook#36993 Fabric relies on the private C++ internals of Yoga. This creates a conundrum in the open source build due to how header creation in Cocoapods works. 1. The default mechanism of specifying public headers needs to include the private headers for them to be made usable by fabric (by default) 2. Cocoapods will roll up all of the public headers when importing a module facebook#33381 fixed the Fabric Cocoapods build which ran into this. React Native relies on FlipperKit which relies on YogaKit, which in turn finally imports the Yoga podspec. Because YogaKit may use Swift, we can only expose the public Yoga C ABI. The first solution in that PR was to allow RN to access Yoga private headers, but this was changed to instead make all Yoga headers public, and to add ifdefs to all of them to no-op when included outside of a C++ environment. Talking to Kudo, we should be able to change back to the earlier approach in the PR, to instead expose the private headers to only RN. This lets us avoid exposing headers that we ideally wouldn't be, and lets us avoid the messy ifdefs in every Yoga header. Changelog: [Internal] Reviewed By: rshest Differential Revision: D45139075 fbshipit-source-id: 96dab7b73f5164cb5103674d074d0759dad9c8b0
NickGerleman
added a commit
to NickGerleman/react-native
that referenced
this pull request
Apr 27, 2023
Summary: X-link: facebook/litho#940 X-link: facebook/yoga#1252 Pull Request resolved: facebook#36993 Fabric relies on the private C++ internals of Yoga. This creates a conundrum in the open source build due to how header creation in Cocoapods works. 1. The default mechanism of specifying public headers needs to include the private headers for them to be made usable by fabric (by default) 2. Cocoapods will roll up all of the public headers when importing a module facebook#33381 fixed the Fabric Cocoapods build which ran into this. React Native relies on FlipperKit which relies on YogaKit, which in turn finally imports the Yoga podspec. Because YogaKit may use Swift, we can only expose the public Yoga C ABI. The first solution in that PR was to allow RN to access Yoga private headers, but this was changed to instead make all Yoga headers public, and to add ifdefs to all of them to no-op when included outside of a C++ environment. Talking to Kudo, we should be able to change back to the earlier approach in the PR, to instead expose the private headers to only RN. This lets us avoid exposing headers that we ideally wouldn't be, and lets us avoid the messy ifdefs in every Yoga header. Changelog: [Internal] Reviewed By: rshest Differential Revision: D45139075 fbshipit-source-id: 99152986a578f7aac8324dffe0e18c42a38cc6a5
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.
Please select one of the following
Summary
CircleCI is always broken in our fork. Looking upstream to facebook's repo, it also appears to be often broken in their main branch. We run our own CI validation in ADO so we don't need the broken CircleCI at all.
Due to the CircleCI being broken all the time, people are unable to merge in their branches without administrator intervention. This should fix that problem by removing the CircleCI altogether.
Test Plan
I'll verify that the CI that runs from this PR doesn't run CircleCI anymore. If that's the case, then this change should be clear to merge.