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

use PROJ_jll for binary #33

Merged
merged 4 commits into from
Mar 7, 2020
Merged

use PROJ_jll for binary #33

merged 4 commits into from
Mar 7, 2020

Conversation

visr
Copy link
Member

@visr visr commented Dec 5, 2019

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

@c42f
Copy link
Member

c42f commented Dec 9, 2019

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.

@visr
Copy link
Member Author

visr commented Dec 9, 2019

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.
@visr visr marked this pull request as ready for review March 1, 2020 12:17
@visr
Copy link
Member Author

visr commented Mar 1, 2020

I updated the artifacts with the latest proj-datumgrid packages that were released today.
PROJ 7 was also released today, see the release notes. Since it provides a new way to interact with the gridded models, it's probably best to refrain from working too much on the old system now.

PROJ 7 still supports the old proj_api.h API, but will be the last to do so.

I think it's best to just merge this as is. Anyone still wants to review?

@visr visr merged commit 63aa9ad into master Mar 7, 2020
@visr visr deleted the artifact branch March 7, 2020 20:36
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
Copy link
Member

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.

Copy link
Member Author

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).

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.

3 participants