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.

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

Replace Travis with Github Actions for CI builds #6071

Merged
merged 3 commits into from
Sep 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
588 changes: 588 additions & 0 deletions .github/workflows/osrm-backend.yml

Large diffs are not rendered by default.

585 changes: 0 additions & 585 deletions .travis.yml

This file was deleted.

4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# Unreleased
- Changes from 5.25.0
- API:
- FIXED: Allow for special characters in the profile/method as part of the HTTP URL.
- FIXED: Allow for special characters in the profile/method as part of the HTTP URL. [#6090](https://github.com/Project-OSRM/osrm-backend/pull/6090)
- Build:
- CHANGED: Replace Travis with Github Actions for CI builds [#6071](https://github.com/Project-OSRM/osrm-backend/pull/6071)

# 5.25.0
- Changes from 5.24.0
Expand Down
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ if(ENABLE_MASON)
set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${LINKER_FLAGS}")

# current mason packages target -D_GLIBCXX_USE_CXX11_ABI=0
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -D_GLIBCXX_USE_CXX11_ABI=0")
add_dependency_defines(-D_GLIBCXX_USE_CXX11_ABI=0)
Copy link
Member Author

Choose a reason for hiding this comment

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

On Travis Trusty builds, the compilers all had GLIBCXX_USE_CXX11_ABI=0 by default. This setting has been flipped in Bionic, so we need to explicitly pass this to pkg-config so that the example builds.

Copy link
Member

Choose a reason for hiding this comment

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

I forgot why we had this in the first place. I think this was mason related given that is afaik the only place where we care about ABI of our binaries.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, the Mason Boosts are built against the old ABI .


# note: we avoid calling find_package(Osmium ...) here to ensure that the
# expat and bzip2 are used from mason rather than the system
Expand Down Expand Up @@ -862,4 +862,4 @@ if (ENABLE_NODE_BINDINGS)
endforeach()
add_library(check-headers STATIC EXCLUDE_FROM_ALL ${sources})
set_target_properties(check-headers PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${check_headers_dir})
endif()
endif()
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
6 changes: 0 additions & 6 deletions src/extractor/maneuver_override_relation_parser.cpp
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
#include "extractor/maneuver_override_relation_parser.hpp"
#include "extractor/maneuver_override.hpp"

#include "util/log.hpp"

#include <boost/algorithm/string.hpp>
#include <boost/algorithm/string/predicate.hpp>
#include <boost/algorithm/string/regex.hpp>
#include <boost/optional/optional.hpp>
#include <boost/ref.hpp>
#include <boost/regex.hpp>

#include <osmium/osm.hpp>
#include <osmium/tags/filter.hpp>
Expand Down
17 changes: 8 additions & 9 deletions src/extractor/restriction_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
#include "util/log.hpp"

#include <boost/algorithm/string/predicate.hpp>
#include <boost/algorithm/string/regex.hpp>
#include <boost/optional/optional.hpp>
#include <boost/ref.hpp>
#include <boost/regex.hpp>

#include <osmium/osm.hpp>
#include <osmium/tags/regex_filter.hpp>
Expand Down Expand Up @@ -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("[;][ ]*"));
const std::regex delimiter_re("[;][ ]*");
Copy link
Member Author

@mjjbell mjjbell Jul 4, 2021

Choose a reason for hiding this comment

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

Similar problem to the Travis Xenial upgrade attempt.
Boost.Regex and GCC5 appears to be buggy with respect to [abi:cxx11] tags, so using this as an opportunity to replace Boost.Regex with std::regex.

Copy link
Member

Choose a reason for hiding this comment

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

Nice!

std::sregex_token_iterator except_tags_begin(
except_tag_string.begin(), except_tag_string.end(), delimiter_re, -1);
std::sregex_token_iterator except_tags_end;

return std::any_of(
std::begin(exceptions), std::end(exceptions), [&](const std::string &current_string) {
return std::end(restrictions) !=
std::find(std::begin(restrictions), std::end(restrictions), current_string);
});
return std::any_of(except_tags_begin, except_tags_end, [&](const std::string &current_string) {
return std::end(restrictions) !=
std::find(std::begin(restrictions), std::end(restrictions), current_string);
});
}
} // namespace extractor
} // namespace osrm
5 changes: 2 additions & 3 deletions unit_tests/library/json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "engine/api/json_factory.hpp"
#include "osrm/coordinate.hpp"

#include <iterator>
#include <vector>

using namespace osrm;
Expand All @@ -27,7 +26,7 @@ BOOST_AUTO_TEST_CASE(test_json_linestring)
const auto coords = geom.values["coordinates"].get<util::json::Array>().values;
BOOST_CHECK_EQUAL(coords.size(), 3); // array of three location arrays

for (const auto each : coords)
for (const auto &each : coords)
Copy link
Member Author

Choose a reason for hiding this comment

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

Unit test changes are due to compiler errors from updated macOS clang.

Copy link
Member

Choose a reason for hiding this comment

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

This crashes clang?

Copy link
Member Author

Choose a reason for hiding this comment

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

Running make tests with clang 12

error: loop variable 'each' creates a copy from type 'const mapbox::util::variant<osrm::util::json::String, osrm::util::json::Number, mapbox::util::recursive_wrapperosrm::util::json::Object, mapbox::util::recursive_wrapperosrm::util::json::Array, osrm::util::json::True, osrm::util::json::False, osrm::util::json::Null>' [-Werror,-Wrange-loop-construct]
for (const auto each : coords)

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see -Werror strikes again.

{
const auto loc = each.get<util::json::Array>().values;
BOOST_CHECK_EQUAL(loc.size(), 2);
Expand All @@ -53,7 +52,7 @@ BOOST_AUTO_TEST_CASE(test_json_single_point)
const auto coords = geom.values["coordinates"].get<util::json::Array>().values;
BOOST_CHECK_EQUAL(coords.size(), 2); // array of two location arrays

for (const auto each : coords)
for (const auto &each : coords)
{
const auto loc = each.get<util::json::Array>().values;
BOOST_CHECK_EQUAL(loc.size(), 2);
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/library/match.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ BOOST_AUTO_TEST_CASE(test_match_fb_serialization)
const auto matchings = fb->routes();
const auto &number_of_matchings = matchings->size();

for (const auto &waypoint : *waypoints)
for (const auto waypoint : *waypoints)
Copy link
Member

Choose a reason for hiding this comment

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

Also clang-related?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, similar errors to above: https://github.com/mjjbell/osrm-backend/runs/2957246502?check_suite_focus=true#step:15:2784

error: loop variable 'destination' is always a copy because the range of type 'const flatbuffers::Vector<flatbuffers::Offsetosrm::engine::api::fbresult::Waypoint >' does not return a reference [-Werror,-Wrange-loop-analysis]
for (const auto &destination : *destinations_array)
^
/Users/runner/work/osrm-backend/osrm-backend/unit_tests/library/table.cpp:335:10: note: use non-reference type 'const osrm::engine::api::fbresult::Waypoint *'
for (const auto &destination : *destinations_array)

{
BOOST_CHECK(waypoint_check(waypoint));
const auto matchings_index = waypoint->matchings_index();
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/library/nearest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ BOOST_AUTO_TEST_CASE(test_nearest_fb_serialization)
auto waypoints = fb->waypoints();
BOOST_CHECK(waypoints->size() > 0); // the dataset has at least one nearest coordinate

for (const auto &waypoint : *waypoints)
for (const auto waypoint : *waypoints)
{
BOOST_CHECK(waypoint->distance() >= 0);
BOOST_CHECK(waypoint->nodes()->first() != 0);
Expand Down
10 changes: 5 additions & 5 deletions unit_tests/library/route.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ void test_route_same_coordinates(bool use_json_only_api)
const auto &entries = intersection_object.at("entry").get<json::Array>().values;
BOOST_CHECK(bearings.size() == entries.size());

for (const auto bearing : bearings)
for (const auto &bearing : bearings)
BOOST_CHECK(0. <= bearing.get<json::Number>().value &&
bearing.get<json::Number>().value <= 360.);

Expand Down Expand Up @@ -619,7 +619,7 @@ BOOST_AUTO_TEST_CASE(test_route_serialize_fb)
const auto waypoints = fb->waypoints();
BOOST_CHECK(waypoints->size() == params.coordinates.size());

for (const auto &waypoint : *waypoints)
for (const auto waypoint : *waypoints)
{
const auto longitude = waypoint->location()->longitude();
const auto latitude = waypoint->location()->latitude();
Expand All @@ -633,15 +633,15 @@ BOOST_AUTO_TEST_CASE(test_route_serialize_fb)
const auto routes = fb->routes();
BOOST_REQUIRE_GT(routes->size(), 0);

for (const auto &route : *routes)
for (const auto route : *routes)
{
BOOST_CHECK_EQUAL(route->distance(), 0);
BOOST_CHECK_EQUAL(route->duration(), 0);

const auto &legs = route->legs();
BOOST_CHECK(legs->size() > 0);

for (const auto &leg : *legs)
for (const auto leg : *legs)
{
BOOST_CHECK_EQUAL(leg->distance(), 0);

Expand Down Expand Up @@ -720,7 +720,7 @@ BOOST_AUTO_TEST_CASE(test_route_serialize_fb_skip_waypoints)
const auto routes = fb->routes();
BOOST_REQUIRE_GT(routes->size(), 0);

for (const auto &route : *routes)
for (const auto route : *routes)
{
BOOST_CHECK_EQUAL(route->distance(), 0);
BOOST_CHECK_EQUAL(route->duration(), 0);
Expand Down
4 changes: 2 additions & 2 deletions unit_tests/library/table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,14 +332,14 @@ BOOST_AUTO_TEST_CASE(test_table_serialiaze_fb)
// check destinations array of waypoint objects
const auto &destinations_array = fb->table()->destinations();
BOOST_CHECK_EQUAL(destinations_array->size(), params.destinations.size());
for (const auto &destination : *destinations_array)
for (const auto destination : *destinations_array)
{
BOOST_CHECK(waypoint_check(destination));
}
// check sources array of waypoint objects
const auto &sources_array = fb->waypoints();
BOOST_CHECK_EQUAL(sources_array->size(), params.sources.size());
for (const auto &source : *sources_array)
for (const auto source : *sources_array)
{
BOOST_CHECK(waypoint_check(source));
}
Expand Down
2 changes: 1 addition & 1 deletion unit_tests/library/trip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ BOOST_AUTO_TEST_CASE(test_roundtrip_response_fb_serialization)
const auto trips = fb->routes();
BOOST_CHECK_EQUAL(trips->size(), 1);

for (const auto &waypoint : *waypoints)
for (const auto waypoint : *waypoints)
{
const auto longitude = waypoint->location()->longitude();
const auto latitude = waypoint->location()->latitude();
Expand Down
6 changes: 4 additions & 2 deletions unit_tests/server/url_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,10 @@ BOOST_AUTO_TEST_CASE(valid_urls)
BOOST_CHECK_EQUAL(reference_8.prefix_length, result_8->prefix_length);

// profile with special characters
api::ParsedURL reference_9{"route", 1, "foo-bar_baz.profile", "0,1;2,3;4,5?options=value&foo=bar", 30UL};
auto result_9 = api::parseURL("/route/v1/foo-bar_baz.profile/0,1;2,3;4,5?options=value&foo=bar");
api::ParsedURL reference_9{
"route", 1, "foo-bar_baz.profile", "0,1;2,3;4,5?options=value&foo=bar", 30UL};
auto result_9 =
api::parseURL("/route/v1/foo-bar_baz.profile/0,1;2,3;4,5?options=value&foo=bar");
BOOST_CHECK(result_9);
BOOST_CHECK_EQUAL(reference_9.service, result_9->service);
BOOST_CHECK_EQUAL(reference_9.version, result_9->version);
Expand Down