mozilla / spidernode

Node.js on top of SpiderMonkey
https://ehsanakhgari.org/blog/2016-04-20/project-spidernode
Other
561 stars 42 forks source link

align SpiderShim visibility settings to those of Node (via cares) #355

Closed mykmelez closed 7 years ago

mykmelez commented 7 years ago

Mac builds complain a lot (thousands of times on release builds, tens of times on debug builds) about "different translation units being compiled with different visibility settings." Here's an example:

ld: warning: direct access in js::RegExpGetSubstitution(JSContext, JS::Handle<JSLinearString>, JS::Handle<JSLinearString>, unsigned long, JS::Handle<JSObject>, JS::Handle<JSLinearString>, unsigned long, JS::MutableHandle) to global weak symbol JS::StructGCPolicy<JS::GCVector<JS::Value, 0ul, js::TempAllocPolicy> >::trace(JSTracer, JS::GCVector<JS::Value, 0ul, js::TempAllocPolicy>, char const) means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

It looks like the visibility settings are set in deps/cares/common.gypi, which appears to get applied to Node but not to SpiderShim. The specific settings are the xcode_settings keys GCC_INLINES_ARE_PRIVATE_EXTERN and GCC_SYMBOLS_PRIVATE_EXTERN, which are set to the string 'YES', which causes GYP's xcode_emulation.py script to set the cflags -fvisibility-inlines-hidden and -fvisibility=hidden, respectively.

This branch adds the same settings to deps/spidershim/spidershim.gyp, which squelches those warnings.

mykmelez commented 7 years ago

@tbsaunde Among other benefits, this reduces the Travis log size for Mac builds significantly. For example, job 17 (a Mac release build with BUILD_CONFIG=normal) goes from 6096 lines of log in https://travis-ci.org/mozilla/spidernode/jobs/194908348 to 3185 lines in https://travis-ci.org/mozilla/spidernode/jobs/195008140 .

mykmelez commented 7 years ago

A bit more investigation reinforces that this is the correct change. I'll go ahead and merge it.