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 features for hdf5, netcdf, postgresql #21231

Merged
merged 19 commits into from
Jan 3, 2022

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Nov 6, 2021

  • What does your PR fix?

    Adds feature control for hdf5, netcdf, postgresql.
    Unblocks builds for arm64-windows and uwp. (However, not successful in CI.) (UWP not supported due to comsupp lib.)
    Fixes finding expat debug lib on mingw.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    all, yes

  • Does your PR follow the maintainer guide?

    yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    yes

@dg0yt dg0yt force-pushed the gdal-wip branch 2 times, most recently from 0fa580c to 5a8885d Compare November 6, 2021 18:59
@dg0yt dg0yt force-pushed the gdal-wip branch 3 times, most recently from c5bf7d5 to 6cc0a7e Compare November 7, 2021 17:17
@PhoebeHui PhoebeHui added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Nov 16, 2021
@longhuan2018
Copy link
Contributor

Can you add a SPATIALITE_412_OR_LATER macro definition to libspatialite?
dependency_win.cmake

  if ("libspatialite" IN_LIST FEATURES)
    ........
    set(HAVE_SPATIALITE "-DHAVE_SPATIALITE -DSPATIALITE_412_OR_LATER")
  endif()

The description of SPATIALITE_412_OR_LATER is in the nmake.opt file

# Uncomment following line if spatialite is 4.1.2 or later
#SPATIALITE_412_OR_LATER=yes

@dg0yt
Copy link
Contributor Author

dg0yt commented Nov 17, 2021

Can you add a SPATIALITE_412_OR_LATER macro definition to libspatialite?

Thanks for the hint!
I will update the PR when the prerequisite PRs are merged. I get no response for #20480.

@dg0yt

This comment has been minimized.

@JackBoosY
Copy link
Contributor

Oh, libcrypto from openssl needs User32.lib now?

libcrypto.lib(cryptlib.obj) : error LNK2019: unresolved external symbol __imp_GetProcessWindowStation referenced in function OPENSSL_isservice
libcrypto.lib(cryptlib.obj) : error LNK2019: unresolved external symbol __imp_GetUserObjectInformationW referenced in function OPENSSL_isservice
libcrypto.lib(cryptlib.obj) : error LNK2019: unresolved external symbol __imp_MessageBoxW referenced in function OPENSSL_showfatal
spatialite.dll : fatal error LNK1120: 3 unresolved externals

I remember openssl's cmake wrapper doing this stuff. But why this regression happened?

@dg0yt
Copy link
Contributor Author

dg0yt commented Dec 7, 2021

I remember openssl's cmake wrapper doing this stuff. But why this regression happened?

There are two other libs, but not user32.lib. I think this is okay for cmake because of the implicit link libraries which do contain user32.lib (but not the other two libs).
The new issue is with a nmake build, probably the first which uses libcrypto transitively (via proj and curl, this PR) basedf on pkgconfig data, for a library (libspatialite). That's why user32.lib isn't already present in the port' link libs.

@dg0yt
Copy link
Contributor Author

dg0yt commented Dec 7, 2021

Wait, I made that error note to the wrong PR initially, and forgot to hide it. Sorry for the noise. My comments apply to #20443.

@dg0yt
Copy link
Contributor Author

dg0yt commented Dec 8, 2021

Comment on lines 340 to 344
# Requires ATL for ARM64 to be installed in CI
gdal:arm64-windows=fail
# Requires comsuppw[d].lib for UWP to be installed in CI
gdal:arm-uwp=fail
gdal:x64-uwp=fail
Copy link
Contributor

Choose a reason for hiding this comment

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

@BillyONeal Should we add these Visual Studio Components on our CI?

Copy link
Member

Choose a reason for hiding this comment

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

I don't believe comsupp supports UWP at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dg0yt Can you double confirm that?
If gdal doesn't support arm or uwp, please move them to supports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't say much about gdal usage about uwp. I don't care if uwp is supported at all.
But gdal works well on all types of Android, including ARM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I really don't understand is how the MSVC toolchains manage to pull in libs which are not installed or supported for the given platform. Why not fail early?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ping @BillyONeal for reply.

Copy link
Member

Choose a reason for hiding this comment

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

@BillyONeal The open question was: Is ATLS.LIB for arm64-windows (not UWP!) missing intentionally, or doesn't it exist?

For desktop there is an ATL available for all the things:

image

Checking these boxes gives me:

"C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\atlmfc\lib\arm64\atls.lib"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it is missing in the CI image only. This PR handles this correctly by the CI baseline entry.

Copy link
Member

Choose a reason for hiding this comment

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

I think this means gdal:arm64-windows should be in ci.baseline.txt but there should be supports:!uwp since there seems to be no attempt by upstream to support that (if they need comsupp.lib and ATL)

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see you already did that

@dg0yt
Copy link
Contributor Author

dg0yt commented Dec 19, 2021

x64-linux error is unrelated:

CMake Error at /mnt/vcpkg-ci/installed/x64-linux/share/ffmpeg/FindFFMPEG.cmake:68 (find_library):
  Could not find FFMPEG_DEPENDENCY_asound_RELEASE using the following names:
  asound
Call Stack (most recent call first):
  /mnt/vcpkg-ci/installed/x64-linux/share/ffmpeg/FindFFMPEG.cmake:148 (append_dependencies)
  /mnt/vcpkg-ci/installed/x64-linux/share/ffmpeg/vcpkg-cmake-wrapper.cmake:6 (_find_package)
  /agent/_work/1/s/scripts/buildsystems/vcpkg.cmake:742 (include)
  modules/videoio/cmake/detect_ffmpeg.cmake:6 (find_package)
  modules/videoio/cmake/init.cmake:7 (include)
  modules/videoio/cmake/init.cmake:11 (add_backend)
  cmake/OpenCVModule.cmake:298 (include)
  cmake/OpenCVModule.cmake:361 (_add_modules_1)
  cmake/OpenCVModule.cmake:385 (ocv_glob_modules)
  CMakeLists.txt:917 (ocv_register_modules)

@JackBoosY
Copy link
Contributor

JackBoosY commented Dec 20, 2021

libasound is provided by port alsa.

usr@usr:/home/usr/work/vcpkg# ./vcpkg depend-info opencv4[ade,contrib,core,default-features,dnn,eigen,ffmpeg,gdcm,gtk,ipp,jasper,jpeg,lapack,nonfree,openexr,opengl,openmp,png,python,qt,quirc,sfm,tiff,vtk,webp]:x64-linux
zlib:
vcpkg-cmake-config:
vcpkg-cmake:
libiconv:
brotli:
bzip2:
libpng: vcpkg-cmake, vcpkg-cmake-config, zlib
pthreads:
libffi: vcpkg-cmake, vcpkg-cmake-config
json-c:
libuuid:
gettext: libiconv
pcre: vcpkg-cmake, vcpkg-cmake-config
freetype[png, bzip2, zlib, brotli]: brotli, bzip2, libpng, vcpkg-cmake, vcpkg-cmake-config, zlib
pthread: pthreads
expat:
vcpkg-tool-meson:
dirent:
lzo:
libjpeg-turbo:
liblzma: vcpkg-cmake, vcpkg-cmake-config
fontconfig: dirent, expat, freetype, gettext, json-c, libiconv, libuuid, pthread, vcpkg-tool-meson
glib: dirent, gettext, libffi, libiconv, pcre, vcpkg-tool-meson, zlib
openblas: pthread, vcpkg-cmake, vcpkg-cmake-config
szip: vcpkg-cmake, vcpkg-cmake-config
openssl: vcpkg-cmake, vcpkg-cmake-config
egl-registry:
pixman: libpng, vcpkg-tool-meson
opengl:
opengl-registry: egl-registry
openjpeg: vcpkg-cmake, vcpkg-cmake-config
lz4: vcpkg-cmake, vcpkg-cmake-config
libxml2: libiconv, liblzma, vcpkg-cmake, vcpkg-cmake-config, zlib
libwebp[simd, nearlossless]: vcpkg-cmake, vcpkg-cmake-config
libsass:
tiff[lzma, jpeg, zip]: libjpeg-turbo, liblzma, vcpkg-cmake, zlib
vcpkg-pkgconfig-get-modules:
libogg:
sqlite3[tool]: vcpkg-cmake, vcpkg-cmake-config
zstd: vcpkg-cmake, vcpkg-cmake-config
hdf5[szip, zlib]: szip, vcpkg-cmake, vcpkg-cmake-config, zlib
giflib: vcpkg-cmake
blas: openblas
gflags:
getopt:
cairo[x11, gobject, freetype, fontconfig]: dirent, expat, fontconfig, freetype, glib, libpng, lzo, pixman, pthread, vcpkg-tool-meson, zlib
harfbuzz: freetype, vcpkg-tool-meson
fribidi:
curl[openssl, ssl, non-http]: openssl, vcpkg-cmake, vcpkg-cmake-config, zlib
proj4[tiff, net]: curl, sqlite3, tiff, vcpkg-cmake, vcpkg-cmake-config
pegtl-2:
double-conversion:
freeglut:
pcre2: vcpkg-cmake, vcpkg-cmake-config
utfcpp:
pango: cairo, fontconfig, freetype, fribidi, gettext, glib, harfbuzz, vcpkg-tool-meson
gdk-pixbuf: gettext, glib, libpng, tiff, vcpkg-tool-meson, zlib
pugixml:
sassc: getopt, libsass
eigen3:
glew: opengl, vcpkg-cmake, vcpkg-cmake-config
netcdf-c[netcdf-4, nczarr, hdf5, platform-default-features, dap]: curl, hdf5, vcpkg-cmake, vcpkg-cmake-config, vcpkg-pkgconfig-get-modules
glog: gflags
graphene: gettext, glib, vcpkg-tool-meson
atk: gettext, glib, vcpkg-tool-meson
libtheora: libogg
icu:
angle: egl-registry, opengl-registry, zlib
libarchive[lzma, lz4, openssl, libxml2, zstd, bzip2]: bzip2, liblzma, libxml2, lz4, openssl, vcpkg-cmake, zlib, zstd
jsoncpp: vcpkg-cmake, vcpkg-cmake-config
libepoxy: vcpkg-tool-meson
lapack-reference[noblas, blas-select]: blas, vcpkg-cmake-config
leptonica: giflib, libjpeg-turbo, libpng, libwebp, openjpeg, tiff, vcpkg-cmake, vcpkg-cmake-config, zlib
libharu[notiffsymbols]: libpng, tiff, vcpkg-cmake, zlib
ade:
tesseract: leptonica, libarchive, vcpkg-cmake, vcpkg-cmake-config
ceres: eigen3, glog
vtk: double-conversion, eigen3, expat, freetype, glew, hdf5, jsoncpp, libharu, libjpeg-turbo, liblzma, libogg, libpng, libtheora, libxml2, lz4, netcdf-c, pegtl-2, proj4, pugixml, sqlite3, tiff, utfcpp, vcpkg-cmake, vcpkg-cmake-config, zlib
quirc:
qt5-base: angle, double-conversion, egl-registry, fontconfig, freetype, harfbuzz, icu, libjpeg-turbo, libpng, openssl, pcre2, sqlite3, vcpkg-pkgconfig-get-modules, zlib, zstd
ffmpeg[swscale, swresample, avdevice, avformat, avfilter, avcodec]: vcpkg-cmake, vcpkg-pkgconfig-get-modules
protobuf:
gdcm: expat, openjpeg, vcpkg-cmake, vcpkg-cmake-config, zlib
openexr: zlib
gtk: atk, cairo, gdk-pixbuf, gettext, glib, graphene, libepoxy, pango, sassc, vcpkg-tool-meson
jasper: freeglut, libjpeg-turbo, opengl, vcpkg-cmake, vcpkg-cmake-config
lapack: lapack-reference
opencv4[webp, sfm, quirc, python, qt, png, opengl, openexr, openmp, lapack, jpeg, vtk, jasper, ipp, tiff, nonfree, gtk, ffmpeg, eigen, dnn, default-features, contrib, gdcm, ade]: ade, blas, ceres, eigen3, ffmpeg, gdcm, gflags, glog, gtk, hdf5, jasper, lapack, libjpeg-turbo, libpng, libwebp, openexr, opengl, protobuf, qt5-base, quirc, tesseract, tiff, vcpkg-cmake, vcpkg-cmake-config, vtk, zlib

alsa is not in opencv4's dependency list. Even not in ffmpeg's dependency list.

@JackBoosY JackBoosY added depends:different-pr This PR or Issue depends on a PR which has been filed and removed depends:different-pr This PR or Issue depends on a PR which has been filed labels Dec 20, 2021
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dg0yt
Copy link
Contributor Author

dg0yt commented Dec 31, 2021

Please merge this soon. The next GDAL release is waiting.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Dec 31, 2021
@BillyONeal BillyONeal merged commit cccfe83 into microsoft:master Jan 3, 2022
@BillyONeal
Copy link
Member

Thanks for the update!

@dg0yt dg0yt deleted the gdal-wip branch January 4, 2022 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants