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

Disable gflags include #28451

Closed
wants to merge 3 commits into from
Closed

Conversation

KDederichs
Copy link
Contributor

Summary

Fixes the issue explained in #28446
It basically disabled the gflags include before configure can detect the header on the users system.

Changelog

[iOS] [Fixed] - Fixed inability to build apps when gflags is installed

Test Plan

Tested by installing gflags brew install gflags and verifying that apps build after.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 30, 2020
@react-native-bot react-native-bot added Bug Platform: iOS iOS applications. labels Mar 30, 2020
@analysis-bot
Copy link

analysis-bot commented Mar 30, 2020

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,945,831 +0
android hermes armeabi-v7a 8,479,654 +0
android hermes x86 9,364,791 +0
android hermes x86_64 9,332,277 +0
android jsc arm64-v8a 10,628,106 +0
android jsc armeabi-v7a 9,549,444 +0
android jsc x86 10,641,345 +0
android jsc x86_64 11,252,516 +0

Base commit: 8e66f0b

@analysis-bot
Copy link

analysis-bot commented Mar 30, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 8e66f0b

@facebook-github-bot
Copy link
Contributor

@sota000 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@sota000 sota000 self-assigned this Aug 16, 2021
Copy link
Contributor

@sota000 sota000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @KDederichs, apologies for taking the time to review. Could you please address the comment I made inline and solve the merge conflict? Thank you for the contribution!

scripts/ios-configure-glog.sh Outdated Show resolved Hide resolved
@KDederichs
Copy link
Contributor Author

No worries, projects of this size get a lot of contribution so I understand that small stuff like this lies around for a while.
I added the issue link to the comment, I didn't see any conflicts though when merging upstream into the fix branch, so I assume that's good?

Copy link
Contributor

@sota000 sota000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating!

@facebook-github-bot
Copy link
Contributor

@sota000 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@sota000 merged this pull request in ab8dbdf.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants