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

Propagate a PR from GDAL to GDAL.jl to ArchGDAL.jl for testing #133

Closed
mathieu17g opened this issue Jan 21, 2022 · 4 comments
Closed

Propagate a PR from GDAL to GDAL.jl to ArchGDAL.jl for testing #133

mathieu17g opened this issue Jan 21, 2022 · 4 comments

Comments

@mathieu17g
Copy link
Contributor

@visr considering OSGeo/gdal/issues/5115, I have started by setting up my environnement to be able to make a PR on GDAL. Now I should be ready. Before starting I would like to know how to propagate it to an ArchGDAL PR. I guess that I have to:

  1. build a new gdal_jll from my dev branch on GDAL with BinaryBuilder.jl
  2. put it as GDAL_branch_name in Yggdrasil/tree/master/G/GDAL, if several version can coexist there, in Yggdrasil/tree/master/G with another name otherwise
  3. make a PR on GDAL.jl based on this jll with the adhoc modifications to expose ogr_f_stealgeometryex
  4. make a PR on ArchGDAL.jl using the new gdal_jll and GDAL.jl function
  5. wait for a new release of GDAL and its transposition in GDAL.jl

Could you please correct my guess ?

@visr
Copy link
Member

visr commented Jan 21, 2022

So far there has only been one GDAL_jll line, which has followed the GDAL releases, with only a few patches to make the cross compilation work for all those platforms.

What would be the most simple is that we wait until a GDAL release that includes your new feature, and then do a new patch. I don't think we should do (2) in either form. If your GDAL patch is accepted and we know it will be in the next feature release and cannot wait for it, we could consider backporting the patch to the last release and include it as a patch in https://github.com/JuliaPackaging/Yggdrasil/tree/master/G/GDAL/bundled/patches.

Or are you just looking for a custom GDAL_jll for your own testing? Because it is also possible to build a custom JLL locally, see https://docs.binarybuilder.org/stable/building/#Building-a-custom-JLL-package-locally.

But indeed once a new GDAL_jll lands, and it contains a new feature that we will wrap in GDAL.jl and use in ArchGDAL.jl, then we need to require that version and tag new releases for those packages. For bugfix GDAL_jll builds, there is nothing that we need to do, since Pkg will install it for us automatically.

@mathieu17g
Copy link
Contributor Author

Thanks a lot I will look into building a custom JLL package locally

@visr
Copy link
Member

visr commented Jan 21, 2022

It looks like instead of deploying locally you can also deploy to your own repository. Then we can point GDAL.jl to that repo with a Manifest.toml in a branch for testing.

@visr
Copy link
Member

visr commented Jun 2, 2022

@mathieu17g look at that, after #136 a shiny new function appeared :)

GDAL.jl/src/libgdal.jl

Lines 25166 to 25189 in 68d8c3f

"""
OGR_F_StealGeometryEx(OGRFeatureH hFeat,
int iGeomField) -> OGRGeometryH
Take away ownership of geometry.
### Parameters
* **hFeat**: feature from which to steal a geometry.
* **iGeomField**: index of the geometry field to steal.
### Returns
the pointer to the stolen geometry.
"""
function ogr_f_stealgeometryex(arg1, iGeomField)
aftercare(
ccall(
(:OGR_F_StealGeometryEx, libgdal),
OGRGeometryH,
(OGRFeatureH, Cint),
arg1,
iGeomField,
),
)
end

Available in GDAL.jl 1.4

@visr visr closed this as completed Jun 2, 2022
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

No branches or pull requests

2 participants