Skip to content

[tesseract]: find_package(lz4) in TesseractConfig.cmake#15180

Closed
mheyman wants to merge 4 commits intomicrosoft:masterfrom
mheyman:master
Closed

[tesseract]: find_package(lz4) in TesseractConfig.cmake#15180
mheyman wants to merge 4 commits intomicrosoft:masterfrom
mheyman:master

Conversation

@mheyman
Copy link
Copy Markdown
Contributor

@mheyman mheyman commented Dec 17, 2020

This fixes an issue I was having with building opencv4[contrib]. OpenCV[contrib] uses tesseract and tesseract uses lz4 but doesn't make the lz4:lz4 target available. It does make targets for other dependencies available so I figured this small change was okay..

Copy link
Copy Markdown
Contributor

@NancyLi1013 NancyLi1013 left a comment

Choose a reason for hiding this comment

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

Could you please also bump the Port-Version in CONTROL file?

@NancyLi1013 NancyLi1013 added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Dec 18, 2020
@NancyLi1013
Copy link
Copy Markdown
Contributor

It seems like lz4 is not required for tesseract directly. tesseract depends on libarchive and leptonica.

Lz4 is required for libarchive. I think we should fix this on libarchive instead of tesseract. Other targets are the same case.

Could you please help confirm and try?

@mheyman
Copy link
Copy Markdown
Contributor Author

mheyman commented Dec 21, 2020

I updated libarchive to have a simple libarchiveConfig.cmake that tesseract could depend on. Now opencv4 can use lz4::lz4 because it does a find_package(tesseract) which does a find_package(libarchive) which does a find_package(lz4).

portfile version numbers also bumped.

@cenit
Copy link
Copy Markdown
Contributor

cenit commented Dec 21, 2020

#15089 ?

@cenit
Copy link
Copy Markdown
Contributor

cenit commented Dec 21, 2020

if this one gets merged, I will fix mine to still enhance CI coverage for the future... we cannot have these problems slip in again :)

@cenit
Copy link
Copy Markdown
Contributor

cenit commented Dec 21, 2020

also, because you are using a find_package(libarchive), you can remove many find_packages in the tesseractConfig.cmake which were just transitively added from libarchive.
Also, please take care of spelling, with caps and lowercases. Your spelling of LibLZMA is just asking for troubles on case sensitive filesystems

@NancyLi1013
Copy link
Copy Markdown
Contributor

@cenit

I noticed that you also update these changes for libarchive and tesseract in PR #15089.

So let's revert these changes in this PR and we use the former way to fix the dependency lz4, what do you think?

@cenit
Copy link
Copy Markdown
Contributor

cenit commented Dec 23, 2020

I updated these changes also in #15089 because you're right, it was not tesseract directly depending on lz4 but its dependency libarchive is.
Since I'd had to merge these changes in that PR sooner or later, I already did it while I had some time.
That PR already contains latest requested you just asked here (and also I used find_dependency in order to properly manage REQUIRED keyword upstream). We are just duplicating work I think...

@NancyLi1013
Copy link
Copy Markdown
Contributor

@cenit

Thanks for instant response and also for your hard work.
I think we can merge this PR first, then you update the changes in your PR.

@mheyman

Since this PR includes the same change as PR #15089.

Could you please revert this changes in this PR and only add find_package(lz4 REQUIRED)" TESSERACT_CONFIG "${TESSERACT_CONFIG}")?

I'm so sorry for my poor consideration, which leads you to do repeated work.

dan-shaw pushed a commit that referenced this pull request Jan 17, 2021
… tesseract downstream and unblock opencv CI (#15089)

* [tesseract] add missing reference for downstream projects

* [tesseract] restore ci, fix many regressions that are uncovered by that

* Update ports/opencv2/CONTROL

Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>

* [opencv] fix regressions on uwp, accept failure on arm64 for now

* Apply suggestions from code review

Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>

* [opencv4] allow failures on all arm windows targets, both win32 and uwp

* adopts hints from #15180

* [libarchive] bump control version

* [libarchive] use vcpkg-cmake-wrapper instead of a custom libarchiveConfig, since it is vcpkg-provided and not port-provided

* enable features to be visible in parent scope

* apply documentation fix from CI

* [libarchive] remove unnecessary lines in portfile

* fix regressions

* Update ports/gdcm/CONTROL

* use more compact logic syntax

* add new versions to baseline

Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
@NancyLi1013
Copy link
Copy Markdown
Contributor

Since PR #15089 has fixed this issue, which has been merged to master.

So this PR is not needed now. Still thanks for your contribution @mheyman.

dan-shaw pushed a commit that referenced this pull request Jan 22, 2021
* [tesseract] add missing reference for downstream projects

* [tesseract] restore ci, fix many regressions that are uncovered by that

* Update ports/opencv2/CONTROL

Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>

* [opencv] fix regressions on uwp, accept failure on arm64 for now

* Apply suggestions from code review

Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>

* [opencv4] allow failures on all arm windows targets, both win32 and uwp

* [OpenCV] update to v4.5, draft

* Restore CI tests on arm architectures, they will have to work!

* adopts hints from #15180

* [libarchive] bump control version

* [libarchive] use vcpkg-cmake-wrapper instead of a custom libarchiveConfig, since it is vcpkg-provided and not port-provided

* enable features to be visible in parent scope

* apply documentation fix from CI

* [libarchive] remove unnecessary lines in portfile

* update patches

* restore ci tests for all opencv4 configs

* add port versions to baseline

* [OpenCV contrib] fix glog integration

* [OpenCV4] fix target processor detection

* update version refs

* [OpenCV3] fix target processor detection

* fix also ocv3

* remove vtk feature from opencv-ci testing

* remove qt5-tools from baseline, it works locally

Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants