Skip to content

Commit b81a6c2

Browse files
nilsnoldenilsnolde
authored
move argparse boilerplate code to private header (valhalla#4169)
* refactor valhalla_add_elevation a bit and move the mjolnir util to baldr * add -j flag to all multithreaded executables * changelog * remove deprecated heder * oops * moved the argparse boilerplate code to a private header which all programs can share * cxxopts entry not needed in cmake * add missing midgard header * some fixes * huch, quite a logical mistake.. * few nits * more fixes * remove left-overs * keep deduplication in the executable valhalla_add_elevation * keep deduplication in the executable valhalla_add_elevation * use __FILE__ to find the executable name; overhaul src/CMakeLists.txt, we need to put the ./src directory to target_include_directories() * move get_tile_ids function into the executable * add filesystem::stem() to return the file name without extension and adapt all executables
1 parent d6d2d48 commit b81a6c2

28 files changed

+578
-958
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
* FIXED: typo in use value of map matching API (`platform_connection` was misspelled) [#4174](https://github.com/valhalla/valhalla/pull/4174)
77
* **Enhancement**
88
* UPDATED: French translations, thanks to @xlqian [#4159](https://github.com/valhalla/valhalla/pull/4159)
9+
* CHANGED: -j flag for multithreaded executables to override mjolnir.concurrency [#4168](https://github.com/valhalla/valhalla/pull/4168)
10+
* CHANGED: moved the argparse boilerplate code to a private header which all programs can share [#4169](https://github.com/valhalla/valhalla/pull/4169)
911
* ADDED: CI runs a spell check on the PR to detect spelling mistakes [#4179](https://github.com/valhalla/valhalla/pull/4179)
1012
* ADDED: `preferred_side_cutoff` parameter for locations [#4182](https://github.com/valhalla/valhalla/pull/4182)
1113
* ADDED: PBF output for matrix endpoint [#4121](https://github.com/valhalla/valhalla/pull/4121)

‎src/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ endif()
229229
target_include_directories(valhalla
230230
PUBLIC
231231
${VALHALLA_SOURCE_DIR}
232+
${VALHALLA_SOURCE_DIR}/src # For private headers in /src
232233
${VALHALLA_SOURCE_DIR}/valhalla # TODO: this path must be removed and changed to #include <valhalla/...> in headers
233234
${VALHALLA_BINARY_DIR}
234235
${VALHALLA_BINARY_DIR}/valhalla # TODO: this path must be removed and changed to #include <valhalla/...> in headers

‎src/argparse_utils.h

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
#include <boost/property_tree/ptree.hpp>
2+
#include <cxxopts.hpp>
3+
4+
#include <valhalla/baldr/rapidjson_utils.h>
5+
#include <valhalla/config.h>
6+
#include <valhalla/filesystem.h>
7+
#include <valhalla/midgard/logging.h>
8+
#include <valhalla/midgard/util.h>
9+
10+
/**
11+
* Parses common command line arguments across executables. It
12+
* - alters the config ptree and sets the concurrency config, where it favors the command line arg,
13+
* then falls back to the config and finally to all threads
14+
* - sets the logging configuration
15+
*
16+
* @param program The executable's name
17+
* @param opts The command line options
18+
* @param result The parsed result
19+
* @param pt The config which will be populated here
20+
* @param log The logging config node's key
21+
* @param use_threads Whether this program multi-threads
22+
*
23+
* @returns true if the program should continue, false if we should EXIT_SUCCESS
24+
* @throws cxxopts::OptionException Thrown if there's no valid configuration
25+
*/
26+
bool parse_common_args(const std::string& program,
27+
const cxxopts::Options& opts,
28+
const cxxopts::ParseResult& result,
29+
boost::property_tree::ptree& pt,
30+
const std::string& log,
31+
const bool use_threads = false) {
32+
if (result.count("help")) {
33+
std::cout << opts.help() << "\n";
34+
return false;
35+
}
36+
37+
if (result.count("version")) {
38+
std::cout << std::string(program) << " " << VALHALLA_VERSION << "\n";
39+
return false;
40+
}
41+
42+
// Read the config file
43+
if (result.count("inline-config")) {
44+
std::stringstream ss;
45+
ss << result["inline-config"].as<std::string>();
46+
rapidjson::read_json(ss, pt);
47+
} else if (result.count("config") &&
48+
filesystem::is_regular_file(result["config"].as<std::string>())) {
49+
rapidjson::read_json(result["config"].as<std::string>(), pt);
50+
} else {
51+
throw cxxopts::OptionException("Configuration is required\n\n" + opts.help() + "\n\n");
52+
}
53+
54+
// configure logging
55+
auto logging_subtree = pt.get_child_optional(log);
56+
if (logging_subtree) {
57+
auto logging_config =
58+
valhalla::midgard::ToMap<const boost::property_tree::ptree&,
59+
std::unordered_map<std::string, std::string>>(logging_subtree.get());
60+
valhalla::midgard::logging::Configure(logging_config);
61+
}
62+
63+
if (use_threads) {
64+
// override concurrency config if specified as arg
65+
auto num_threads = std::max(1U, result.count("concurrency")
66+
? result["concurrency"].as<uint32_t>()
67+
: pt.get<uint32_t>("mjolnir.concurrency",
68+
std::thread::hardware_concurrency()));
69+
pt.put<uint32_t>("mjolnir.concurrency", num_threads);
70+
71+
LOG_INFO("Running " + std::string(program) + " with " + std::to_string(num_threads) +
72+
" thread(s).");
73+
}
74+
75+
return true;
76+
}

‎src/mjolnir/elevationbuilder.cc

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <boost/format.hpp>
88

99
#include "baldr/graphconstants.h"
10+
#include "baldr/graphid.h"
1011
#include "baldr/graphreader.h"
1112
#include "filesystem.h"
1213
#include "midgard/logging.h"

‎src/mjolnir/valhalla_add_elevation.cc

+76-77
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,51 @@
66
#include <boost/property_tree/ptree.hpp>
77
#include <cxxopts.hpp>
88

9+
#include "baldr/graphid.h"
910
#include "baldr/graphreader.h"
1011
#include "baldr/graphtile.h"
1112
#include "baldr/rapidjson_utils.h"
13+
#include "config.h"
1214
#include "mjolnir/elevationbuilder.h"
13-
#include "mjolnir/valhalla_add_elevation_utils.h"
15+
16+
#include "argparse_utils.h"
1417

1518
namespace opt = cxxopts;
1619

1720
using namespace valhalla::baldr;
1821
using namespace valhalla::midgard;
1922
using namespace valhalla::mjolnir;
2023

21-
enum Input { CONFIG, TILES };
24+
namespace {
25+
std::deque<GraphId> get_tile_ids(const boost::property_tree::ptree& pt,
26+
const std::unordered_set<std::string>& tiles) {
27+
if (tiles.empty())
28+
return {};
29+
30+
auto tile_dir = pt.get_optional<std::string>("mjolnir.tile_dir");
31+
if (!tile_dir || !filesystem::exists(*tile_dir)) {
32+
LOG_WARN("Tile storage directory does not exist");
33+
return {};
34+
}
35+
36+
std::unordered_set<std::string> tiles_set{tiles.begin(), tiles.end()};
37+
38+
std::deque<GraphId> tilequeue;
39+
GraphReader reader(pt.get_child("mjolnir"));
40+
std::for_each(std::begin(tiles), std::end(tiles), [&](const auto& tile) {
41+
auto tile_id = GraphTile::GetTileId(*tile_dir + tile);
42+
GraphId local_tile_id(tile_id.tileid(), tile_id.level(), tile_id.id());
43+
if (!reader.DoesTileExist(local_tile_id)) {
44+
LOG_WARN("Provided tile doesn't belong to the tile directory from config file");
45+
return;
46+
}
47+
48+
tilequeue.push_back(tile_id);
49+
});
50+
51+
return tilequeue;
52+
}
53+
} // namespace
2254

2355
/*
2456
* This tool downloads elevations from remote storage for each provided tile.
@@ -30,95 +62,62 @@ enum Input { CONFIG, TILES };
3062
* - tiles - stays for the path to a tile file.
3163
* */
3264

33-
std::pair<std::string, std::vector<std::string>> parse_arguments(int argc, char** argv) {
34-
opt::Options options(
35-
"help",
36-
" Usage: valhalla_add_elevation [options]\n"
37-
"valhalla_add_elevation is a tool for loading elevations for a provided tile. "
38-
"The service checks if required elevations stored locally if they are not "
39-
"it tries to establish connection to the remote storage(based on the information from configuration file)"
40-
"and loads required elevations.\n");
41-
42-
options.add_options()("h,help",
43-
"Print usage")("c,config", "Path to the configuration file.",
44-
opt::value<
45-
std::string>())("t,tiles", "Tiles to add elevations to",
46-
opt::value<std::vector<std::string>>());
47-
48-
options.allow_unrecognised_options();
49-
std::string config_file;
65+
int main(int argc, char** argv) {
66+
const auto program = filesystem::path(__FILE__).stem().string();
67+
// args
68+
boost::property_tree::ptree config;
5069
std::vector<std::string> tiles;
51-
try {
52-
auto result = options.parse(argc, argv);
53-
54-
if (result.count("help")) {
55-
std::cout << options.help() << "\n";
56-
return {};
57-
}
5870

59-
if (!result.count("config")) {
60-
std::cerr << "No configuration file provided"
61-
<< "\n\n";
62-
} else {
63-
config_file = result["config"].as<std::string>();
64-
}
71+
try {
72+
// clang-format off
73+
opt::Options options(
74+
program,
75+
std::string(program) + " " + VALHALLA_VERSION + "\n\n"
76+
"a tool for loading elevations for a provided tile. "
77+
"The service checks if required elevations stored locally if they are not "
78+
"it tries to establish connection to the remote storage (based on the information from configuration file)"
79+
"and loads required elevations.\n");
80+
81+
options.add_options()
82+
("h,help", "Print usage")
83+
("v,version", "Print the version of this software.")
84+
("c,config", "Path to the configuration file.", opt::value<std::string>())
85+
("i,inline-config", "Inline JSON config", cxxopts::value<std::string>())
86+
("t,tiles", "Tiles to add elevations to", opt::value<std::vector<std::string>>(tiles))
87+
("j,concurrency", "Number of threads to use. Defaults to all threads.", opt::value<uint32_t>());
88+
// clang-format on
89+
90+
const auto result = options.parse(argc, argv);
91+
if (!parse_common_args(program, options, result, config, "mjolnir.logging", true))
92+
return EXIT_SUCCESS;
6593

6694
if (!result.count("tiles")) {
67-
std::cerr << "No tile file provided"
68-
<< "\n\n";
95+
std::cerr << "Tile file is required\n\n" << options.help() << "\n\n";
96+
return EXIT_FAILURE;
6997
} else {
70-
tiles = result["tiles"].as<std::vector<std::string>>();
98+
for (const auto& tile : result["concurrency"].as<std::vector<std::string>>()) {
99+
if (filesystem::exists(tile) && filesystem::is_regular_file(tile))
100+
return true;
101+
}
102+
std::cerr << "All tile files are invalid\n\n" << options.help() << "\n\n";
103+
return EXIT_FAILURE;
71104
}
72-
} catch (std::exception& e) {
73-
std::cerr << "Unable to parse command line options. Error: " << e.what() << "\n";
74-
return {};
75-
}
76-
77-
return {config_file, tiles};
78-
}
79-
80-
std::unordered_set<std::string> get_valid_tile_paths(std::vector<std::string>&& tiles) {
81-
std::unordered_set<std::string> st;
82-
for (const auto& tile : tiles) {
83-
if (filesystem::exists(tile) && filesystem::is_regular_file(tile))
84-
st.insert(tile);
85-
}
86-
87-
return st;
88-
}
89-
90-
int main(int argc, char** argv) {
91-
auto params = parse_arguments(argc, argv);
92-
if (std::get<Input::CONFIG>(params).empty() && std::get<Input::TILES>(params).empty()) {
93-
return EXIT_SUCCESS;
94-
}
95-
96-
if (std::get<Input::CONFIG>(params).empty() || std::get<Input::TILES>(params).empty()) {
97-
std::cerr << "Invalid input: " << (std::get<Input::CONFIG>(params).empty() ? "config" : "tile")
98-
<< " was not provided\n\n";
105+
} catch (cxxopts::OptionException& e) {
106+
std::cerr << e.what() << std::endl;
99107
return EXIT_FAILURE;
100-
}
101-
102-
if (!filesystem::exists(std::get<Input::CONFIG>(params)) ||
103-
!filesystem::is_regular_file(std::get<Input::CONFIG>(params))) {
104-
std::cerr << "Fail to parse configuration file\n\n";
105-
return EXIT_FAILURE;
106-
}
107-
108-
auto tiles = get_valid_tile_paths(std::move(std::get<Input::TILES>(params)));
109-
if (tiles.empty()) {
110-
std::cerr << "All tile files are invalid\n\n";
108+
} catch (std::exception& e) {
109+
std::cerr << "Unable to parse command line options because: " << e.what() << "\n"
110+
<< "This is a bug, please report it at " PACKAGE_BUGREPORT << "\n";
111111
return EXIT_FAILURE;
112112
}
113113

114-
boost::property_tree::ptree pt;
115-
rapidjson::read_json(std::get<Input::CONFIG>(params), pt);
116-
auto tile_ids = valhalla::mjolnir::get_tile_ids(pt, tiles);
114+
// pass the deduplicated tiles
115+
auto tile_ids = get_tile_ids(config, std::unordered_set<std::string>(tiles.begin(), tiles.end()));
117116
if (tile_ids.empty()) {
118117
std::cerr << "Failed to load tiles\n\n";
119118
return EXIT_FAILURE;
120119
}
121120

122-
ElevationBuilder::Build(pt, tile_ids);
121+
ElevationBuilder::Build(config, tile_ids);
123122
return EXIT_SUCCESS;
124123
}

0 commit comments

Comments
 (0)