Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Fix npm install from source #2237

Closed
4 tasks done
lucaswoj opened this issue Sep 2, 2015 · 11 comments · Fixed by #2288
Closed
4 tasks done

Fix npm install from source #2237

lucaswoj opened this issue Sep 2, 2015 · 11 comments · Fixed by #2288
Assignees
Labels
build Node.js node-mapbox-gl-native
Milestone

Comments

@lucaswoj
Copy link
Contributor

lucaswoj commented Sep 2, 2015

Node bindings are now included in this repo but it is impossible to install mapbox-gl-native from source using npm because

  1. building mapbox-gl-native from source relies on the ability to install git submodules
  2. npm strips out all git info (including submodules)

In order to make it possible to install from source, we will

  • require that mason be installed globally (or clone the latest release?)
  • install build geojsonvt via mason
  • modify build scripts to ignore irrelevant missing dependencies
  • fix iOS release and linux release builds

ref #1522

cc @jfirebaugh @mikemorris @tmcw @incanus @bsudekum

@lucaswoj lucaswoj changed the title Fix npm build from source Fix npm install from source Sep 2, 2015
@incanus incanus added the build label Sep 2, 2015
@incanus
Copy link
Contributor

incanus commented Sep 2, 2015

Refs also #2226 (keep in mind the date there).

@jfirebaugh
Copy link
Contributor

npm install mapbox-gl-native should not require styles or tests so those should actually be irrelevant.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Sep 2, 2015

@jfirebaugh Ok! Modified the original issue. Omitting those should somewhat simplify the task.

@mikemorris
Copy link
Contributor

Moving from submodules to node_modules refs #1358

@jfirebaugh jfirebaugh added the Node.js node-mapbox-gl-native label Sep 3, 2015
@mikemorris
Copy link
Contributor

I tracked down the issue with the Linux GCC build: the linux-x86_64/geojsonvt/1.1.0 mason binary was built with clang-3.5, and is incompatible with mapbox-gl-native compiled under g++-4.9.

Building both libraries with g++-4.9 works fine, as does building both libraries with clang-3.5, but mixing generates mismatched symbols.

@TheMarex suggested that this change in GCC 4.9 could be responsible:

When using a linker plugin, compiling with the -flto option now generates slim object files (.o) which only contain intermediate language representation for LTO. Use -ffat-lto-objects to create files which contain additionally the object code. To generate static libraries suitable for LTO processing, use gcc-ar and gcc-ranlib; to list symbols from a slim object file use gcc-nm. (This requires that ar, ranlib and nm have been compiled with plugin support.)
https://gcc.gnu.org/gcc-4.9/changes.html

Suggestions @springmeyer @kkaefer?

@daniel-j-h
Copy link

Capturing from chat:

Re. LTO: GCC dumps a different intermediate format than Clang (which is LLVM IR, you can even link those modules using llvm-link).

More importantly, mixing compilers is a bad idea and will not work. Even mixing different versions of the same compiler normally won't work, but for that you can read the changelog about ABI breakage. Mixing stdlibs is worse, and mixing libraries that were compiled with different C++ standard (e.g. 98 and 11) also wont work (example: copy on write string implementations were basically forbidden with latest standards).

Sounds a bit harsh? :P

@mikemorris
Copy link
Contributor

Tested locally (and now passing on Travis too), fcb8f0d is 👍 as a workaround to just build geojsonvt from source rather than look for a mason binary. I'm sure somebody more familiar with shell scripting could clean this up a bit.

@jfirebaugh
Copy link
Contributor

@springmeyer Thoughts on how C++ stdlib conflicts should be handled with mason?

@springmeyer
Copy link
Contributor

I tracked down the issue with the Linux GCC build: the linux-x86_64/geojsonvt/1.1.0 mason binary was built with clang-3.5, and is incompatible with mapbox-gl-native compiled under g++-4.9.

Building both libraries with g++-4.9 works fine, as does building both libraries with clang-3.5, but mixing generates mismatched symbols.

Reading back here... To clarify: this issue has nothing to do with LTO. It certainly could, but in this case LTO (-flto) is not a flag that be being used for mgbl or geojson-vt. Also it does not represent a conflict between libstdc++ and libc++ since only libstdc++ is used (both both clang++ and g++ on linux are using libstdc++ in this case).

So that leaves possible culprits as:

  • Different libstdc++ versions linked for the mason pre-compiled geojson-vt binary vs the mgbl build (unlikely)
  • Different behavior of clang++-3.5 (used to compile geojson-vt in mason: https://github.com/mapbox/mason/blob/geojsonvt-1.1.0/.travis.yml#L12) vs g++-4.9 due to fundamental differences in those compilers (I'm doubtful)
  • Different behavior in compiler object file generation due to compiler flags/visibility issues in the code (possible).

@jfirebaugh helped me dig up the actual linking error which was:

./Release/obj.target/libmbgl-core.a(annotation.o): In function `_ZN4mbgl17AnnotationManager14addTileFeatureEjRKSt6vectorIS1_INS_6LatLngESaIS2_EESaIS4_EERKS1_IS1_INS_4vec2IdEESaISA_EESaISC_EERKNS_14AnnotationTypeERKN6mapbox4util7variantIJNS_14FillPropertiesENS_14LinePropertiesENS_16CirclePropertiesENS_16SymbolPropertiesENS_16RasterPropertiesENS_20BackgroundPropertiesESt17integral_constantIbLb0EEEEERKSt13unordered_mapISsSsSt4hashISsESt8equal_toISsESaISt4pairIKSsSsEEEh':
/home/travis/build/mapbox/mapbox-gl-native/build/linux-x86_64/../../src/mbgl/map/annotation.cpp:137: undefined reference to `mapbox::util::geojsonvt::Convert::create(std::map<std::string, std::string, std::less<std::string>, std::allocator<std::pair<std::string const, std::string> > >, mapbox::util::geojsonvt::ProjectedFeatureType, mapbox::util::variant<mapbox::util::geojsonvt::ProjectedPoint, mapbox::util::geojsonvt::ProjectedGeometryContainer>)'
collect2: error: ld returned 1 exit status

Which occurred when using g++-4.9 to link mapbox-gl-native against libgeojsonvt.a built with clang-3.5 per @mikemorris

Next actions:

  • rule out if different libstdc++ versions are being used on mason compile runs vs mgbl travis runs
  • @mikemorris helps @springmeyer get set up to replicate linker error

@mikemorris
Copy link
Contributor

rule out if different libstdc++ versions are being used on mason compile runs vs mgbl travis runs

  • libstdc++-20141104 (date is version from __GLIBCXX__) is being used on clang-3.5 and gcc-4.9 mason Linux builds of geojsonvt-2.1.6.1
  • libc++-1 (1 being _LIBCPP_ABI_VERSION, this seemed sufficient instead of _LIBCPP_VERSION http://sourceforge.net/p/predef/wiki/Libraries/)

Linux and OS X builds of MBGL are using the same respective stdlibs in every case.

@mikemorris
Copy link
Contributor

Replicated the g++-4.9 linker error in #2489
https://travis-ci.org/mapbox/mapbox-gl-native/jobs/83385446

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build Node.js node-mapbox-gl-native
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants