Improve build errors reporting #3579
Conversation
scripts/valhalla_build_timezones
Outdated
| echo "$1" 1>&2 | ||
| #exit 1 | ||
| echo "error: $1" 1>&2 | ||
| exit 1 |
There was a problem hiding this comment.
This comment seems to be committed accidentally.
There was a problem hiding this comment.
maybe we can detect the geos version and only exit 1 if it was a newer one?
for example pkg-config geos --modversion returns 3.9.1 on my system and it fails because of double free but the output sqlite is perfectly fine
There was a problem hiding this comment.
interestingly on github 3.9 doesnt even exist as a tag... so perhaps thats a sign ;)
There was a problem hiding this comment.
in a month we'll have a new ubuntu version so maybe this will be rectified then..
| @@ -1,18 +1,16 @@ | |||
| #!/bin/sh | |||
|
|
|||
| error_exit() { | |||
| echo "$1" 1>&2 | |||
| #exit 1 | |||
| echo "error: $1" 1>&2 | |||
There was a problem hiding this comment.
Add "error" to emphasize that this is a critical error (and to highlight it where applicable)
There was a problem hiding this comment.
The reason this is commented is because of recent versions of libgeos double freeing some memory. In others words, on some platforms this breaks the build. The double free happens at the end of the operation so it's harmless but erroring caused everything else to fail
| @@ -114,7 +114,8 @@ set_target_properties(logging PROPERTIES COMPILE_DEFINITIONS LOGGING_LEVEL_ALL) | |||
|
|
|||
| add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/test/data/tz.sqlite | |||
| COMMAND ${CMAKE_COMMAND} -E make_directory test/data/ | |||
| COMMAND ${VALHALLA_SOURCE_DIR}/scripts/valhalla_build_timezones 2>/dev/null > ${CMAKE_BINARY_DIR}/test/data/tz.sqlite | |||
| COMMAND ${VALHALLA_SOURCE_DIR}/scripts/valhalla_build_timezones > ${CMAKE_BINARY_DIR}/test/data/tz.sqlite.tmp | |||
| @@ -114,7 +114,8 @@ set_target_properties(logging PROPERTIES COMPILE_DEFINITIONS LOGGING_LEVEL_ALL) | |||
|
|
|||
| add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/test/data/tz.sqlite | |||
| COMMAND ${CMAKE_COMMAND} -E make_directory test/data/ | |||
| COMMAND ${VALHALLA_SOURCE_DIR}/scripts/valhalla_build_timezones 2>/dev/null > ${CMAKE_BINARY_DIR}/test/data/tz.sqlite | |||
| COMMAND ${VALHALLA_SOURCE_DIR}/scripts/valhalla_build_timezones > ${CMAKE_BINARY_DIR}/test/data/tz.sqlite.tmp | |||
| COMMAND ${CMAKE_COMMAND} -E rename ${CMAKE_BINARY_DIR}/test/data/tz.sqlite.tmp ${CMAKE_BINARY_DIR}/test/data/tz.sqlite | |||
There was a problem hiding this comment.
Create output file only in case of success. Otherwise subsequent builds will consider tz.sqlite fresh and won't attempt to recreate it
Co-authored-by: Kevin Kreiser <kevinkreiser@gmail.com>
Issue
Improve build errors reporting
Tasklist