Closed autoantwort closed 1 year ago
DanBloomberg
commented
1 year ago I don't maintain the CMakeLists files, so I don't understand everything you are doing. It is not necessary to have webp or openjpeg libraries in order to build leptonica. And it may not be necessary to have pkgconfig (not sure). But your elegant version, while fixing transitive dependencies, seems to require these libraries.
autoantwort
commented
1 year ago It is not necessary to have webp or openjpeg libraries in order to build leptonica.
That is right. So if you don't want them you can simply pass -DLIBWEBP_SUPPORT=OFF. In general all this "we build with xy support if we detect xy" must be patched out by package managers like vcpkg or conan because we want reproducible builds and it should not depend on the environment.
And it may not be necessary to have pkgconfig (not sure).
Yes one could also use the find_package(xy CONFIG REQUIRED) variants, but I through pkgconfig was used with an intention.
Is there a specific person responsible for maintaining the CMakeLists?
DanBloomberg
commented
1 year ago This difference you mention:
we build with requirementsversus
we build with what we findis very interesting. I can understand that package managers want to require libraries, whereas individuals who download the leptonica may not want to have all the libraries. And, in fact, leptonica has been written to require internally only a small set of image i/o libraries (actually, none!). But we've recommended a minimum set of (tiff, jpg, png/zlib). You can get an idea of the range of options provided to the user of leptonica by looking at the names of source files ending with stub.c.
This was done primarily to lower the barriers for use. Today, the most intensive use of leptonica is in conjunction with tesseract, so the tesseract maintainers (who also maintain leptonica) should have some interesting views on this aspect of your PRs.
autoantwort
commented
1 year ago @zdenop any opinions about this? :)
zdenop
commented
1 year ago I plan to have a look at this - maybe during this weekend...
zdenop
commented
1 year ago >cmake -Bbuild.667 -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% -DCMAKE_PREFIX_PATH=%INSTALL_DIR% -DBUILD_PROG=OFF -DSW_BUILD=OFF -DBUILD_SHARED_LIBS=ON
-- Building for: Visual Studio 16 2019
-- Selecting Windows SDK version 10.0.22621.0 to target Windows 10.0.19045.
-- The C compiler identification is MSVC 19.29.30147.0
-- The CXX compiler identification is MSVC 19.29.30147.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found GIF: F:/win64/lib/gif.lib (found version "5.2.1")
-- Found JPEG: F:/win64/lib/jpeg.lib (found version "62")
-- Found ZLIB: F:/win64/lib/zlib.lib (found version "1.2.12")
-- Found PNG: F:/win64/lib/libpng16.lib (found version "1.6.37")
-- Found TIFF: F:/win64/lib/tiff.lib (found version "4.4.0")
CMake Error at C:/Program Files/CMake/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE)
Call Stack (most recent call first):
C:/Program Files/CMake/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
C:/Program Files/CMake/share/cmake-3.25/Modules/FindPkgConfig.cmake:99 (find_package_handle_standard_args)
CMakeLists.txt:88 (find_package)cmake build must be possible without 3rd party tools!
autoantwort
commented
1 year ago @zdenop PkgConfig is not a requirement anymore :)
DanBloomberg
commented
1 year ago I don't know why the recent changes in sarray1.c, bmpio.c, readfile.c, etc are included in this merge, since they're already committed. Perhaps it doesn't matter?
zdenop
commented
1 year ago JP2K library - there was a very good reason why we implemented the detection of JP2K as was.>git fetch upstream pull/667/head:pr667
>git checkout pr667
>cmake -Bbuild.666 -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% -DCMAKE_PREFIX_PATH=%INSTALL_DIR% -DBUILD_PROG=OFF -DSW_BUILD=OFF -DBUILD_SHARED_LIBS=ON
-- Selecting Windows SDK version 10.0.22621.0 to target Windows 10.0.19045.
CMAKE_BUILD_TYPE = Release
--
-- General configuration for Leptonica 1.84.0
-- --------------------------------------------------------
-- Build type: Release
-- Compiler: MSVC
-- C compiler options:
/DWIN32 /D_WINDOWS /W3
-- Linker options: /machine:x64
-- Install directory: F:/win64
--
-- Build with sw [SW_BUILD]: OFF
-- Build utility programs [BUILD_PROG]: OFF
-- Used ZLIB library: F:/win64/lib/zlib.lib
-- Used PNG library: F:/win64/lib/libpng16.lib;F:/win64/lib/zlib.lib
-- Used JPEG library: F:/win64/lib/jpeg.lib
-- Used JP2K library:
-- Used TIFF library: F:/win64/lib/tiff.lib
-- Used GIF library: F:/win64/lib/gif.lib
-- Used WEBP library: webpdecoder;webp;webpdemux;webpmux
-- --------------------------------------------------------
--
-- Configuring done
CMake Error at src/CMakeLists.txt:49 (target_link_libraries):
Target "leptonica" links to:
WebP::libwebpmux
but the target was not found. Possible reasons include:
* There is a typo in the target name.
* A find_package call is missing for an IMPORTED target.
* An ALIAS target is missing.
-- Generating done
CMake Generate step failed. Build files cannot be regenerated correctly.I also just built on linux from head, and got this error trying to find openjpeg:
CMake Warning at CMakeLists.txt:124 (find_package):
By not providing "FindOpenJPEG.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "OpenJPEG",
but CMake did not find one.
Could not find a package configuration file provided by "OpenJPEG" with any
of the following names:
OpenJPEGConfig.cmake
openjpeg-config.cmake
Add the installation prefix of "OpenJPEG" to CMAKE_PREFIX_PATH or set
"OpenJPEG_DIR" to a directory containing one of the above files. If
"OpenJPEG" provides a separate development package or SDK, be sure it has
been installed.and, as expected, got this error message when trying to run jp2kio_reg:
jp2kio is not enabled
libopenjp2 is required for jp2kio_reg
See environ.h: #define HAVE_LIBJP2K
See prog/Makefile: link in -lopenjp2I don't know why the recent changes in sarray1.c, bmpio.c, readfile.c, etc are included in this merge, since they're already committed. Perhaps it doesn't matter?
Because I merged with master to resolve the conflicts.
it did not find JP2K library
How have you installed the JP2K library?
CMake Error at src/CMakeLists.txt:49 (target_link_libraries): Target "leptonica" links to: WebP::libwebpmux
I wonder how you get there when there is a find_package(WebP CONFIG REQUIRED) before that.
CMake Warning at CMakeLists.txt:124 (find_package): By not providing "FindOpenJPEG.cmake" in CMAKE_MODULE_PATH this project has ...
Same thing here. But it seems that you are using an old version. On line 124 is no find_package call ...
Hm I can't reproduce the errors you described. I am using the following script on linux and windows:
set -e
mkdir test_667
cd test_667
git clone git@github.com:webmproject/libwebp.git
cmake -Bbuild_libwebp -S libwebp -DCMAKE_INSTALL_PREFIX=dest -DCMAKE_PREFIX_PATH=$(pwd)/dest -DCMAKE_DISABLE_FIND_PACKAGE_ZLIB=ON -DCMAKE_DISABLE_FIND_PACKAGE_PNG=ON -DCMAKE_DISABLE_FIND_PACKAGE_JPEG=ON
cmake --build build_libwebp --config Release
cmake --install build_libwebp --config Release
git clone git@github.com:uclouvain/openjpeg.git
git -C openjpeg checkout 'v2.5.0'
cmake -Bbuild_openjpeg -S openjpeg -DCMAKE_INSTALL_PREFIX=dest -DCMAKE_PREFIX_PATH=$(pwd)/dest
cmake --build build_openjpeg --config Release
cmake --install build_openjpeg --config Release
git clone git@github.com:autoantwort/leptonica.git
git -C leptonica checkout fix-webp-transitive-dependencies
cmake -Bbuild_leptonica -S leptonica -DCMAKE_INSTALL_PREFIX=dest_leptonica -DCMAKE_PREFIX_PATH=$(pwd)/dest -DCMAKE_DISABLE_FIND_PACKAGE_ZLIB=ON -DCMAKE_DISABLE_FIND_PACKAGE_PNG=ON -DSW_BUILD=OFF
cmake --build build_leptonica --config Release
cmake --install build_leptonica --config Release
Building from head:
git clone https://github.com/danbloomberg/leptonica.git .
mkdir build
cd build
cmake ..gives this error message:
CMake Warning at CMakeLists.txt:124 (find_package):
By not providing "FindOpenJPEG.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "OpenJPEG",
but CMake did not find one.
Could not find a package configuration file provided by "OpenJPEG" with any
of the following names:
OpenJPEGConfig.cmake
openjpeg-config.cmake
Add the installation prefix of "OpenJPEG" to CMAKE_PREFIX_PATH or set
"OpenJPEG_DIR" to a directory containing one of the above files. If
"OpenJPEG" provides a separate development package or SDK, be sure it has
been installed.@DanBloomberg And how is this related to this PR? (And it is only a warning)
DanBloomberg
commented
1 year ago There have been several PRs, not all merged yet into head. I don't know which one or ones have caused this problem with openjpeg, but the consequence of the existing merges are that I can't run any program that uses libopenjp2 (see above when I tried to run prog/jp2kio_reg).
@zdenop has also noted an issue with webp.
Maintaining the cmake builds in leptonica is above my pay grade. @zdenop: do you have authority to merge in leptonica? If not, what do I need to do to give it to you?
autoantwort
commented
1 year ago The last PR that touched that code is at least 8 months old https://github.com/DanBloomberg/leptonica/blame/master/CMakeLists.txt#L124 :)
DanBloomberg
commented
1 year ago I have libopenjp2 (version 2.3.0) in /usr/local/lib cmake has had no trouble finding the package. The library and programs have built without error.
Which is simply to say that the changes from the latest 2 or 3 PRs have broken the build for programs using openjpeg.
I want to make sure that @zdenop has full authority to handle this :-)
autoantwort
commented
1 year ago Hm ok :/ So If you do a
git clone https://github.com/danbloomberg/leptonica.git .
git checkout 22b9028013c8991e8fcf8cac3d521652beddb6dc # commit before all my merged changes
mkdir build
cd build
cmake ..You don't get the error? :)
DanBloomberg
commented
1 year ago Same result. I get the same error message, and after building jp2kio_reg, the same failure when trying to run it:
jp2kio is not enabled
libopenjp2 is required for jp2kio_reg
See environ.h: #define HAVE_LIBJP2K
See prog/Makefile: link in -lopenjp2@DanBloomberg : I do not have permission to leptonica repository for merge.
Today I made a fresh cmake (3.25.2) MSVC (2019) installation of leptonica dependencies using the latest versions (usually git master). libwebp 1.3.0 and libopenjp2 2.5.0 are troublemakers ;-) :
webp andlibwebp`). However this PR detect&configure it correctly.find_package(OpenJPEG) does not work (should be in 2.4.0, so I need to check why it does not work for me).I think we need to test different (older) versions of these libraries to better fine-tune their detection. Maybe also a different cmake version (I need to check the versions installed on the oldest supported Ubuntu&Debian).
@autoantwort : you do not have time to wait for this test, I would suggest using the detection you need at the first stage and if detection fails use the current detection as a fallback.
autoantwort
commented
1 year ago Hm even OpenJPEG 2.4.0 is detected on my computer, but it fails later because the target in OpenJPEG 2.4.0 does not set the INTERFACE_INCLUDE_DIRECTORIES property. I will fix that :)
zdenop
commented
1 year ago tesseract runs a build test on Ubuntu 20.04 which has libwebp 0.6.1 : libopenjp2 2.3.1 - IMO this is the minumum versions we need to test...
zdenop
commented
1 year ago find_package(OpenJPEG) produces a lot of warnings but at it seems that Openjpeg is detected correctly. webp is not detected because of "_...By not providing "FindWebP.cmake" in CMAKE_MODULEPATH..."find_package(OpenJPEG) runs without problem. webp fails the same way as on Ubuntu.I found out that even google in libwebp2 does not rely on find_package(WebP) and makes several detections of webp...
DanBloomberg
commented
1 year ago @zdenop It doesn't surprise me that the situation with webp support is complicated, even at Google where webp was invented. (It was adopted from webm, a video encoder.) Leptonica was an early supporter of webp, which uses wavelets in the encoder (instead of the cosine transform used in jpeg). For a given amount of distortion, webp compresses better than jpeg, and because it uses wavelets, edges are preserved much better than jpeg.
When Google came out with an animated version of webp (better than gif, of course), I added support for writing those files. (Animated webp requires linking a new library, webpmux.) That was 5 or 6 years ago. But to this date, gmail, with about 2 billion users, does not even support animated webp. If you email an animated webp, it just displays the first image.
autoantwort
commented
1 year ago Yeah detecting WebP is fun... (even the cmake config file they provided in the past was wrong). I think I have now implemented a robust version that always finds WebP.
zdenop
commented
1 year ago Yeah detecting WebP is fun...
I agree ;-) At least OpenJPEG support is better than when I fixed it last time. I should have more time this week, so I hope we can finish this.
First of all: this PR should be split into smaller parts for better detection of possible problems:
set(libs_private "${libs_private} -ljpeg") with set(pkgs_private "${pkgs_private} libjpeg")autoantwort
commented
1 year ago I have now split the PR like you described :)
Includes #664
Currently not all transitive dependencies are forwarded in the generated cmake config which results in build failures downstream. For example tesseract fails with
in https://github.com/microsoft/vcpkg/pull/29266/commits/5504baf1726ebdd4506aab4988331200d918adc1