add gdal/3.1.0 #1722
Merged
add gdal/3.1.0 #1722
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Failure in build 1 (
|
|
Few comments:
I've tried to use as much as possible external dependencies instead of internal ones, except:
I've also tried to fix a little bit nmake files because there are discrepancies between autotools and nmake files (some options can't be disabled in nmake files). Conflicts: there are conflicts in the dependency tree of CCI. For example, Choice for default options is not straightforward.
(this recipe was hard to create, a lot of things are very fragile in gdal build system) |
|
Some configurations of 'gdal/3.1.0' failed in build 2 (
|
|
Some configurations of 'gdal/3.1.0' failed in build 3 (
|
|
How to deal with that 😓 : |
|
Some configurations of 'gdal/3.1.0' failed in build 4 (
|
|
It probably makes more sense to not build GDAL executables in this recipe (rather in a dedicated recipe):
|
|
Could you clean up manually this condiguration please? CI fails to remove dirty source folder: Removing |
|
@danimtb seems like we have an internal problem here. |
1 task
|
I can replace https://github.com/OSGeo/gdal/archive/v3.1.0.tar.gz by https://github.com/OSGeo/gdal/releases/download/v3.1.0/gdal-3.1.0.tar.gz (doesn't contain |
SpaceIm
commented
May 27, 2020
| if self.options.threadsafe: | ||
| self.cpp_info.system_libs.append("pthread") | ||
| elif self.settings.os == "Windows": | ||
| self.cpp_info.system_libs.extend(["psapi", "ws2_32"]) |
There was a problem hiding this comment.
GDAL directly call functions defined in kernel32 if using win32 threads. Should I add this lib also? It's not required by MinGW Posix Threads I think.
There was a problem hiding this comment.
No, kernel32 is not required. I used to write code with WinAPI in past, including windows threads, kernel32 was not required.
There was a problem hiding this comment.
functions like CreateThread reside in kernel32.lib. it's usually included by default in Visual Studio, but for other compilers (MinGW, Clang, Intel, Borland, etc.) it might not be the case.
|
Some configurations of 'gdal/3.1.0' failed in build 5 (
|
|
All green in build 6 (
|
|
Great, it builds. Now, important question: #1722 (comment) I think that GDAL executables should have a dedicated recipe (too big in debug mode, and I don't see any reason to use debug executables). |
Only if they are distributed separated. We are trying to avoid it, because we had many problems with installer package, and now we have build context, which allows get them at same package. Otherwise, you can add an option to build or not the executables, with build context, you can different profiles if it's used as build requirement. |
|
Disk space is not an issue? I didn't check but it's probably something like 150 GB of executables in CCI for each (version, revision) |
|
Disk space is not a problem at all, also both VCPKG and Conda provide all together. |
|
Well ok, therefore this PR is ready for review. |
|
All green in build 9 (
|
|
All green in build 10 (
|
danimtb
previously approved these changes
Jun 10, 2020
|
I can add a small patch to prevent adding |
uilianries
approved these changes
Jun 10, 2020
SpaceIm
commented
Jun 10, 2020
| options = { | ||
| "shared": [True, False], | ||
| "fPIC": [True, False], | ||
| "simd_intrinsics": [None, "sse", "ssse3", "avx"], |
There was a problem hiding this comment.
Maybe I should use False instead of None for consistency like
|
All green in build 11 (
|
jgsogo
approved these changes
Jun 11, 2020
danimtb
approved these changes
Jun 11, 2020
@jgsogo, we are using parts of GDAL at Mira Geoscience in two of our products: There was no easy way to get the latest SDK of GDAL. Now there is Conan! Thank you @SpaceIm and all the contributors for your amazing work here! I have just started to explore Conan a couple of months ago as a better way to handle our third parties. |
|
This is a great work! Than you @SpaceIm )) |
|
Good catch @b1x We need to:
Could you open an issue? |
I will try it) |
|
For the PR, I already have something ready. |
|
For linux this patch, works for me, but unfortunately I don't have Windows |
|
Well there are several things:
|
|
should be fixed in #1884 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Specify library name and version: gdal/3.1.0
conan-center hook activated.
closes #1023