Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
07c9a9d
Peform detection and warning, both on the output side
afarber Aug 30, 2025
2c9ba8c
Add check_buffer_for_locations_on_ways
afarber Aug 30, 2025
75957ad
Make methods const, fix build errors
afarber Aug 30, 2025
fb5af6d
Move from setup_header() to warn_about_locations_on_ways()
afarber Aug 30, 2025
3d00dc8
Add public getters and cat cmd unit tests
afarber Aug 30, 2025
cb950e3
Add format param to the unit tests
afarber Aug 30, 2025
51071e2
Modify the sort cmd
afarber Aug 30, 2025
7dfa762
Add parametrized unit tests for sort
afarber Aug 30, 2025
8fe2800
Add integration tests for cat
afarber Aug 30, 2025
00f056b
Add integration tests for sort
afarber Aug 30, 2025
aa7974b
Remove warn_about_locations_on_ways()
afarber Aug 30, 2025
743c15f
Sort alphabetically and add 2 entries
afarber Aug 30, 2025
bb98b63
Check the PBF header only
afarber Aug 30, 2025
ca613a8
Add cat tests
afarber Aug 30, 2025
ea40f08
Add sort tests, use build dir for temp files
afarber Aug 30, 2025
86c39d0
Add no-warning tests
afarber Aug 30, 2025
e0fed63
Implement for add-locations-to-ways cmd
afarber Aug 31, 2025
f01c2ac
Add test/add-locations-to-ways
afarber Aug 31, 2025
4e5658c
Move warning from cmds to io.cpp
afarber Sep 3, 2025
9ab08b6
Revert file
afarber Sep 3, 2025
8bef8f9
Add has_locations_on_ways
afarber Sep 4, 2025
4fcc526
Change tests
afarber Sep 4, 2025
f00d5ac
Temp reduce CI builds
afarber Sep 9, 2025
eea021c
Add virtual method
afarber Sep 9, 2025
0fdae6b
Revert temp changes
afarber Sep 9, 2025
3efe9ae
Fix uninit shared ptr
afarber Sep 9, 2025
1980208
Remove input_begin, input_end
afarber Sep 27, 2025
5d06e86
Revert input_begin, input_end for gcc 15
afarber Sep 27, 2025
dab577e
Change tests to use multiple files
afarber Sep 27, 2025
bde61f6
Rename multipass tests to _mp
afarber Sep 27, 2025
29c8c64
Use const auto
afarber Sep 27, 2025
fc519fd
Remove input_begin, input_end again
afarber Sep 27, 2025
30c7ea4
Add warn_locations_on_ways_lost
afarber Sep 27, 2025
c14f867
Fix the call
afarber Sep 27, 2025
4cbf180
Suppress false positive GCC 15 warning
afarber Sep 27, 2025
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
27 changes: 27 additions & 0 deletions cmake/run_test_check_no_stderr.cmake
Original file line number Diff line number Diff line change
@@ -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")
31 changes: 31 additions & 0 deletions cmake/run_test_check_stderr.cmake
Original file line number Diff line number Diff line change
@@ -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")
9 changes: 9 additions & 0 deletions src/cmd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ along with this program. If not, see <https://www.gnu.org/licenses/>.
#include "cmd.hpp"

#include "exception.hpp"
#include "util.hpp"

#include <osmium/index/map.hpp>
#include <osmium/osm/entity_bits.hpp>
Expand Down Expand Up @@ -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<osmium::io::File>& 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.");
}
}
6 changes: 6 additions & 0 deletions src/cmd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -262,6 +266,8 @@ class with_osm_output {
return m_output_overwrite;
}

void warn_locations_on_ways_lost(const std::vector<osmium::io::File>& input_files, const Command& command) const;

}; // class with_osm_output


Expand Down
10 changes: 10 additions & 0 deletions src/command_apply_changes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -357,12 +359,20 @@ bool CommandApplyChanges::run() {
const auto input = osmium::io::make_input_iterator_range<osmium::OSMObject>(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
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/command_apply_changes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 2 additions & 0 deletions src/command_cat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/command_cat.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions src/command_sort.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<osmium::memory::Buffer> data;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/command_sort.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
21 changes: 21 additions & 0 deletions src/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ along with this program. If not, see <https://www.gnu.org/licenses/>.
#include "exception.hpp"

#include <osmium/io/file.hpp>
#include <osmium/io/reader.hpp>
#include <osmium/osm/area.hpp>
#include <osmium/tags/tags_filter.hpp>
#include <osmium/util/file.hpp>
Expand Down Expand Up @@ -276,3 +277,23 @@ double show_gbytes(std::size_t value) noexcept {
return static_cast<double>(show_mbytes(value)) / 1000; // NOLINT(bugprone-integer-division)
}

bool has_locations_on_ways(const std::vector<osmium::io::File>& 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;
}

1 change: 1 addition & 0 deletions src/util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<osmium::io::File>& input_files);

#endif // UTIL_HPP
2 changes: 1 addition & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
90 changes: 90 additions & 0 deletions test/locations-on-ways/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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 "$<TARGET_FILE:osmium> 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 "$<TARGET_FILE:osmium> 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 "$<TARGET_FILE:osmium> 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 "$<TARGET_FILE:osmium> 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 "$<TARGET_FILE:osmium> 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 "$<TARGET_FILE:osmium> 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)

#-----------------------------------------------------------------------------
Binary file added test/locations-on-ways/input-with-locations.osm.pbf
Binary file not shown.
Binary file not shown.