diff --git a/cmake/run_test_check_no_stderr.cmake b/cmake/run_test_check_no_stderr.cmake new file mode 100644 index 00000000..660bd9bc --- /dev/null +++ b/cmake/run_test_check_no_stderr.cmake @@ -0,0 +1,27 @@ +# +# Test script to check that stderr does not contain a warning message +# + +if(NOT cmd) + message(FATAL_ERROR "Variable 'cmd' not defined") +endif() + +separate_arguments(cmd) + +execute_process( + COMMAND ${cmd} + RESULT_VARIABLE _return_code + OUTPUT_VARIABLE _stdout + ERROR_VARIABLE _stderr +) + +if(NOT _return_code EQUAL 0) + message(FATAL_ERROR "Command failed with return code ${_return_code}") +endif() + +string(FIND "${_stderr}" "WARNING:" _found_pos) +if(NOT _found_pos EQUAL -1) + message(FATAL_ERROR "Unexpected warning message found in stderr output: '${_stderr}'") +endif() + +message(STATUS "Test passed: No warning message found") \ No newline at end of file diff --git a/cmake/run_test_check_stderr.cmake b/cmake/run_test_check_stderr.cmake new file mode 100644 index 00000000..4448b3ed --- /dev/null +++ b/cmake/run_test_check_stderr.cmake @@ -0,0 +1,31 @@ +# +# Test script to check that stderr contains the expected warning message +# + +if(NOT cmd) + message(FATAL_ERROR "Variable 'cmd' not defined") +endif() + +if(NOT expected_stderr) + message(FATAL_ERROR "Variable 'expected_stderr' not defined") +endif() + +separate_arguments(cmd) + +execute_process( + COMMAND ${cmd} + RESULT_VARIABLE _return_code + OUTPUT_VARIABLE _stdout + ERROR_VARIABLE _stderr +) + +if(NOT _return_code EQUAL 0) + message(FATAL_ERROR "Command failed with return code ${_return_code}") +endif() + +string(FIND "${_stderr}" "${expected_stderr}" _found_pos) +if(_found_pos EQUAL -1) + message(FATAL_ERROR "Expected stderr message '${expected_stderr}' not found in stderr output: '${_stderr}'") +endif() + +message(STATUS "Test passed: Found expected stderr message") \ No newline at end of file diff --git a/src/cmd.cpp b/src/cmd.cpp index 7e232565..9c82a7c4 100644 --- a/src/cmd.cpp +++ b/src/cmd.cpp @@ -23,6 +23,7 @@ along with this program. If not, see . #include "cmd.hpp" #include "exception.hpp" +#include "util.hpp" #include #include @@ -177,3 +178,11 @@ std::string check_index_type(const std::string& index_type_name, bool allow_none return index_type_name; } + +void with_osm_output::warn_locations_on_ways_lost(const std::vector& input_files, const Command& command) const { + if (command.should_warn_locations_lost() && + has_locations_on_ways(input_files) && + m_output_format.find("locations_on_ways") == std::string::npos) { + warning("Input file contains locations on ways that will be lost in output. Use --output-format with locations_on_ways option to preserve node locations on ways."); + } +} diff --git a/src/cmd.hpp b/src/cmd.hpp index d2f574f1..59c2c749 100644 --- a/src/cmd.hpp +++ b/src/cmd.hpp @@ -118,6 +118,10 @@ class Command { void print_arguments(const std::string& command); void show_memory_used(); + virtual bool should_warn_locations_lost() const { + return false; + } + osmium::osm_entity_bits::type osm_entity_bits() const { return m_osm_entity_bits; } @@ -262,6 +266,8 @@ class with_osm_output { return m_output_overwrite; } + void warn_locations_on_ways_lost(const std::vector& input_files, const Command& command) const; + }; // class with_osm_output diff --git a/src/command_apply_changes.cpp b/src/command_apply_changes.cpp index 22d712dc..2d7043e1 100644 --- a/src/command_apply_changes.cpp +++ b/src/command_apply_changes.cpp @@ -279,6 +279,8 @@ bool CommandApplyChanges::run() { m_output_file.set("locations_on_ways"); } + warn_locations_on_ways_lost({m_input_file}, *this); + m_vout << "Opening output file...\n"; osmium::io::Writer writer{m_output_file, m_output_overwrite, m_fsync}; @@ -357,12 +359,20 @@ bool CommandApplyChanges::run() { const auto input = osmium::io::make_input_iterator_range(reader); auto output_it = boost::make_function_output_iterator(copy_first_with_id(&writer)); + // Suppress false positive GCC 15 warning with boost::make_function_output_iterator +#if defined(__GNUC__) && (__GNUC__ >= 15) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif std::set_union(objects.begin(), objects.end(), input.begin(), input.end(), output_it, osmium::object_order_type_id_reverse_version()); +#if defined(__GNUC__) && (__GNUC__ >= 15) +#pragma GCC diagnostic pop +#endif } } diff --git a/src/command_apply_changes.hpp b/src/command_apply_changes.hpp index af2a4e16..3c06afd8 100644 --- a/src/command_apply_changes.hpp +++ b/src/command_apply_changes.hpp @@ -67,6 +67,10 @@ class CommandApplyChanges : public CommandWithSingleOSMInput, public with_osm_ou return "osmium apply-changes [OPTIONS] OSM-FILE OSM-CHANGE-FILE..."; } + bool should_warn_locations_lost() const override { + return true; + } + }; // class CommandApplyChanges diff --git a/src/command_cat.cpp b/src/command_cat.cpp index a4d627e7..f6e84f9e 100644 --- a/src/command_cat.cpp +++ b/src/command_cat.cpp @@ -154,6 +154,8 @@ void report_filename(osmium::VerboseOutput* vout, const osmium::io::File& file, } // anonymous namespace bool CommandCat::run() { + warn_locations_on_ways_lost(m_input_files, *this); + std::size_t bytes_written = 0; if (m_input_files.size() == 1) { // single input file diff --git a/src/command_cat.hpp b/src/command_cat.hpp index 8532daf9..5bb9d393 100644 --- a/src/command_cat.hpp +++ b/src/command_cat.hpp @@ -61,6 +61,10 @@ class CommandCat : public CommandWithMultipleOSMInputs, public with_osm_output { return "osmium cat [OPTIONS] OSM-FILE..."; } + bool should_warn_locations_lost() const override { + return true; + } + }; // class CommandCat #endif // COMMAND_CAT_HPP diff --git a/src/command_sort.cpp b/src/command_sort.cpp index 8ff3dc49..bbe8ee5a 100644 --- a/src/command_sort.cpp +++ b/src/command_sort.cpp @@ -104,6 +104,8 @@ void CommandSort::show_arguments() { } bool CommandSort::run_single_pass() { + warn_locations_on_ways_lost(m_input_files, *this); + osmium::io::Writer writer{m_output_file, m_output_overwrite, m_fsync}; std::vector data; @@ -170,6 +172,8 @@ bool CommandSort::run_single_pass() { } bool CommandSort::run_multi_pass() { + warn_locations_on_ways_lost(m_input_files, *this); + osmium::io::Writer writer{m_output_file, m_output_overwrite, m_fsync}; osmium::Box bounding_box; diff --git a/src/command_sort.hpp b/src/command_sort.hpp index 8c2ad804..6ecb3d21 100644 --- a/src/command_sort.hpp +++ b/src/command_sort.hpp @@ -56,6 +56,10 @@ class CommandSort : public CommandWithMultipleOSMInputs, public with_osm_output return "osmium sort [OPTIONS] OSM-FILE..."; } + bool should_warn_locations_lost() const override { + return true; + } + }; // class CommandSort diff --git a/src/util.cpp b/src/util.cpp index 1a3468af..e7899ad3 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -25,6 +25,7 @@ along with this program. If not, see . #include "exception.hpp" #include +#include #include #include #include @@ -276,3 +277,23 @@ double show_gbytes(std::size_t value) noexcept { return static_cast(show_mbytes(value)) / 1000; // NOLINT(bugprone-integer-division) } +bool has_locations_on_ways(const std::vector& input_files) { + for (const auto& file : input_files) { + try { + osmium::io::Reader reader{file, osmium::osm_entity_bits::nothing}; + const osmium::io::Header& header = reader.header(); + + for (const auto& option : header) { + if (option.first.find("pbf_optional_feature") != std::string::npos && + option.second == "LocationsOnWays") { + return true; + } + } + } catch (...) { + // Ignore files that can't be opened for header check + continue; + } + } + return false; +} + diff --git a/src/util.hpp b/src/util.hpp index 13c6140a..8bd9512b 100644 --- a/src/util.hpp +++ b/src/util.hpp @@ -53,5 +53,6 @@ const char* object_type_as_string(const osmium::OSMObject& object) noexcept; bool ends_with(const std::string& str, const std::string& suffix); std::size_t show_mbytes(std::size_t value) noexcept; double show_gbytes(std::size_t value) noexcept; +bool has_locations_on_ways(const std::vector& input_files); #endif // UTIL_HPP diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index f3eb779f..c6aba05d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -99,7 +99,7 @@ foreach(_cmd IN LISTS OSMIUM_COMMANDS) endif() endforeach() -foreach(_extra_tests formats help misc) +foreach(_extra_tests formats help locations-on-ways misc) message(STATUS "Adding tests in ${_extra_tests}") add_subdirectory(${_extra_tests}) endforeach() diff --git a/test/locations-on-ways/CMakeLists.txt b/test/locations-on-ways/CMakeLists.txt new file mode 100644 index 00000000..bd368656 --- /dev/null +++ b/test/locations-on-ways/CMakeLists.txt @@ -0,0 +1,90 @@ +# +# Test LocationsOnWays warning functionality across multiple commands +# + +function(check_warning_cat _name _format _expected_stderr) + set(_input_paths "") + foreach(_file ${ARGN}) + set(_input_paths "${_input_paths} ${CMAKE_SOURCE_DIR}/test/locations-on-ways/${_file}") + endforeach() + set(_cmd "$ cat --no-progress --generator=test${_input_paths} -f ${_format} -o ${CMAKE_BINARY_DIR}/test/locations-on-ways/warning-cat-${_name}.osm --overwrite") + add_test( + NAME "locations-on-ways-cat-${_name}" + COMMAND ${CMAKE_COMMAND} + -D cmd:FILEPATH=${_cmd} + -D expected_stderr:STRING=${_expected_stderr} + -P ${CMAKE_SOURCE_DIR}/cmake/run_test_check_stderr.cmake + ) +endfunction() + +function(check_no_warning_cat _name _format) + set(_input_paths "") + foreach(_file ${ARGN}) + set(_input_paths "${_input_paths} ${CMAKE_SOURCE_DIR}/test/locations-on-ways/${_file}") + endforeach() + set(_cmd "$ cat --no-progress --generator=test${_input_paths} -f ${_format} -o ${CMAKE_BINARY_DIR}/test/locations-on-ways/no-warning-cat-${_name}.osm --overwrite") + add_test( + NAME "locations-on-ways-cat-${_name}" + COMMAND ${CMAKE_COMMAND} + -D cmd:FILEPATH=${_cmd} + -P ${CMAKE_SOURCE_DIR}/cmake/run_test_check_no_stderr.cmake + ) +endfunction() + +function(check_warning_sort _name _format _expected_stderr) + set(_input_paths "") + foreach(_file ${ARGN}) + set(_input_paths "${_input_paths} ${CMAKE_SOURCE_DIR}/test/locations-on-ways/${_file}") + endforeach() + set(_cmd "$ sort --no-progress --generator=test${_input_paths} -f ${_format} -o ${CMAKE_BINARY_DIR}/test/locations-on-ways/warning-sort-${_name}.osm --overwrite") + add_test( + NAME "locations-on-ways-sort-${_name}" + COMMAND ${CMAKE_COMMAND} + -D cmd:FILEPATH=${_cmd} + -D expected_stderr:STRING=${_expected_stderr} + -P ${CMAKE_SOURCE_DIR}/cmake/run_test_check_stderr.cmake + ) + set(_cmd_mp "$ sort --no-progress --generator=test -s multipass${_input_paths} -f ${_format} -o ${CMAKE_BINARY_DIR}/test/locations-on-ways/warning-sort-${_name}_mp.osm --overwrite") + add_test( + NAME "locations-on-ways-sort-${_name}_mp" + COMMAND ${CMAKE_COMMAND} + -D cmd:FILEPATH=${_cmd_mp} + -D expected_stderr:STRING=${_expected_stderr} + -P ${CMAKE_SOURCE_DIR}/cmake/run_test_check_stderr.cmake + ) +endfunction() + +function(check_no_warning_sort _name _format) + set(_input_paths "") + foreach(_file ${ARGN}) + set(_input_paths "${_input_paths} ${CMAKE_SOURCE_DIR}/test/locations-on-ways/${_file}") + endforeach() + set(_cmd "$ sort --no-progress --generator=test${_input_paths} -f ${_format} -o ${CMAKE_BINARY_DIR}/test/locations-on-ways/no-warning-sort-${_name}.osm --overwrite") + add_test( + NAME "locations-on-ways-sort-${_name}" + COMMAND ${CMAKE_COMMAND} + -D cmd:FILEPATH=${_cmd} + -P ${CMAKE_SOURCE_DIR}/cmake/run_test_check_no_stderr.cmake + ) + set(_cmd_mp "$ sort --no-progress --generator=test -s multipass${_input_paths} -f ${_format} -o ${CMAKE_BINARY_DIR}/test/locations-on-ways/no-warning-sort-${_name}_mp.osm --overwrite") + add_test( + NAME "locations-on-ways-sort-${_name}_mp" + COMMAND ${CMAKE_COMMAND} + -D cmd:FILEPATH=${_cmd_mp} + -P ${CMAKE_SOURCE_DIR}/cmake/run_test_check_no_stderr.cmake + ) +endfunction() + +#----------------------------------------------------------------------------- + +# Test cases with files that HAVE locations on ways - should warn when output format doesn't preserve them +# Uses multiple files where one has locations on ways +check_warning_cat(warning-pbf-to-xml xml "WARNING: Input file contains locations on ways that will be lost in output." input-with-locations.osm.pbf input-without-locations.osm.pbf) +check_warning_sort(warning-pbf-to-xml xml "WARNING: Input file contains locations on ways that will be lost in output." input-with-locations.osm.pbf input-without-locations.osm.pbf) + +# Test cases with files that DON'T have locations on ways - should NOT warn +# Uses multiple files where none have locations on ways +check_no_warning_cat(no-warning-pbf-to-xml xml input-without-locations.osm.pbf input-without-locations.osm.pbf) +check_no_warning_sort(no-warning-pbf-to-xml xml input-without-locations.osm.pbf input-without-locations.osm.pbf) + +#----------------------------------------------------------------------------- \ No newline at end of file diff --git a/test/locations-on-ways/input-with-locations.osm.pbf b/test/locations-on-ways/input-with-locations.osm.pbf new file mode 100644 index 00000000..ffc72ac1 Binary files /dev/null and b/test/locations-on-ways/input-with-locations.osm.pbf differ diff --git a/test/locations-on-ways/input-without-locations.osm.pbf b/test/locations-on-ways/input-without-locations.osm.pbf new file mode 100644 index 00000000..5884b660 Binary files /dev/null and b/test/locations-on-ways/input-without-locations.osm.pbf differ