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: simplify libintl check #8

Merged
merged 1 commit into from
Jun 4, 2017
Merged

cmake: simplify libintl check #8

merged 1 commit into from
Jun 4, 2017

Conversation

bkuhls
Copy link
Contributor

@bkuhls bkuhls commented Jun 4, 2017

According to https://cmake.org/cmake/help/v3.4/module/FindIntl.html
the FindIntl module already checks for libintl.so.

@bkuhls
Copy link
Contributor Author

bkuhls commented Jun 4, 2017

The previous fixes did not work out for buildroot in all cases as well, this patch fixed the remaining buildroot problem, please test on Cygwin/MacOSX & Co.

@clanmills
Copy link
Collaborator

@bkuhls I fixed this a couple of hours ago and tested it on Cygwin/Linux/MacOS-X.

@clanmills
Copy link
Collaborator

@bkuhls I've added your code INCLUDE_DIRECTORIES(${Intl_INCLUDE_DIRS}) Thank You very much for this suggestion.

@clanmills clanmills closed this Jun 4, 2017
@bkuhls
Copy link
Contributor Author

bkuhls commented Jun 4, 2017

With current git HEAD as of 4e86c1d the buildroot build still failed without this patch:

[100%] Linking CXX executable ../bin/exiv2
CMakeFiles/exiv2.dir/exiv2.cpp.o: In function `main':
exiv2.cpp:(.text.startup+0x1f0): undefined reference to `libintl_bindtextdomain'
exiv2.cpp:(.text.startup+0x1f8): undefined reference to `libintl_bindtextdomain'
exiv2.cpp:(.text.startup+0x200): undefined reference to `libintl_textdomain'
exiv2.cpp:(.text.startup+0x204): undefined reference to `libintl_textdomain'
libexiv2.so.26.0.0: undefined reference to `libintl_dgettext'

PS: I will try 88cf586 now and will report back.

@bkuhls
Copy link
Contributor Author

bkuhls commented Jun 4, 2017

Build is still broken with 88cf586, I will send a rebased version of this patch

[ 98%] Linking CXX executable ../bin/remotetest
libexiv2.so.26.0.0: undefined reference to `libintl_dgettext'
libexiv2.so.26.0.0: undefined reference to `libintl_bindtextdomain'
[100%] Linking CXX executable ../bin/exiv2
CMakeFiles/exiv2.dir/exiv2.cpp.o: In function `main':
exiv2.cpp:(.text.startup+0x1f0): undefined reference to `libintl_bindtextdomain'
exiv2.cpp:(.text.startup+0x1f8): undefined reference to `libintl_bindtextdomain'
exiv2.cpp:(.text.startup+0x200): undefined reference to `libintl_textdomain'
exiv2.cpp:(.text.startup+0x204): undefined reference to `libintl_textdomain'
libexiv2.so.26.0.0: undefined reference to `libintl_dgettext'

@clanmills
Copy link
Collaborator

I have to go out for 3 hours this morning (and later today). We'll get this fixed today. Thanks for your help.

@clanmills clanmills reopened this Jun 4, 2017
According to https://cmake.org/cmake/help/v3.4/module/FindIntl.html
the FindIntl module already checks for libintl.so.
@bkuhls
Copy link
Contributor Author

bkuhls commented Jun 4, 2017

rebased patch force-pushed to branch

@clanmills clanmills merged commit 910f350 into Exiv2:master Jun 4, 2017
@bkuhls bkuhls deleted the gettext branch June 4, 2017 08:39
@clanmills
Copy link
Collaborator

@bkuhls

I'm new to git and have no idea what the expression "rebased patch force-pushed to branch" means, however I've accepted your patch and it's working fine on MacOS-X/Linux/Cygwin. Very good work. Thank you very much. I'll test/fix on MSVC.

a17r pushed a commit to a17r/exiv2 that referenced this pull request Oct 17, 2017
a17r pushed a commit to a17r/exiv2 that referenced this pull request Oct 22, 2017
a17r pushed a commit to a17r/exiv2 that referenced this pull request Oct 24, 2017
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.

2 participants