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

CMake - Set minimum Leptonica version to 1.74.0 #608

Merged
merged 1 commit into from
Dec 25, 2016
Merged

CMake - Set minimum Leptonica version to 1.74.0 #608

merged 1 commit into from
Dec 25, 2016

Conversation

amitdo
Copy link
Collaborator

@amitdo amitdo commented Dec 25, 2016

No description provided.

@amitdo
Copy link
Collaborator Author

amitdo commented Dec 25, 2016

@stweil

In configure.ac, should we change

PKG_CHECK_MODULES([LEPTONICA], [lept >= 1.74], [have_lept=true], [have_lept=false])

to

PKG_CHECK_MODULES([LEPTONICA], [lept >= 1.74.0], [have_lept=true], [have_lept=false])

?

@egorpugin egorpugin merged commit c124f87 into tesseract-ocr:master Dec 25, 2016
@amitdo amitdo deleted the cmake-leptonica-1740 branch December 25, 2016 11:20
@zdenop
Copy link
Contributor

zdenop commented Dec 26, 2016

@amitdo:
What is benefit/difference between [lept >= 1.74] and [lept >= 1.74.0]?
BTW: pkg-config --modversion lept shows 1.74

@amitdo
Copy link
Collaborator Author

amitdo commented Dec 26, 2016

This is how it is tagged in the github repo. In the Leptonica website it is '1.74'.
You can change it if you want to :-)

@zdenop
Copy link
Contributor

zdenop commented Dec 26, 2016

Well allheaders.h defines only LIBLEPT_MAJOR_VERSION (1) and LIBLEPT_MINOR_VERSION (74), so I see no reason to change...

@amitdo
Copy link
Collaborator Author

amitdo commented Dec 26, 2016

I did it after the .travis.yml commits. As you can see, I needed 3 tries to make it work :-)
1.74 -> 1.74.0 -> +drop the v before the version number ...

So I thought it should be 1.74.0 in CMake too.

@amitdo
Copy link
Collaborator Author

amitdo commented Dec 26, 2016

@DanBloomberg

The 1.74 vs. 1.74.0 is somewhat confusing ...

@egorpugin
Copy link
Contributor

It's a switch to semver also. 1.74.0 tag in git and version numbers in code/build tools.

@egorpugin
Copy link
Contributor

For cmake 1.74 and 1.74.0 are equal. And travis needs precise git tag to work.

@stweil
Copy link
Member

stweil commented Dec 26, 2016

At least for cmake 3.6.2 only 1.74 but not 1.74.0 works. Tested on Raspbian.

@jbreiden
Copy link
Contributor

jbreiden commented Dec 26, 2016 via email

@stweil
Copy link
Member

stweil commented Dec 26, 2016

I just ran a test with cmake 3.7.1-1 on Debian GNU Linux. It also works only with 1.74, but not with 1.74.0.

@DanBloomberg
Copy link

DanBloomberg commented Dec 28, 2016 via email

@jbreiden
Copy link
Contributor

jbreiden commented Dec 28, 2016 via email

@egorpugin
Copy link
Contributor

Dan, the issue (if any) is probably somewhere in autotools config on lept side. No need to rename git tag, it's fine. In case of any critical changes, you could remove tag and add it again, or just make 1.74.1 release with fixes.

@chewi
Copy link

chewi commented Dec 31, 2016

The problem is that Leptonica's lept.pc has 1.74 rather than 1.74.0. If you edit that file to have the latter then it works. This value originates from configure.ac, which also has 1.74. @DanBloomberg, you need to be consistent with this everywhere.

@stweil
Copy link
Member

stweil commented Dec 31, 2016

Yes, I think this is the reason why 1.74.0 did not work for me.

@DanBloomberg
Copy link

DanBloomberg commented Jan 1, 2017 via email

@stweil
Copy link
Member

stweil commented Jan 1, 2017

I suggest using 1.74.1. Fixes are needed for README.html, configure.ac and version-notes.html.

@stweil
Copy link
Member

stweil commented Jan 1, 2017

... and CMakeLists.txt.

@chewi
Copy link

chewi commented Jan 1, 2017

At this point, I don't mind sticking with 1.74 for now as we've already compensated in the necessary places. My Gentoo release ended up being 1.74 as this matched the tarball and the tarball's subdirectory. But if you want to push a new 1.74.1 release out to make sure we get it right while it's still fresh in our minds, that's cool.

@zdenop
Copy link
Contributor

zdenop commented Jan 1, 2017

@DanBloomberg: if you decide to implement version format "1.74.1" do not forget to implement it into allheaders.h as LIBLEPT_MAJOR_VERSION (1), LIBLEPT_MINOR_VERSION (74) and LIBLEPT_MICRO_VERSION (1)...

@stweil
Copy link
Member

stweil commented Jan 1, 2017

LIBLEPT_MICRO_VERSION would be a new interface. IMHO it is not necessary, because C code should not depend on the patch level or micro version.

@egorpugin
Copy link
Contributor

Or better LIBLEPT_PATCH_VERSION as in semver (major.minor.patch).

@DanBloomberg
Copy link

DanBloomberg commented Jan 1, 2017 via email

@DanBloomberg
Copy link

DanBloomberg commented Jan 2, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants