-
Notifications
You must be signed in to change notification settings - Fork 10
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
use PROJ_jll for binary #33
Conversation
Very cool. Note that since proj is (IIRC?) fairly minimal on the dependencies it should be possible to support older julia versions at the same time (if you had the need) once JuliaPackaging/BinaryBuilder.jl#533 has propagated into the generated jll's. |
PROJ now depends on SQLite, and the next release will probably rely on libtiff as well. It should probably be doable to support older julia releases at the same time, given some work. I don't personally have the need though, and unless somebody is willing to make it happen, I'd be happy to drop older versions, given my limited time. They will still work as is, and we can always still make bugfix releases for the older versions as well. I have no rush to merge this PR, for me it is mostly a stepping stone to finally getting a GDAL build with essential drivers that is not plagued by symlink issues for Windows users without administrator rights. Having that will also allow finally merging yeesian/ArchGDAL.jl#76, which is stuck behind a broken GDAL GEOS in Linux, another build issue. Note that PROJ_jll does not include the datumgrid artifact that was included by default before. I still want to make this a lazy artifact, that will download it when needed. Though by the time PROJ 7 hits, these large grid shift files can also be accessed remotely, if PROJ RFC 4 lands. |
Including the script that generates it. As the proj docs mention, the core "proj-datumgrid" is required. The rest is lazily downloaded as required. Still have to test it, and see how the lazy download will interact with setting the search paths.
I updated the artifacts with the latest proj-datumgrid packages that were released today. PROJ 7 still supports the old I think it's best to just merge this as is. Anyone still wants to review? |
export geod_direct, geod_inverse, geod_destination, geod_distance | ||
include("proj_geodesic.jl") # low-level C-facing functions (corresponding to src/geodesic.h) | ||
end | ||
const has_geodesic_support = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this?
If this is not going to change the value, we could delete this variable overall and also delete the references in the tests and in proj_functions.jl
If this is a wanted change, I could prepare a pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for having a look!
Correct, we don't need this internally anymore. At the same time, I don't know if people are using this, and it is fairly harmless to keep around for now.
I guess this can go once we move the package over from the old API to the new one, as in #34 (comment).
I added the PROJBuilder build recipe to Yggdrasil, this led to PROJ_jll. Trying it out here.
https://github.com/JuliaGeo/PROJBuilder
https://github.com/JuliaPackaging/Yggdrasil/tree/master/P/PROJ
https://julialang.org/blog/2019/11/artifacts
TODO
Integrate the artifacts with the rest of the package and tests it(Created an Artifacts.toml with all latest datumgrid releases that is downloaded on demand. Probably best to go the PROJ 7 route for further integration.)