Implements Cucumber shared vs. static library detection, fixes #2999

Technically speaking we're changing the `libosrm` API.

But since we're only lifting restrictions by marking the API threadsafe,
we should be fine here.

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 5.4.0-rc.7
+  - Chages from 5.4.0-rc.6
+    - Bugfixes re-introduce space between two entries in summaries
+
 # 5.4.0-rc.6
   - Changes from 5.4.0-rc.5
     - Bugfixes

CMakeLists.txt

@@ -127,8 +127,8 @@ if(ENABLE_GOLD_LINKER)
 
         # Issue 2785: check gold binutils version and don't use gc-sections for versions prior 2.25
         string(REGEX REPLACE ".*\\(GNU Binutils[^\\)0-9]+([0-9]+\\.[0-9]+)[^\\)]*\\).*" "\\1" GOLD_BINUTILS_VERSION "${LD_VERSION}")
-        if ("${GOLD_BINUTILS_VERSION}" VERSION_LESS "2.25")
+        if ("${GOLD_BINUTILS_VERSION}" VERSION_LESS "2.26")
-          message(STATUS "Disabling gc-sections on gold binutils < 2.25, see: https://sourceware.org/bugzilla/show_bug.cgi?id=17639")
+          message(STATUS "Disabling gc-sections on gold binutils < 2.26, see: https://sourceware.org/bugzilla/show_bug.cgi?id=17639")
           set(LD_AVOID_GC_SECTIONS TRUE)
         endif()
     else()

example/example.cpp

@@ -34,7 +34,7 @@ int main(int argc, const char *argv[])
     config.use_shared_memory = false;
 
     // Routing machine with several services (such as Route, Table, Nearest, Trip, Match)
-    OSRM osrm{config};
+    const OSRM osrm{config};
 
     // The following shows how to use the Route service; configure this service
     RouteParameters params;

features/car/summaries.feature

@@ -23,10 +23,10 @@ Feature: Basic Routing
             | df    | street |
 
         When I route I should get
-            | waypoints | route                               | summary               |
+            | waypoints | route                               | summary                 |
-            | a,e       | road,,1 st,1 st                     | road,1 st             |
+            | a,e       | road,,1 st,1 st                     | road, 1 st              |
-            | a,d,f     | road,,,street,street                | road;street           |
+            | a,d,f     | road,,,street,street                | road;street             |
-            | a,e,f     | road,,1 st,1 st,1 st,street,street  | road,1 st;1 st,street |
+            | a,e,f     | road,,1 st,1 st,1 st,street,street  | road, 1 st;1 st, street |
 
      Scenario: Name Empty
         Given the node map
@@ -51,8 +51,8 @@ Feature: Basic Routing
             | bc    |      | 101 |
 
         When I route I should get
-            | waypoints | route | summary  |
+            | waypoints | route | summary   |
-            | a,c       | road, | road,101 |
+            | a,c       | road, | road, 101 |
 
      Scenario: Only Refs
         Given the node map
@@ -64,8 +64,8 @@ Feature: Basic Routing
             | bc    |      | 101 |
 
         When I route I should get
-            | waypoints | route  | summary |
+            | waypoints | route  | summary  |
-            | a,c       | ,      | 100,101 |
+            | a,c       | ,      | 100, 101 |
 
      Scenario: Single Ref
         Given the node map

features/support/env.js

@@ -51,8 +51,26 @@ module.exports = function () {
         } else {
             this.TERMSIGNAL = 'SIGTERM';
             this.EXE = '';
-            // TODO autodetect if this was build with shared or static libraries
+
-            this.LIB = process.env.BUILD_SHARED_LIBS && '.so' || '.a';
+            // heuristically detect .so/.a suffix
+            this.LIB = null;
+
+            try {
+                const dot_a = util.format('%s/libosrm%s', this.BIN_PATH, '.a');
+                fs.accessSync(dot_a, fs.F_OK);
+                this.LIB = '.a';
+            } catch(e) { /*nop*/ }
+
+            try {
+                const dot_so = util.format('%s/libosrm%s', this.BIN_PATH, '.so');
+                fs.accessSync(dot_so, fs.F_OK);
+                this.LIB = '.so';
+            } catch(e) { /*nop*/ }
+
+            if (!this.LIB) {
+                throw new Error('*** Unable to detect dynamic or static libosrm libraries');
+            }
+
             this.QQ = '';
         }
 
@@ -65,7 +83,7 @@ module.exports = function () {
 
         // eslint-disable-next-line no-console
         console.info(util.format('Node Version', process.version));
-        if (parseInt(process.version.match(/v(\d)/)[1]) < 4) throw new Error('*** PLease upgrade to Node 4.+ to run OSRM cucumber tests');
+        if (parseInt(process.version.match(/v(\d)/)[1]) < 4) throw new Error('*** Please upgrade to Node 4.+ to run OSRM cucumber tests');
 
         fs.exists(this.TEST_PATH, (exists) => {
             if (exists)

features/testbot/summary.feature

@@ -18,12 +18,12 @@ Feature: Basic Routing
             | de    |
 
         When I route I should get
-            | from | to | route          | summary |
+            | from | to | route          | summary  |
-            | a    | e  | ab,bc,cd,de,de | ab,bc   |
+            | a    | e  | ab,bc,cd,de,de | ab, bc   |
-            | e    | a  | de,cd,bc,ab,ab | de,bc   |
+            | e    | a  | de,cd,bc,ab,ab | de, bc   |
-            | a    | b  | ab,ab          | ab      |
+            | a    | b  | ab,ab          | ab       |
-            | b    | d  | bc,cd,cd       | bc,cd   |
+            | b    | d  | bc,cd,cd       | bc, cd   |
-            | 1    | c  | bc,bc          | bc      |
+            | 1    | c  | bc,bc          | bc       |
 
     @smallest
     Scenario: Check handling empty values
@@ -40,8 +40,8 @@ Feature: Basic Routing
             | df    | df   |
 
         When I route I should get
-            | from | to | route          | summary |
+            | from | to | route          | summary  |
-            | e    | a  | de,,bc,ab,ab   | de,bc   |
+            | e    | a  | de,,bc,ab,ab   | de, bc   |
 
     @smallest @todo
     Scenario: Summaries when routing on a simple network
@@ -74,6 +74,6 @@ Feature: Basic Routing
             | xey   | cross  |we need this because phantom node segments are not considered for the summary |
 
         When I route I should get
-            | from | to | route                     | summary      |
+            | from | to | route                     | summary       |
-            | a    | 1  | first,first,second,second | first,second |
+            | a    | 1  | first,first,second,second | first, second |
 

include/engine/api/route_parameters.hpp

@@ -69,6 +69,21 @@ struct RouteParameters : public BaseParameters
 
     RouteParameters() = default;
 
+    template <typename... Args>
+    RouteParameters(const bool steps_,
+                    const bool alternatives_,
+                    const GeometriesType geometries_,
+                    const OverviewType overview_,
+                    const boost::optional<bool> continue_straight_,
+                    Args... args_)
+        : BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
+          annotations{false}, geometries{geometries_}, overview{overview_},
+          continue_straight{continue_straight_}
+    // Once we perfectly-forward `args` (see #2990) this constructor can delegate to the one below.
+    {
+    }
+
+    // RouteParameters constructor adding the `annotations` setting in a API-compatible way.
     template <typename... Args>
     RouteParameters(const bool steps_,
                     const bool alternatives_,

include/engine/engine.hpp

@@ -55,7 +55,7 @@ class Engine final
     // Needs to be public
     struct EngineLock;
 
-    explicit Engine(EngineConfig &config);
+    explicit Engine(const EngineConfig &config);
 
     Engine(Engine &&) noexcept;
     Engine &operator=(Engine &&) noexcept;
@@ -63,12 +63,12 @@ class Engine final
     // Impl. in cpp since for unique_ptr of incomplete types
     ~Engine();
 
-    Status Route(const api::RouteParameters &parameters, util::json::Object &result);
+    Status Route(const api::RouteParameters &parameters, util::json::Object &result) const;
-    Status Table(const api::TableParameters &parameters, util::json::Object &result);
+    Status Table(const api::TableParameters &parameters, util::json::Object &result) const;
-    Status Nearest(const api::NearestParameters &parameters, util::json::Object &result);
+    Status Nearest(const api::NearestParameters &parameters, util::json::Object &result) const;
-    Status Trip(const api::TripParameters &parameters, util::json::Object &result);
+    Status Trip(const api::TripParameters &parameters, util::json::Object &result) const;
-    Status Match(const api::MatchParameters &parameters, util::json::Object &result);
+    Status Match(const api::MatchParameters &parameters, util::json::Object &result) const;
-    Status Tile(const api::TileParameters &parameters, std::string &result);
+    Status Tile(const api::TileParameters &parameters, std::string &result) const;
 
   private:
     std::unique_ptr<EngineLock> lock;

include/engine/guidance/assemble_leg.hpp

@@ -198,7 +198,7 @@ inline RouteLeg assembleLeg(const datafacade::BaseDataFacade &facade,
 
         const auto summary_names = summary_array | boost::adaptors::transformed(name_id_to_string) |
                                    boost::adaptors::filtered(not_empty);
-        summary = boost::algorithm::join(summary_names, ",");
+        summary = boost::algorithm::join(summary_names, ", ");
     }
 
     return RouteLeg{duration, distance, summary, {}};

include/osrm/osrm.hpp

@@ -83,7 +83,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, RouteParameters and json::Object
      */
-    Status Route(const RouteParameters &parameters, json::Object &result);
+    Status Route(const RouteParameters &parameters, json::Object &result) const;
 
     /**
      * Distance tables for coordinates.
@@ -92,7 +92,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, TableParameters and json::Object
      */
-    Status Table(const TableParameters &parameters, json::Object &result);
+    Status Table(const TableParameters &parameters, json::Object &result) const;
 
     /**
      * Nearest street segment for coordinate.
@@ -101,7 +101,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, NearestParameters and json::Object
      */
-    Status Nearest(const NearestParameters &parameters, json::Object &result);
+    Status Nearest(const NearestParameters &parameters, json::Object &result) const;
 
     /**
      * Trip: shortest round trip between coordinates.
@@ -110,7 +110,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, TripParameters and json::Object
      */
-    Status Trip(const TripParameters &parameters, json::Object &result);
+    Status Trip(const TripParameters &parameters, json::Object &result) const;
 
     /**
      * Match: snaps noisy coordinate traces to the road network
@@ -119,7 +119,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, MatchParameters and json::Object
      */
-    Status Match(const MatchParameters &parameters, json::Object &result);
+    Status Match(const MatchParameters &parameters, json::Object &result) const;
 
     /**
      * Tile: vector tiles with internal graph representation
@@ -128,7 +128,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, TileParameters and json::Object
      */
-    Status Tile(const TileParameters &parameters, std::string &result);
+    Status Tile(const TileParameters &parameters, std::string &result) const;
 
   private:
     std::unique_ptr<engine::Engine> engine_;

src/engine/engine.cpp

@@ -124,7 +124,7 @@ namespace osrm
 namespace engine
 {
 
-Engine::Engine(EngineConfig &config)
+Engine::Engine(const EngineConfig &config)
 {
     if (config.use_shared_memory)
     {
@@ -157,32 +157,32 @@ Engine::~Engine() = default;
 Engine::Engine(Engine &&) noexcept = default;
 Engine &Engine::operator=(Engine &&) noexcept = default;
 
-Status Engine::Route(const api::RouteParameters &params, util::json::Object &result)
+Status Engine::Route(const api::RouteParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *route_plugin, result);
 }
 
-Status Engine::Table(const api::TableParameters &params, util::json::Object &result)
+Status Engine::Table(const api::TableParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *table_plugin, result);
 }
 
-Status Engine::Nearest(const api::NearestParameters &params, util::json::Object &result)
+Status Engine::Nearest(const api::NearestParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *nearest_plugin, result);
 }
 
-Status Engine::Trip(const api::TripParameters &params, util::json::Object &result)
+Status Engine::Trip(const api::TripParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *trip_plugin, result);
 }
 
-Status Engine::Match(const api::MatchParameters &params, util::json::Object &result)
+Status Engine::Match(const api::MatchParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *match_plugin, result);
 }
 
-Status Engine::Tile(const api::TileParameters &params, std::string &result)
+Status Engine::Tile(const api::TileParameters &params, std::string &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *tile_plugin, result);
 }

src/osrm/osrm.cpp

@@ -21,32 +21,32 @@ OSRM &OSRM::operator=(OSRM &&) noexcept = default;
 
 // Forward to implementation
 
-engine::Status OSRM::Route(const engine::api::RouteParameters &params, util::json::Object &result)
+engine::Status OSRM::Route(const engine::api::RouteParameters &params, util::json::Object &result) const
 {
     return engine_->Route(params, result);
 }
 
-engine::Status OSRM::Table(const engine::api::TableParameters &params, json::Object &result)
+engine::Status OSRM::Table(const engine::api::TableParameters &params, json::Object &result) const
 {
     return engine_->Table(params, result);
 }
 
-engine::Status OSRM::Nearest(const engine::api::NearestParameters &params, json::Object &result)
+engine::Status OSRM::Nearest(const engine::api::NearestParameters &params, json::Object &result) const
 {
     return engine_->Nearest(params, result);
 }
 
-engine::Status OSRM::Trip(const engine::api::TripParameters &params, json::Object &result)
+engine::Status OSRM::Trip(const engine::api::TripParameters &params, json::Object &result) const
 {
     return engine_->Trip(params, result);
 }
 
-engine::Status OSRM::Match(const engine::api::MatchParameters &params, json::Object &result)
+engine::Status OSRM::Match(const engine::api::MatchParameters &params, json::Object &result) const
 {
     return engine_->Match(params, result);
 }
 
-engine::Status OSRM::Tile(const engine::api::TileParameters &params, std::string &result)
+engine::Status OSRM::Tile(const engine::api::TileParameters &params, std::string &result) const
 {
     return engine_->Tile(params, result);
 }