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

[GDAL] Add zstd and webp drivers #1941

Merged
merged 10 commits into from
Oct 20, 2020
Merged

Conversation

evetion
Copy link
Contributor

@evetion evetion commented Oct 19, 2020

Instead of upgrading, let's broaden some drivers. Had to enable -lstdc++, not sure if it's needed on other platforms than linux.

@evetion
Copy link
Contributor Author

evetion commented Oct 19, 2020

More food for JuliaGeo/GDAL.jl#65. Failing builds are due to hash mismatches or failed downloads.

@evetion
Copy link
Contributor Author

evetion commented Oct 19, 2020

I believe only FreeBSD is still failing. Something something libstdc++.so.6 not found.

@@ -35,6 +35,7 @@ elif [[ "${target}" == *-linux-* ]]; then
fi
autoreconf -vi
fi
export LDFLAGS="$LDFLAGS -lstdc++"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look like a good idea, especially for macOS and FreeBSD. Why did you need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking. All linux builds failed without it, but it seems I moved it slightly to high, will move it into the linux check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see we are already using it for other libraries when building for linux platforms, this is a bit borked

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this caused FreeBSD to fail, now works :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we don't use GCC for macOS and FreeBSD by default, hence no libstdc++

@evetion
Copy link
Contributor Author

evetion commented Oct 19, 2020

Btw, thanks for keeping a very close eye on the failing builds, restarting where necessary. 🥇

@giordano
Copy link
Member

Good to go?

@evetion
Copy link
Contributor Author

evetion commented Oct 19, 2020

I'd say yes! @visr?

@visr
Copy link
Contributor

visr commented Oct 19, 2020

Great, thanks for working on this! I just opened a few random logs like this and see what we want to see:

ZSTD support: yes
WebP support: yes

So this is good to go. @evetion would be great if you could update https://github.com/JuliaGeo/GDAL.jl/blob/ac27d0f35569a560abd813f217d90beaaf1a5f76/test/drivers.jl correspondingly after this is merged, such that we can test regressions in the build.

@giordano
Copy link
Member

Maybe it'd be good to add checks similar to these:

# Make sure that some important libraries are found
grep "HAVE_GEOS='yes'" config.log
grep "HAVE_SQLITE='yes'" config.log
grep "CURL_SETTING='yes'" config.log

@evetion
Copy link
Contributor Author

evetion commented Oct 19, 2020

Maybe it'd be good to add checks similar to these:

Done. And, for good measure, also tried to add a Postgres driver. Let's see how that works out.

@evetion
Copy link
Contributor Author

evetion commented Oct 19, 2020

So this is good to go. @evetion would be great if you could update https://github.com/JuliaGeo/GDAL.jl/blob/ac27d0f35569a560abd813f217d90beaaf1a5f76/test/drivers.jl correspondingly after this is merged, such that we can test regressions in the build.

Will do after this is merged. 👍

@evetion
Copy link
Contributor Author

evetion commented Oct 20, 2020

Ok, it seems PostgreSQL is not as easy as the other drivers, I'll do that in another PR. It seems the builds failed with it failing to find the header files, such as here. Sorry for the noise.

@evetion
Copy link
Contributor Author

evetion commented Oct 20, 2020

@giordano This is ready to merge, only the CI needs a little nudge.

@giordano giordano merged commit bb96d1b into JuliaPackaging:master Oct 20, 2020
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