Use folly/GLog.h instead of directly including glog

[matthargett]

This allows for stubbing of GLog by checking for the presence of its headers in a centralized place.

Test Plan:

Compiled and ran cxxreact tests. Fabric still doesn't compile in OSS due to headers not being in the correct place.

Release Notes:
[INTERNAL][ENHANCEMENT][ReactCommon]

[shergin]

What do you mean by stubbing? Which exact problem are we solving with this?
Honestly, this explanation doesn't sounds sufficient. Seems that Folly's GLog adds some functionality which we probably don't need, so I don't think that adding this as a dependency (I am considering every single part of Folly as a separate dependency due to Folly's nature) is a reasonable solution.

[matthargett]

I have a local branch of folly where I stub out the glog macros if __has_include(<glog/logging.h>) fails. One of the platforms I'm testing cxxreact/fabric on doesn't have a glog port, and getting it to build and run isn't trivial. To test folly (and cxxreact) on that platform, I stub out glog macros in the folly header.

If glog didn't use macros, I could do a link-time substitution. I could also add defines for the macros on the commandline, but then it would need to be duplicated just in a different way.

[pull-bot]

	Warnings
⚠️	📋 Changelog - This PR appears to be missing Changelog. Please add a section called "changelog" and format it as explained in the contributing guidelines.

Generated by 🚫 dangerJS

[shergin]

Yeah, sure. But why do you think this change is a right solutions for that? It's just make it more convenient to you as a side effect which... does not seems so reasonable?

Why couldn't you "implement" glog for your platform with noop implementation?

[matthargett]

I could indeed stub out glog/logging.h to fix this problem, but there's a deeper binary size bloat and version mismatch issue I'm trying to solve as well. If folly is statically linking against glog and consumed as a shared library, then the version brought in by RN would need to match pretty precisely to avoid struct size and function signature mismatches, etc. Additionally, there's then two versions of glog occupying space in the program and that bloats memory size at least and app size at worst.

By having folly consistently be the way glog functionality/binary impl is conveyed, we aren't falling prey to this duplication and its ripply effects. I can understand that the internal monorepo might not have this issue.

FWIW, in my local build of CxxReact, glog, and folly (and boost, gflags, etc), I build them as static libraries with LTO bytecode so the optimization of the final app binary build crosses the library boundaries. This shaved a several hundred kilobytes off my final binary size (and RSS mem usage). When deploying to Android, maybe using the system-installed glog shared library would make more sense from an app binary size perspective, but at a runtime cost.

[shergin]

FWIW, in my local build of CxxReact, glog, and folly (and boost, gflags, etc), I build them as static libraries with LTO bytecode so the optimization of the final app binary build crosses the library boundaries.

Oh my, I want to know more!

Maybe should we include (incapsulate this into) <fabric/debug/logging.h> instead (and include whatever inside our own logging.h)?

(Let me import this to check something...)

[facebook-github-bot]

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

[shergin]

Ooops, suddenly, folly/GLog.h is not a part of folly-molly, the subset of Folly that we are using internally on mobile (and I think it makes sense), so all this does not compile internally.

If you really want to abstract GLOG, I would just move this to fabric/debug module.

[akshetpandey]

@matthargett @shergin : Any updates on this? Including GLog.h is causing macro redefinition issues for us.

[matthargett]

closing this since the GLog.h header isn't included in folly:molly, and the folly repo doesn't seem to expose a way to submit a PR to include it in that build config.