[osgearth] Add draco feature#13423
Conversation
NancyLi1013
added
category:port-feature
…NancyLi/add-feature-for-osgearth # Conflicts: # ports/osgearth/CONTROL
…NancyLi/add-feature-for-osgearth # Conflicts: # ports/osgearth/CONTROL # ports/osgearth/portfile.cmake
….com/NancyLi1013/vcpkg into dev/NancyLi/add-feature-for-osgearth # Conflicts: # ports/osgearth/portfile.cmake
Remove feature test
….com/NancyLi1013/vcpkg into dev/NancyLi/add-feature-for-osgearth # Conflicts: # ports/osgearth/CONTROL
NancyLi1013
changed the title
JackBoosY
added
the
info:needs-maintainer-attention
| find_package(GEOS) | ||
| find_package(Sqlite3) | ||
| -find_package(Draco) | ||
| find_package(BASISU) | ||
| find_package(GLEW) | ||
| find_package(Protobuf) |
There was a problem hiding this comment.
It seems like these should also be listed as dependencies of the port?
There was a problem hiding this comment.
These ports are optional dependencies of the port. So I didn't add them to the dependency lists.
There was a problem hiding this comment.
There was a problem hiding this comment.
They're optional, but it makes this port behave totally differently depending on what other ports are selected. That is, it makes
vcpkg install osgearth
vcpkg install sqlite
and
vcpkg install sqlite
vcpkg install osgearth
produce completely different results. Generally we need to either provide all such optional components or patch them to always be off.
I think your fix here addresses the Draco dependency, but the other dependencies here are just as likely to create the same problems.
There was a problem hiding this comment.
Yes, it would be better to add options for every optional dependency. We can add these ports to the dependency lists or make patches to fix them now. We can consider to ask for help from upstream to make optional dependencies more flexible for users.
BillyONeal
added
requires:author-response
and removed
info:needs-maintainer-attention
| + include(SelectLibraryConfigurations) | ||
| + | ||
| + find_path(DRACO_INCLUDE_DIR draco/core/draco_version.h) | ||
| + find_library(DRACO_LIBRARY_DEBUG NAMES draco dracodec dracoenc libdraco libdracodec libdracoenc NAMES_PER_DIR PATH_SUFFIXES lib PATHS "${_VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/debug" NO_DEFAULT_PATH REQUIRED) |
There was a problem hiding this comment.
Why isn't the draco port exposing the right bits here?
There was a problem hiding this comment.
Sorry, I didn't make sense. Could you please help describe more details?
There was a problem hiding this comment.
I mean, there should probably be a share/draco/dracoConfig.cmake or similar which does this find_library stuff for all consumers instead of only osgearth.
There was a problem hiding this comment.
There was a problem hiding this comment.
Understand now. Thanks for your clarification.
There was a problem hiding this comment.
I tried just making it REQUIRED and that seems to have passed over in #15777 ?
…t less "flaky", and turn on in CI. Add static-crt to platform-expression. Related: microsoft#13423
|
Closing this PR Temporarily. |
3 participants
Describe the pull request
Add
dracoforosgearth. Sincegdalis required dependency ofosgearth3.1, which has been included inosg[plugins].So, it's unnecessary to add
gdalas a feature.Note: Feature has passed with x64-windows.
osgearthonly supports dynamic build.