OSGeo / PROJ-JNI

Java Native Interface for PROJ
https://osgeo.github.io/PROJ-JNI/
MIT License
23 stars 15 forks source link

Added search path for proj.db to Context.create(). #70

Closed ggmuelle closed 1 year ago

ggmuelle commented 1 year ago

This is useful if you can't set environment variable PROJ_LIB.

desruisseaux commented 1 year ago

Thanks for the patch. The addition of search path seems fine. But should we really include the PROJ headers and binaries in the GitHub repository? Versionning binary files together with source codes is not recommended. Especially for executables: why only Windows binaries and not the other platforms? Why this specific PROJ version? Are we going to push new binaries at every PROJ releases? Who will do the maintenance work of keeping those binaries and headers up-to-date?

Would it be possible to modify this pull request so that it addresses only the search path issue, without adding binaries? Instead of binaries, I would suggest to try to improve the INSTALL guide. If nevertheless there is a wish from the community to add binaries, then I think it should be a separated discussion not be be taken lightly.

ggmuelle commented 1 year ago

Sorry, adding the includes and libraries should be only for internal use. I thought only the search-path-stuff will be part of the pull request because I've created it before adding the includes and libraries. I'll try to recreate the PR.

desruisseaux commented 1 year ago

Thanks! No need to recreate a new merge request. A commit for removing the binaries followed by squashing with git rebase -i followed by a git push --force will work well.

ggmuelle commented 1 year ago

The PR is clean now.

ggmuelle commented 1 year ago

Done.

desruisseaux commented 1 year ago

I added a commit with the addition of documentation and for mutl-directories support. I took the initiative to rename the proj.data property as org.osgeo.proj.data for consistency with the existing org.osgeo.proj.maxThreadsPerInstance property (javadoc). Please let me know if this is an issue.

ggmuelle commented 1 year ago

I'm fine with that.

desruisseaux commented 1 year ago

For the record, a similar functionality was provided in an independent fork here.