Conversation
JackBoosY
added
category:port-bug
|
cc @dg0yt |
| # @PROJECT_NAME_UPPER@_LIBRARIES | ||
|
|
||
| +cmake_policy(PUSH) | ||
| +cmake_policy(SET CMP0012 NEW) |
There was a problem hiding this comment.
CMake Warning (dev) at F:/vcpkg/installed/x64-windows/share/GeoTIFF/geotiff-config.cmake:26 (if):
if given arguments:
"ON"
An argument named "ON" appears in a conditional statement. Policy CMP0012
is not set: if() recognizes numbers and boolean constants. Run "cmake
--help-policy CMP0012" for policy details. Use the cmake_policy command to
set the policy and suppress this warning.
Call Stack (most recent call first):
F:/vcpkg/scripts/buildsystems/vcpkg.cmake:824 (_find_package)
CMakeLists.txt:251 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
You have modified or added at least one vcpkg.json where you should check the license field.
Details
If you feel able to do so, please consider adding a "license" field to the following files:
ports/liblas/vcpkg.json
Valid values for the license field can be found in the documentation
There was a problem hiding this comment.
You have modified or added at least one vcpkg.json where you should check the license field.
Details
If you feel able to do so, please consider adding a "license" field to the following files:
ports/liblas/vcpkg.json
Valid values for the license field can be found in the documentation
This issue is unrelated to these ports. Which problem are you really solving? The geotiff patch was already upstream but not release yet. |
Building vtk[all]:
But apparently, there are still problems with geotiff, see my comment above. |
Where does the |
Comes from liblas, that's why I also fixed liblas in this PR. |
So this doesn't explain why you revert the upstreamed libgeotiff patch. |
LilyWangLL
added
the
info:needs-maintainer-attention
JackBoosY
dismissed stale reviews from LilyWangLL and github-actions[bot]
via
75f6d32
|
I will do an independent review of port liblas. |
JackBoosY
removed
the
info:needs-maintainer-attention
| set(PROJ_LIBRARIES PROJ::proj) | ||
| - string(APPEND CONFIG_DEPENDENCIES " find_dependency(PROJ CONFIG)\n") | ||
| + string(APPEND CONFIG_PRIVATE_DEPENDENCIES " find_dependency(PROJ CONFIG)\n") | ||
| + string(APPEND CONFIG_PUBLIC_DEPENDENCIES " find_dependency(PROJ CONFIG)\n") |
There was a problem hiding this comment.
I still think this change is wrong/not needed. It is a private dependency. It is not part of INTERFACE_LINK_LIBRARIES.
However, CMake starts to export a new property IMPORTED_LINK_DEPENDENT_LIBRARIES. If this is going to need find_dependency, you will have a lot of more ports to update.
There was a problem hiding this comment.
Anyway, this port requires to link with proj: #25636 (comment)
There was a problem hiding this comment.
This port does link with proj. That's how proj.dll comes into the list in your comment.
The question is if downstream consumers are forced to explicitly link to proj, too (public dependency).
There was a problem hiding this comment.
No ideas with this. Maybe we also should fix them.
There was a problem hiding this comment.
You have to show what is actually broken. If it ain't broken, don't fix it.
There was a problem hiding this comment.
In geotiff-depends-debug.cmake:
set_target_properties(geotiff_library PROPERTIES
IMPORTED_IMPLIB_DEBUG "${_IMPORT_PREFIX}/debug/lib/geotiff_d_i.lib"
IMPORTED_LINK_DEPENDENT_LIBRARIES_DEBUG "PROJ::proj"
IMPORTED_LINK_INTERFACE_LIBRARIES_DEBUG "TIFF::TIFF"
IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/debug/bin/geotiff_d.dll"
)Doc: https://cmake.org/cmake/help/latest/prop_tgt/IMPORTED_LINK_DEPENDENT_LIBRARIES.html
There was a problem hiding this comment.
Yes. That's what I'm saying: PROJ::proj is not in INTERFACE_LINK_LIBRARIES. Either we don't need to deal with this property (because we don't deal with rpath), or you will have to fix another 200 ports. AFAIU, CMake "just ignores entries of IMPORTED_LINK_DEPENDENT_LIBRARIES that cannot be found", cf. https://discourse.cmake.org/t/use-cases-for-imported-link-dependent-libraries/5662.
| # @PROJECT_NAME_UPPER@_LIBRARIES | ||
|
|
||
| +cmake_policy(PUSH) | ||
| +cmake_policy(SET CMP0012 NEW) |
There was a problem hiding this comment.
I often add the policy in wrappers, but I'm not sure if this should be extended to CMake config. Ports which use CMake config hardly run with such low cmake_minimum_required.
There was a problem hiding this comment.
I think this is a upstream bug, so it's not appropriate to place it in a wrapper.
There was a problem hiding this comment.
I didn't mean that it is to be placed in a wrapper for libgeotiff.
I mean that we should not add patches for config consumers which don't do at least cmake_minimum_required(VERSION 2.8). It is not an upstream bug if they don't care for stone-age consumers.
(And liblas had the extra issue that cmake_minimum_required appeared after project.)
|
Closing this PR since it seems that no progress is being made. Please ping me to reopen if work is still being done. |
3 participants
Related: #24740