Install GeoJSONVT 2.1.6 from mason

[mikemorris]

Fixes #2226, #898

• Defines cxx_host gyp variable to set -fabi-version=0 on non-clang builds, fixes Building mbgl-native with g++ against binary libs built with clang++ #2502
• Sets 'GCC_SYMBOLS_PRIVATE_EXTERN': 'YES' to prevent linker warnings on OS X builds using mason binaries built with -fvisibility-inlines-hidden=hidden or 'GCC_SYMBOLS_PRIVATE_EXTERN': 'YES'
• Links libgeojsonvt.a in test/ios/ios-tests.xcodeproj to fix an undefined symbol (unused in libmbgl-core.a) when linking iOS tests.
• Sets Inline Methods Hidden in test/ios/ios-tests.xcodeproj to fix linker warnings.

TODO:

• gcc-5 #2582 should be merged first for gcc-5, necessary for compatibility with mason libraries build with clang-3.5 (Building mbgl-native with g++ against binary libs built with clang++ #2502)
• Pull request and merge geojson-vt-cpp branch visibility-hidden and tag a new release (v2.1.6.2).
• Add a new mason package with the -fvisibility=hidden geojson-vt-cpp release
• After merging this, rebase Enable npm to build mapbox-gl-native package from source #2352, preserving @lucaswoj changes to build without a mason submodule

/cc @jfirebaugh @1ec5 @incanus @springmeyer

[mikemorris]

Travis iOS build is running to completion now, but still failing, looks like lots of linker warnings from symbol visibility issues still 😞

[mikemorris]

I'm hitting a problem with MBGL mason-installed RapidJSON 1.0.2 causing symbol visibility linker warnings when building the iOS tests, which causes the Travis build to fail even though the tests compile, run and complete successfully.

GeoJSONVT depends on mason-installed RapidJSON 1.0.2 and I wonder if duplicate copies of these headers could be a problem? Reverting 2066295 seems to resolve this for now, maybe we should revisit RapidJSON as a mason module at a later point.

More info on Xcode build settings at https://developer.apple.com/library/mac/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html

/cc @jfirebaugh @1ec5

[jfirebaugh]

I get link errors building osxapp in Xcode, starting with:

duplicate symbol __ZN6mapbox4util9geojsonvt9GeoJSONVT7getTileEhjj in:
    /Users/john/Library/Developer/Xcode/DerivedData/osx-bcewhggtbhhvglavjnxcctvozqgg/Build/Products/Debug/libmbgl-core.a(geojsonvt.o)
    /Users/john/Development/mapbox-gl-native/mason_packages/osx-10.11/geojsonvt/2.1.6/lib/libgeojsonvt.a(geojsonvt.o)

[jfirebaugh]

Oh, stale submodule leftovers, disregard.

[mikemorris]

From herzbube/littlego#3 (comment) and https://stackoverflow.com/questions/1601900/dont-expose-symbols-from-a-used-library-in-own-static-library it sounds like XCode does all sorts of screwy uncontrollable things when compiling and linking. I'm completely frustrated with trying to get mason GeoJSONVT and RapidJSON to work together. Current work in https://github.com/mapbox/mapbox-gl-native/compare/geojsonvt-2.1.6-rapidjson, could really use some insight from @1ec5 or @incanus into XCode build settings here.

[1ec5]

After merging geojsonvt-2.1.6-rapidjson with master locally, I also get this linker error:

Undefined symbols for architecture x86_64:
  "_OBJC_CLASS_$_MGLPointAnnotation", referenced from:
      objc-class-ref in MapViewTests.o

[mikemorris]

@1ec5 That linker error should just be a simple fix with MBGL_PUBLIC prefix on class declaration like ce074bc (curious why I didn't see it though).

[1ec5]

curious why I didn't see it though

The use of MGLPointAnnotation is new (introduced in #2612).

[mikemorris]

Added MBGL_PUBLIC to MGLPointAnnotation upstream in 898-visibility-hidden and rebased @1ec5.

[mikemorris]

Pushed up a version that preserves the RapidJSON mason package, uses a new geojson-vt-cpp branch that sets -fvisibility=hidden, and sets -fvisibility=hidden in MBGL core to avoid differing visibility linker errors. This is building with no linker errors for me locally now. I edited the initial comment here with an updated TODO list (should tag a new release of geojson-vt-cpp before merging this).

/cc @jfirebaugh @1ec5 @tmpsantos for review

[mikemorris]

Bitrise iOS tests are failing because of a known issue (tracked in #2769), but are compiling and linking successfully, so not considering this to be blocked.

/cc @incanus

[lucaswoj]

[incanus]

We don't do iOS on Travis anymore as of a243f41, just Bitrise. Also, just closed/merged #2769 today.

[mikemorris]

Current Travis test failures being tracked in #2819

[1ec5]

No good: if I let CocoaPods wrap libMapbox.a in a dynamic library and link an application against that library, I get the following linker errors:

Undefined symbols for architecture x86_64:
  "mapbox::util::geojsonvt::Convert::create(std::__1::map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::less<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > >, mapbox::util::geojsonvt::ProjectedFeatureType, mapbox::util::variant<mapbox::util::geojsonvt::ProjectedPoint, mapbox::util::geojsonvt::ProjectedGeometryContainer>)", referenced from:
      mbgl::ShapeAnnotationImpl::updateTile(mbgl::TileID const&, mbgl::AnnotationTile&) in libMapbox.a(shape_annotation_impl.o)
  "mapbox::util::geojsonvt::Convert::project(std::__1::vector<mapbox::util::geojsonvt::LonLat, std::__1::allocator<mapbox::util::geojsonvt::LonLat> > const&, double)", referenced from:
      mbgl::ShapeAnnotationImpl::updateTile(mbgl::TileID const&, mbgl::AnnotationTile&) in libMapbox.a(shape_annotation_impl.o)
  "mapbox::util::geojsonvt::GeoJSONVT::getTile(unsigned char, unsigned int, unsigned int)", referenced from:
      mbgl::ShapeAnnotationImpl::updateTile(mbgl::TileID const&, mbgl::AnnotationTile&) in libMapbox.a(shape_annotation_impl.o)
  "mapbox::util::geojsonvt::GeoJSONVT::GeoJSONVT(std::__1::vector<mapbox::util::geojsonvt::ProjectedFeature, std::__1::allocator<mapbox::util::geojsonvt::ProjectedFeature> >, unsigned char, unsigned char, unsigned int, bool, double)", referenced from:
      mbgl::ShapeAnnotationImpl::updateTile(mbgl::TileID const&, mbgl::AnnotationTile&) in libMapbox.a(shape_annotation_impl.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

[incanus]

Dumb, obvious statement: x86_64 refers to the iOS simulator slice (one of two, usually, the other being i386 32-bit) that needs to be built for local simulation. Is GeoJSONVT being built for that slice?

[jfirebaugh]

@1ec5 Is that something we could/should be testing in CI?

[1ec5]

I get the same errors after setting GEOJSONVT_VERSION=visibility-default in scripts/ios/configure.sh, distcleaning, and dropping the library into an existing pod.

[mikemorris]

Welp, those symbols are definitely missing @1ec5. Not sure why yet.

nm -m build/ios/pkg/static/libMapbox.a | grep geojsonvt
                 (undefined) external __ZN6mapbox4util9geojsonvt7Convert6createENSt3__13mapINS3_12basic_stringIcNS3_11char_traitsIcEENS3_9allocatorIcEEEESA_NS3_4lessISA_EENS8_INS3_4pairIKSA_SA_EEEEEENS1_20ProjectedFeatureTypeENS0_7variantIJNS1_14ProjectedPointENS1_26ProjectedGeometryContainerEEEE
                 (undefined) external __ZN6mapbox4util9geojsonvt7Convert7projectERKNSt3__16vectorINS1_6LonLatENS3_9allocatorIS5_EEEEd
                 (undefined) external __ZN6mapbox4util9geojsonvt9GeoJSONVT7getTileEhjj
                 (undefined) external __ZN6mapbox4util9geojsonvt9GeoJSONVTC1ENSt3__16vectorINS1_16ProjectedFeatureENS3_9allocatorIS5_EEEEhhjbd

[springmeyer]

Welp, those symbols are definitely missing @1ec5. Not sure why yet.

If this is not solvable through visibility fixes, it may be that the linker is intentionally removing the symbols because they appear unused. Something to try then would be to ask the linker to create libMapbox.a with all symbols, even those that look unused by passing -Wl,--whole-archive

[ljbade]

I tested this branch on my Android Samsung and did not have any problems.

[mikemorris]

Yea, that's kinda my current line of thinking @springmeyer, that they appear unused because they're defined in libmbgl-core.a but only used in platform-ios.a. I think the multiple tiers of compilation units/linking is definitely making this more complex (iOS in particular is using libtool to squish everything together at the end) - I had tried adding -Wl,--whole-archive around geojsonvt_static_libs yesterday but I'm assuming I implemented it wrong since it didn't seem to have an effect.

[mikemorris]

Still seeing 199 variations of the following (apparently spurious) linker warning in Xcode when copying libMapbox.a into a CocoaPods project (created following the instructions at https://www.mapbox.com/ios-sdk), but no more errors and I'm able to successfully build and make a map show up now! (Following https://www.mapbox.com/guides/first-steps-ios-sdk/)

warning: (x86_64) /Users/mikemorris/Dropbox (Mapbox)/Projects/CocoaPods/Pods/Mapbox-iOS-SDK/libMapbox.a(libuv_la-fs-poll.o) unable to open object file: No such file or directory

(I may have missed some MGL* classes that need to be made publicly visible with MBGL_PUBLIC, but won't cause linker errors until actually trying to use them in an Xcode project).

Can you try this out @1ec5 @springmeyer?

[mikemorris]

Okay, pushed up a rebased branch that only sets -fvsibility-inlines-hidden instead of -fvisibility=hidden and drops the __attribute__ ((visibility ("default"))) stuff from the iOS public API. We can try moving fully to -fvisibility=hidden later but I think this is all working as is now...