Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update timezone database #4446

Merged
merged 13 commits into from
Dec 14, 2023
Merged

update timezone database #4446

merged 13 commits into from
Dec 14, 2023

Conversation

nilsnolde
Copy link
Member

@nilsnolde nilsnolde commented Dec 11, 2023

fixes #3817

  • removes most of the date_time folder (except windowsZones.xml) and submodules the tz repo, which needed a bit of CMake stuff to make it work; now we know the version of the IANA data as well
  • added some code to try and prevent failures on future tz upgrades (new timezones or renamed/merged)
  • full compatibility with existing code/new tiles and new code/old tiles
  • add docs for future tz updates

Also we could use the OS timezone DBs now. I left the windowsZones.xml in here as well.

Other than that, the code itself is pretty well documented.

@nilsnolde nilsnolde force-pushed the nn-timezone-update branch 2 times, most recently from dbe14b9 to e18463d Compare December 11, 2023 17:21
@@ -1,433 +0,0 @@
"ID","STD ABBR","STD NAME","DST ABBR","DST NAME","GMT offset","DST adjustment","DST Start Date rule","Start time","DST End date rule","End time"
Copy link
Member Author

@nilsnolde nilsnolde Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this was for boost::date_time which was long abandoned

gknisely
gknisely previously approved these changes Dec 12, 2023
WORKING_DIRECTORY ${VALHALLA_SOURCE_DIR}
COMMENT "Compiling date_time/windowsZones.xml to date_time_windows_zones.h"
DEPENDS ${VALHALLA_SOURCE_DIR}/date_time/windowsZones.xml)

file(GLOB headers ${VALHALLA_SOURCE_DIR}/valhalla/baldr/*.h)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huch, this was definitely not intended..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry @gknisely that dismissed your review

gknisely
gknisely previously approved these changes Dec 12, 2023

echo "downloading timezone polygon file." 1>&2
curl -L -s -o ./timezones-with-oceans.shapefile.zip ${url} || error_exit "curl failed for ${url}"
unzip ./timezones-with-oceans.shapefile.zip 1>&2 || error_exit "unzip failed"

tz_file=$(mktemp)
spatialite_tool -i -shp ./dist/combined-shapefile-with-oceans -d ${tz_file} -t tz_world -s 4326 -g geom -c UTF-8 1>&2 || error_exit "spatialite_tool import failed"
spatialite_tool -i -shp "./combined-shapefile-with-oceans" -d "${tz_file}" -t tz_world -s 4326 -g geom -c UTF-8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove the error checking, surely its still possible something goes wrong

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh, just a fail, will add again

@@ -24,6 +24,7 @@ sqlite3* GetDBHandle(const std::string& database) {
sqlite3_close(db_handle);
return nullptr;
}
// sqlite3_extended_result_codes(db_handle, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, let me uncomment. it will give better error codes/msgs

kevinkreiser
kevinkreiser previously approved these changes Dec 13, 2023
Copy link
Member

@kevinkreiser kevinkreiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, two tiny quesitons though i do think we should error check in the script

@nilsnolde nilsnolde dismissed stale reviews from kevinkreiser and gknisely via 35a0069 December 13, 2023 14:37
@nilsnolde
Copy link
Member Author

fixed the small comments: 35a0069

@nilsnolde nilsnolde merged commit 29cb1be into master Dec 14, 2023
6 of 7 checks passed
@nilsnolde nilsnolde deleted the nn-timezone-update branch December 14, 2023 13:38
nilsnolde added a commit that referenced this pull request Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TZDB shapefile is out of date
3 participants