enable internal lerc for gdal build #100
Conversation
|
huh that seemed to work... @vincentsarago @sgillies is there any concern using GDAL's internal LERC vs building it conventionally? |
|
@AndrewAnnex this doesn't quite work. GDAL is configured to use the external, system libtiff, so that it can be shared with PROJ (which also uses libtiff now to read grid shift data). So that build directive comes too late. BTW, I didn't realize actions were open to first-time contributors, so I've changed the settings. Builds take a long time and we need to keep the queue short. Our current workflow can't support multiple people making builds at the same time. |
|
@sgillies apologies for the build resources used, okay so if I understand it all correctly I'd add a build_lerc step to config.sh and have that run inside build_tiff (somewhere in lines 154-158)? |
|
@sgillies looks like a similar config is needed for cirrus-ci, I killed those jobs just in case. I've updated the build script to build lerc from source but I am confused about the comment about libtiff as I can see libtiff being compiled within this project. Are you saying that ultimately that the system level libtiff shared library must be compiled with lerc? |
I'm the one who approved the run, sorry. |
|
No worries @AndrewAnnex, @vincentsarago ! I'm just at a loss at how to enable PRs that add features and drivers, but that don't run the entire platform matrix/matrices while we're debugging. If you have any ideas, lemme know! |
|
@sgillies looks like you can do an auto-cancellation based on some environment variables they provide: https://cirrus-ci.org/guide/writing-tasks/#environment-variables CIRRUS_USER_COLLABORATOR and CIRRUS_REPO_OWNER look like promising ways to filter out builds |
|
@sgillies just wanted to check about this pr and if you've had a chance to update the cirrus ci filters. I haven't looked into the github actions to see what else is being run on your infrastructure but we should be able to create a separate job just for contributor pull requests. |
| function build_tiff { | ||
| if [ -e tiff-stamp ]; then return; fi | ||
| build_lerc |
There was a problem hiding this comment.
we may have to add line like https://github.com/lambgeo/docker-lambda/blob/master/dockerfiles/Dockerfile.gdal3.5#L144-L145
There was a problem hiding this comment.
@vincentsarago yes, something like that. And of course, build_lerc would need to be written. I actually looked into this over a year ago, found that Lerc wasn't packaged properly, and punted.
There was a problem hiding this comment.
@sgillies I added build_lerc (lines 152-167), looks like LERC fixed their issues with their packaging since then so I think it should be fine
There was a problem hiding this comment.
okay added those two additional lines @vincentsarago, let me know how that looks. Also should I just download the tar gz to figure out what configuration options libtiff has?
|
@AndrewAnnex yes, I'm going to do something about it later this week. |
|
okay @sgillies sounds good, hopefully just aborting the runs in cirrus-ci is enough for now, if not sorry for the two additional runs! |
|
This OSGeo/gdal#7306 is interesting. I'm learning a bunch. |
|
@sgillies lost track of this pr, have you had a chance yet to adjust the permissions on github/cirrus actions? |
|
@AndrewAnnex I haven't made any progress on opening this up to PRs, which makes me unhappy 😞 But I do like what I see here and will try merging it into the branch I'm building the 1.3.7 wheels on. |
|
@sgillies guess this pr can be closed then? |
|
@AndrewAnnex I try to update the credits file for every .x release, and you'll be in there! Thanks for moving this ahead. |
possible fix for #70 and #94, although could be convinced to not use lerc internal to gdal (untested). I don't build gdal by hand too often (perhaps once a long time ago) so not sure if it's really this easy or not