Fix NDEBUG macro not present in Release builds#5366
Merged
Conversation
tjzel
commented
Nov 10, 2023
tjzel
force-pushed
the
@tjzel/fix-release-NDEBUG
branch
from
9296b92 to
33c5104
Compare
tomekzaw
requested changes
Nov 16, 2023
TVOSExample/ios/TVOSExample.xcodeproj/xcshareddata/xcschemes/TVOSExample-tvOS.xcscheme
Outdated
Show resolved
Hide resolved
tjzel
commented
Nov 20, 2023
tjzel
changed the title
Latropos
pushed a commit
that referenced
this pull request
Nov 24, 2023
## Summary This PR aims to fix the issue when Release builds would fail on Paper on iOS. The reason this regression happened is #5113. XCode ships by default to new projects `DEBUG` macro for Debug Configuration. Due to some issues (#4902) we decided to switch over to `NDEBUG` macro. However, the `NDEBUG` is not provided by XCode out of the box. Incidentally, it's provided by React Native which puts its install script into project's Podfile - however, [only in Fabric](facebook/react-native@421df9f). What is more, some time ago React Native made it so `PRODUCTION=1 pod install` is [no longer a necessary step](facebook/react-native@93fdcba) to make a release build, therefore we cannot rely on it. Until React Native defines `NDEBUG` for Paper too we have to detect, based on available options, whether we potentially have a Release build and this pull request does this. Thanks to #5383 we finally figures out how to do it the most agreeable way. We hardcode that for a build config named `Release` NDEBUG=1 will be set. If the user uses some custom release build config e.g. "MyRelease", NDEBUG=1 will not be set - in this situation we'll require the user to use `PRODUCTION=1`. ## Test plan Run all example apps (except TVOS cause I couldn't make it cooperate with me, and Web) on both iOS and Android in both Release and Debug and see if they work properly (e.g. add a throw in #ifndef fragment).
This was referenced Dec 7, 2023
Latropos
pushed a commit
that referenced
this pull request
Dec 12, 2023
## Summary This PR aims to fix the issue when Release builds would fail on Paper on iOS. The reason this regression happened is #5113. XCode ships by default to new projects `DEBUG` macro for Debug Configuration. Due to some issues (#4902) we decided to switch over to `NDEBUG` macro. However, the `NDEBUG` is not provided by XCode out of the box. Incidentally, it's provided by React Native which puts its install script into project's Podfile - however, [only in Fabric](facebook/react-native@421df9f). What is more, some time ago React Native made it so `PRODUCTION=1 pod install` is [no longer a necessary step](facebook/react-native@93fdcba) to make a release build, therefore we cannot rely on it. Until React Native defines `NDEBUG` for Paper too we have to detect, based on available options, whether we potentially have a Release build and this pull request does this. Thanks to #5383 we finally figures out how to do it the most agreeable way. We hardcode that for a build config named `Release` NDEBUG=1 will be set. If the user uses some custom release build config e.g. "MyRelease", NDEBUG=1 will not be set - in this situation we'll require the user to use `PRODUCTION=1`. ## Test plan Run all example apps (except TVOS cause I couldn't make it cooperate with me, and Web) on both iOS and Android in both Release and Debug and see if they work properly (e.g. add a throw in #ifndef fragment).
Latropos
pushed a commit
that referenced
this pull request
Dec 18, 2023
This PR aims to fix the issue when Release builds would fail on Paper on iOS. The reason this regression happened is #5113. XCode ships by default to new projects `DEBUG` macro for Debug Configuration. Due to some issues (#4902) we decided to switch over to `NDEBUG` macro. However, the `NDEBUG` is not provided by XCode out of the box. Incidentally, it's provided by React Native which puts its install script into project's Podfile - however, [only in Fabric](facebook/react-native@421df9f). What is more, some time ago React Native made it so `PRODUCTION=1 pod install` is [no longer a necessary step](facebook/react-native@93fdcba) to make a release build, therefore we cannot rely on it. Until React Native defines `NDEBUG` for Paper too we have to detect, based on available options, whether we potentially have a Release build and this pull request does this. Thanks to #5383 we finally figures out how to do it the most agreeable way. We hardcode that for a build config named `Release` NDEBUG=1 will be set. If the user uses some custom release build config e.g. "MyRelease", NDEBUG=1 will not be set - in this situation we'll require the user to use `PRODUCTION=1`. Run all example apps (except TVOS cause I couldn't make it cooperate with me, and Web) on both iOS and Android in both Release and Debug and see if they work properly (e.g. add a throw in #ifndef fragment).
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
This PR aims to fix the issue when Release builds would fail on Paper on iOS.
The reason this regression happened is #5113.
XCode ships by default to new projects
DEBUGmacro for Debug Configuration. Due to some issues (#4902) we decided to switch over toNDEBUGmacro. However, theNDEBUGis not provided by XCode out of the box. Incidentally, it's provided by React Native which puts its install script into project's Podfile - however, only in Fabric.What is more, some time ago React Native made it so
PRODUCTION=1 pod installis no longer a necessary step to make a release build, therefore we cannot rely on it. Until React Native definesNDEBUGfor Paper too we have to detect, based on available options, whether we potentially have a Release build and this pull request does this.Thanks to #5383 we finally figures out how to do it the most agreeable way. We hardcode that for a build config named
ReleaseNDEBUG=1 will be set. If the user uses some custom release build config e.g. "MyRelease", NDEBUG=1 will not be set - in this situation we'll require the user to usePRODUCTION=1.Test plan
Run all example apps (except TVOS cause I couldn't make it cooperate with me, and Web) on both iOS and Android in both Release and Debug and see if they work properly (e.g. add a throw in #ifndef fragment).