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.

Sign up for GitHub

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

Jump to bottom

Upgrade ci to xenial #5901

Closed
wants to merge 5 commits into from
Closed

Upgrade ci to xenial #5901

wants to merge 5 commits into from

Conversation

dburnsii
Copy link
Contributor

@dburnsii dburnsii commented Dec 2, 2020

Issue

Closes #5900

Tasklist

Requirements / Relations

#5887

@dburnsii dburnsii force-pushed the upgrade_ci_to_xenial branch from 010e246 to 7fc5c05 Compare December 2, 2020 00:38
@mjjbell mjjbell closed this May 22, 2021
@mjjbell mjjbell reopened this May 22, 2021
@mjjbell
Copy link
Member

mjjbell commented May 22, 2021

A few build/compiler changes are needed to get this to go green. Will make some fixes.

@mjjbell mjjbell added the CI label May 22, 2021
@mjjbell mjjbell force-pushed the upgrade_ci_to_xenial branch from f302508 to f575aed Compare May 23, 2021 21:43
@mjjbell
Copy link
Member

mjjbell commented May 24, 2021

The example program fails to build for mason-linux-release with a CXX11 ABI linker failure.

CMakeFiles/osrm-example.dir/example.cpp.o:/home/travis/build/Project-OSRM/osrm-backend/example/example.cpp:function main: error: undefined reference to 'osrm::OSRM::Route(osrm::engine::api::RouteParameters const&, mapbox::util::variant<osrm::util::json::Object, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, flatbuffers::FlatBufferBuilder>&) const'

Whilst this could be fixed by also building the example with -D_GLIBCXX_USE_CXX11_ABI=0, inspection of the node binary shows it contains libstdc++ symbols from GCC 5.1+

readelf -s ../lib/binding/node_osrm.node | grep GLIBCXX_3.4.21
    58: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND _ZNSt13runtime_errorC2EPK@GLIBCXX_3.4.21 (6)
    59: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND _ZNSt11logic_errorC1EPKc@GLIBCXX_3.4.21 (6)
    68: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND _ZNSt6thread15_M_start_th@GLIBCXX_3.4.21 (6)
   248: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND _ZNSt12out_of_rangeC1EPKc@GLIBCXX_3.4.21 (6)
   296: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND _ZNSt13runtime_errorC1EPK@GLIBCXX_3.4.21 (6)

This means maintaining standard lib compatibility by downgrading to libstdc++4.9 is not viable with Xenial builds (unless someone knows how to make this work).

Node 10 has GCC 4.9 as a minimum toolchain requirement. So despite Trusty being EOL, I don't think we can upgrade CI if we want to keep the same level of support for Node 10.

@danpat
Copy link
Member

danpat commented May 24, 2021

@mjjbell I don't feel strongly that we need to continue supporting Node 10 - it's totally EOL.

@mjjbell
Copy link
Member

mjjbell commented May 24, 2021 •
edited
Loading

👍 that will make things easier. In which case we can bump the toolchain to the Xenial default (GCC 5.4.0)

@mjjbell mjjbell force-pushed the upgrade_ci_to_xenial branch 4 times, most recently from f335c44 to 53382d7 Compare May 26, 2021 20:32
mjjbell added 3 commits May 26, 2021 21:45
@mjjbell
Remove Boost.Regex dependency
f0c8375 
Travis Xenial container has Boost 1.58.0 built with GCC5 against
the CXX11 ABI. However, there is a bug that affects the Regex library.
Some symbols are missing [abi:cxx11] tags which causes linking to
fail if not also building OSRM with GCC5.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=823978

Given there is only one use of the Regex libary, use this opportunity
to replace it with std::regex and avoid the linking issue entirely.
@mjjbell
Remove support for libstdc++4.9
1ddc684 
Xenial toolchain only supports GCC 5. Builds against older versions
of libstdc++ do not work.
@mjjbell
Include -D_GLIBCXX_USE_CXX11_ABI=0 in pkg-config flags
17dcb7a 
Ensure that any program using libosrm as a dependency builds against
the same libstdc++ ABI. This is needed as Mason dependencies are
still built against the old ABI.
@mjjbell mjjbell force-pushed the upgrade_ci_to_xenial branch from 53382d7 to 17dcb7a Compare May 26, 2021 20:46
mjjbell
mjjbell reviewed May 26, 2021
View reviewed changes
src/extractor/restriction_parser.cpp
@@ -244,14 +242,15 @@ bool RestrictionParser::ShouldIgnoreRestriction(const std::string &except_tag_st

// Be warned, this is quadratic work here, but we assume that
// only a few exceptions are actually defined.
std::vector<std::string> exceptions;
boost::algorithm::split_regex(exceptions, except_tag_string, boost::regex("[;][ ]*"));
Copy link
Member

Choose a reason for hiding this comment

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

Removed usage of Boost.Regex as there's a bug in the version used by Xenial CI.
Some symbols are missing [abi:cxx11] tags which causes linking to fail if not also building OSRM with GCC5.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=823978

Used this as an opportunity to replace it with std::regex.

I didn't remove Boost.Regex as a dependency completely as it makes the Windows build fail - I'm not sure how to fix that with out changing the Wolt hosted dependencies.

This was referenced Jul 4, 2021
Closed
Merged
@mjjbell
Copy link
Member

mjjbell commented Sep 3, 2021

Superseded by #6071

@mjjbell mjjbell closed this Sep 3, 2021
@mjjbell mjjbell mentioned this pull request Jul 9, 2022
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjjbell mjjbell mjjbell left review comments

Assignees
No one assigned
Labels
CI Ready for Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Trusty is EOL, update CI
3 participants
@dburnsii @mjjbell @danpat