feat(traffictile.h): Adds versioning checks#2484
Conversation
|
|
||
| #basic timezone stuff | ||
| list(APPEND sources | ||
| ${CMAKE_CURRENT_BINARY_DIR}/traffic_tile_version.h |
There was a problem hiding this comment.
should we use GENERATED_VERSION_HEADER here as well?
88fb984 to
f790796
Compare
|
The |
Allocates one of the spare fields in the TrafficTileHeader for versioning data, with the version being made up of the first four bytes of the MD5 checksum of baldr/traffictile.h
f790796 to
d60dac1
Compare
1898c94 to
9c23054
Compare
There was a problem hiding this comment.
Why not have one if block and use an ||
There was a problem hiding this comment.
Logged different messages while debugging, I just pushed a commit that collaps them 👍
385ea98 to
39b0fdd
Compare
|
Ok, I finally got all the platforms in CI passing after fixing the nvm setup in Osx and removing the pipes which upset Windows. cc @mandeepsandhu |
| echo 'nvm install v8.11.2' >> $BASH_ENV | ||
| echo 'nvm alias default v8.11.2' >> $BASH_ENV | ||
| source $BASH_ENV && nvm install v8.11.2 | ||
| source $BASH_ENV && nvm alias default v8.11.2 |
There was a problem hiding this comment.
CircleCi is supposed to source this itself, but on Osx it doesn't.
| # Builds a checksum of the traffictile header and store it in `traffic_tile_version.h` | ||
| set(GENERATED_VERSION_HEADER "${CMAKE_CURRENT_BINARY_DIR}/traffic_tile_version.h") | ||
| message("Generating traffictile version header") | ||
| configure_file(${VALHALLA_SOURCE_DIR}/valhalla/baldr/traffictile.h |
There was a problem hiding this comment.
@danpat suggested a way to evaluate this entirely in cmake, with the hack here to implicitly add a dependency on the file to re-run checksum on changes. My first attempt without this hack only generated the checksum on first run.
| install_macos_dependencies: | ||
| steps: | ||
| - run: brew install protobuf cmake ccache libtool boost-python libspatialite pkg-config luajit curl wget czmq lz4 spatialite-tools unzip | ||
| - run: brew install protobuf cmake ccache coreutils libtool boost-python libspatialite pkg-config luajit curl wget czmq lz4 spatialite-tools unzip |
There was a problem hiding this comment.
This change isn't needed now that the checksum is done in pure cmake
There was a problem hiding this comment.
I left it in since it was listed as a build requirement on osx in readme: https://github.com/valhalla/valhalla#building-from-source
But I can remove it, fix pushed.
* 'master' of github.com:valhalla/valhalla: Make tile building reproducible (#2480) Fix install-script for ubuntu 18.04 (#2306) (#2486) nit(git): Configures changelog to resolve conflicts with union strategy (#2489) feat(traffictile.h): Adds versioning checks (#2484) Fix dereferencing of end for std::lower_bound and possible UB (#2488) Minor fixes to tests (#2483) nit: Missing changelog entry (#2478) Safiefy protobuf (#2477)
Allocates one of the spare fields in the TrafficTileHeader
for versioning data, with the version being made up of
the first four bytes of the MD5 checksum of baldr/traffictile.h