search

rasterio / rasterio-wheels

MIT License
11 stars 16 forks source link

enable internal lerc for gdal build #100

Closed AndrewAnnex closed 1 year ago

AndrewAnnex commented 1 year ago

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

AndrewAnnex commented 1 year ago

huh that seemed to work... @vincentsarago @sgillies is there any concern using GDAL's internal LERC vs building it conventionally?

sgillies commented 1 year ago

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

AndrewAnnex commented 1 year ago

@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)?

AndrewAnnex commented 1 year ago

@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?

vincentsarago commented 1 year ago

I didn't realize actions were open to first-time contributors

I'm the one who approved the run, sorry.

sgillies commented 1 year ago

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!

AndrewAnnex commented 1 year ago

@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

AndrewAnnex commented 1 year ago

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

sgillies commented 1 year ago

@AndrewAnnex yes, I'm going to do something about it later this week.

AndrewAnnex commented 1 year ago

okay @sgillies sounds good, hopefully just aborting the runs in cirrus-ci is enough for now, if not sorry for the two additional runs!

sgillies commented 1 year ago

This https://github.com/OSGeo/gdal/issues/7306 is interesting. I'm learning a bunch.

AndrewAnnex commented 1 year ago

@sgillies lost track of this pr, have you had a chance yet to adjust the permissions on github/cirrus actions?

sgillies commented 1 year ago

@AndrewAnnex I haven't made any progress on opening this up to PRs, which makes me unhappy :disappointed: 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.

AndrewAnnex commented 1 year ago

@sgillies guess this pr can be closed then? idk if there is a contributor list to add me to but I'd appreciate that. I see my name on the list, nvm!

sgillies commented 1 year ago

@AndrewAnnex I try to update the credits file for every .x release, and you'll be in there! Thanks for moving this ahead.