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

Addition of libpostal preset #502

Merged
merged 24 commits into from
Jan 15, 2018
Merged

Addition of libpostal preset #502

merged 24 commits into from
Jan 15, 2018

Conversation

Maurice-Betzel
Copy link
Contributor

Requesting addition of the libpostal preset for Windows, Linux and MacOS 64bit.
libpostal is a C library for parsing/normalizing street addresses around the world using statistical NLP and open data. The goal of this project is to understand location-based strings in every language, everywhere.

@saudet
Copy link
Member

saudet commented Dec 29, 2017

Thanks! Please also add to .travis.yml and .appveyor.yml and let's see if it builds fine on all platforms.

@saudet
Copy link
Member

saudet commented Dec 30, 2017

Thanks! Could you also add entries for unsupported platforms to make sure empty artifacts get created?

@saudet
Copy link
Member

saudet commented Dec 30, 2017

Yeah, unfortunately Maven, etc require empty artifacts for unsupported platforms, or users have to exclude them manually.

@Maurice-Betzel
Copy link
Contributor Author

Now the unsupported platform builds are failing because of the missing libpostal header file that JavaCPP is expecting. How to solve this?

@saudet
Copy link
Member

saudet commented Dec 31, 2017

List only the supported platforms in the @Platform(value = ... annotation value.

@Maurice-Betzel
Copy link
Contributor Author

Maurice-Betzel commented Dec 31, 2017

Keep your fingers crossed, and i wish you a happy and healthy new year :)
(Now i am beginning to understand how the annotations steer JavaCPP)

@Maurice-Betzel
Copy link
Contributor Author

Since libpostal is written in c and the header file declares #ifdef __cplusplus extern "C" {..., do i need to use cinclude instead of include in the annotations declaration?

@saudet
Copy link
Member

saudet commented Jan 1, 2018

It doesn't look like it, but it's usually preferable yes.

Maurice-Betzel referenced this pull request in openvenues/libpostal Jan 2, 2018
…iven components if there are other ignorable components
@Maurice-Betzel
Copy link
Contributor Author

Maurice-Betzel commented Jan 3, 2018

g++ has no cpp 11 for Linux x86_64? LLVM seems to have it, what am i missing/overseeing?

@Maurice-Betzel
Copy link
Contributor Author

Samuel, i see you have removed the need for empty artefacts. If the builds run fine now I will commit the resolved conflicts.

@saudet
Copy link
Member

saudet commented Jan 4, 2018

Yes, please do. Sorry for the breaking changes, but you had me started thinking about this and it clicked so I made the changes. :) And while you're at it, version "1.1-alpha" was just released, so let's use that for the presets: https://github.com/openvenues/libpostal/releases

As for C++11 support with GCC or Clang, we need to pass them an option, and we can do that with a @Platform(compiler="cpp11", ... annotation.

@Maurice-Betzel
Copy link
Contributor Author

I just removed the cpp 11 directive because the linux build failed on travis because of it and i cannot trace why. I will try the alpha version locally, yesterday morning the build was breaking untill the latest commits of albarrentine.

@Maurice-Betzel
Copy link
Contributor Author

Having following issues with v1.1-alpha executing JavaCPP:

[INFO] --- javacpp:1.3.4-SNAPSHOT:build (javacpp.compiler) @ libpostal ---
[INFO] Detected platform "windows-x86_64"
[INFO] Building for platform "windows-x86_64"
[INFO] Generating C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp
[INFO] Generating C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnilibpostal.cpp
[INFO] Compiling C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\windows-x86_64\jnilibpostal.dll
[INFO] cl /IC:\msys64\home\betzm\javacpp-presets\libpostal/cppbuild/windows-x86_64/include/ /IC:\Users\betzm\Development\jdk\jdk1.8.0_144\include /IC:\Users\betzm\Development\jdk\jdk1.8.0_144\include\win32 C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnilibpostal.cpp C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp /Oi /O2 /EHsc /Gy /GL /MD /LD /W3 /link /OUT:jnilibpostal.dll /LIBPATH:C:\msys64\home\betzm\javacpp-presets\libpostal/cppbuild/windows-x86_64/lib/ libpostal.lib psapi.lib
Microsoft (R) C/C++ Optimizing Compiler Version 19.12.25830.2 for x64
Copyright (C) Microsoft Corporation. All rights reserved.

jnilibpostal.cpp
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnilibpostal.cpp(241): warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible loss of data
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnilibpostal.cpp(242): warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible loss of data
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnilibpostal.cpp(283): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(132): note: see declaration of 'strcpy'
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnilibpostal.cpp(285): warning C4996: 'strncpy': This function or variable may be unsafe. Consider using strncpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(337): note: see declaration of 'strncpy'
jnijavacpp.cpp
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(501): warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible loss of data
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(502): warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible loss of data
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(543): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(132): note: see declaration of 'strcpy'
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(545): warning C4996: 'strncpy': This function or variable may be unsafe. Consider using strncpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(337): note: see declaration of 'strncpy'
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(1167): warning C4996: 'strcat': This function or variable may be unsafe. Consider using strcat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(89): note: see declaration of 'strcat'
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(1230): warning C4996: 'strncpy': This function or variable may be unsafe. Consider using strncpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(337): note: see declaration of 'strncpy'
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(1252): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(132): note: see declaration of 'strcpy'
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(1310): warning C4996: 'strncat': This function or variable may be unsafe. Consider using strncat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(265): note: see declaration of 'strncat'
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(1362): warning C4996: 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(181): note: see declaration of 'strerror'
C:\msys64\home\betzm\javacpp-presets\libpostal\target\classes\org\bytedeco\javacpp\jnijavacpp.cpp(1448): warning C4996: 'strtok': This function or variable may be unsafe. Consider using strtok_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
C:\Program Files (x86)\Windows Kits\10\include\10.0.16299.0\ucrt\string.h(439): note: see declaration of 'strtok'
Microsoft (R) Incremental Linker Version 14.12.25830.2
Copyright (C) Microsoft Corporation. All rights reserved.

/out:jnilibpostal.dll
/ltcg
/dll
/implib:jnilibpostal.lib
/OUT:jnilibpostal.dll
/LIBPATH:C:\msys64\home\betzm\javacpp-presets\libpostal/cppbuild/windows-x86_64/lib/
libpostal.lib
psapi.lib
jnilibpostal.obj
jnijavacpp.obj
Creating library jnilibpostal.lib and object jnilibpostal.exp
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_toponym_duplicate
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_street_duplicate_fuzzy
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_house_number_duplicate
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_get_duplicate_options_with_languages
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_get_default_duplicate_options
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_near_dupe_hashes
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_floor_duplicate
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_po_box_duplicate
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_get_default_fuzzy_duplicate_options
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_place_languages
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_name_duplicate
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_postal_code_duplicate
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_unit_duplicate
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_tokenize
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_near_dupe_hashes_languages
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_normalize_string
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_get_default_fuzzy_duplicate_options_with_languages
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_get_near_dupe_hash_default_options
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_street_duplicate
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_is_name_duplicate_fuzzy
jnilibpostal.obj : error LNK2001: unresolved external symbol __imp_libpostal_expand_address_root
jnilibpostal.obj : error LNK2001: unresolved external symbol libpostal_normalized_tokens
jnilibpostal.dll : fatal error LNK1120: 22 unresolved externals
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------

@Maurice-Betzel
Copy link
Contributor Author

Indeed, so you say configure this in the preset?

@saudet
Copy link
Member

saudet commented Jan 8, 2018 via email

@Maurice-Betzel
Copy link
Contributor Author

Yes, it is ether cp ../lib/libpostal.dylib ../lib/liblibpostal.dylib (.so for linux) with link="libpostal" OR no cp and link="postal". Your pick :)

@saudet
Copy link
Member

saudet commented Jan 8, 2018

Why can't we do exactly like libdc1394?

@Maurice-Betzel
Copy link
Contributor Author

Analog to libdc1394 where link=dc1394 with no lib prefix, that would be option 2 then.

@Maurice-Betzel
Copy link
Contributor Author

Maurice-Betzel commented Jan 9, 2018

Some modules other then libpostal are breaking, as far as i can see by missing dependencies / timeouts. Is it possible to restart these individually?

@saudet
Copy link
Member

saudet commented Jan 9, 2018

Yes, but it's not related to libpostal, so don't worry about them. One thing we'll need to fix though is that libpostal-1.dll doesn't get bundled. Add a preload like for libdc1394, in this case preload = "libpostal-1.dll" and make sure it gets copied. Also, no need of the "oldcompiler" stuff in the pom.xml, so clean that to how it was before. :) BTW, I try to order the presets by themes. I guess I'd put "libpostal" just after "llvm"... What do you think? And while we're at it, there are also small but unnecessary changes to the formatting in .travis.yml, .appveyor.yml, and the main pom.xml. Could you clean that up as well? Thanks!

@saudet
Copy link
Member

saudet commented Jan 9, 2018

Also you forgot to include the version of the link library. It'll probably fail to load on Linux and Mac because of that. We'll need something similar to libdc1394, and I'm guessing link = "[email protected]" should work. On Mac, you might also need to patch the configure file to add an @rpath, as is done for libdc1394 as well: https://github.com/bytedeco/javacpp-presets/blob/master/libdc1394/libdc1394-2.2.5-macosx.patch

@Maurice-Betzel
Copy link
Contributor Author

Potverdorie, i need to delete my local Maven repo before builing and testing...

@saudet
Copy link
Member

saudet commented Jan 9, 2018

No you don't need to do that, but you do need to clean up the target and cppbuild directories, or it will find the libraries there by their install full paths.

Corrected formatting in the pom and CI/CD markup,
corrected ordering of libpostal after llvm and before mkl if llvm is not available for specific a platform.
Tested after cleaning and deleting the.javacpp folder.
@saudet
Copy link
Member

saudet commented Jan 12, 2018

You'll probably need to add an @rpath on Mac as well. What does otool -L libjniale.dylib returns?

@Maurice-Betzel
Copy link
Contributor Author

Maurice-Betzel commented Jan 12, 2018

Well, i tested libpostal on all 3 platforms without problems so i thought we might not need the @rpath. And i just saw i have forgotten to reset the platform pom...

@Maurice-Betzel
Copy link
Contributor Author

Maurice-Betzel commented Jan 12, 2018

I just tested all three platforms again, doing a maven clean on the preset and the platform, deleting the maven vectors in my local repo and deleting the .javacpp cache folders. They where all functional without errors. Is the addition of a patch with @rpath really needed?

@saudet
Copy link
Member

saudet commented Jan 12, 2018 via email

@saudet
Copy link
Member

saudet commented Jan 13, 2018

It looks like they keep updating v1.1-alpha:
https://github.com/openvenues/libpostal/releases
Does it work now?

@Maurice-Betzel
Copy link
Contributor Author

I tested the lib on Centos 7, Windows 10 and MacOS High Sierra, all 64 bit arch, successfully.
Before the test i deleted the local Maven repos of libpostal and the complete .javacpp cache folders and executed a Maven build on the preset and platform module.

@saudet
Copy link
Member

saudet commented Jan 13, 2018

But you didn't remove the javacpp-presets directory itself, so it probably doesn't work... In any case, if v1.1-alpha works just as well, let's use that as version!

@Maurice-Betzel
Copy link
Contributor Author

...a Maven clean was included with the build, deleting the project files, I will try the 1.1 version as well.

@Maurice-Betzel
Copy link
Contributor Author

You change the mac configuration on the fly with sed -i="" 's/-install_name \\$rpath/-install_name @rpath/g' configure, what is this good for?

@Maurice-Betzel
Copy link
Contributor Author

Maurice-Betzel commented Jan 14, 2018

Got it, the binary can search the dynamic library through a relative path allowing it to run everywhere:

On the Mac a dynamic library (dylib) has an "install name". The install name is a path baked into the dynamic library that says where to find the library at runtime. When you link against the dylib this path is saved in your binary so that your binary can find the dylib at runtime.

Since run paths is rather new to macos and is a standard linux feature, why do we not have to configure this for the linux build as well? Or does JavaCPP anticipate on this?

@saudet
Copy link
Member

saudet commented Jan 15, 2018

We don't need it on Linux. It does what @rpath does by default, always has.

@saudet saudet merged commit daea194 into bytedeco:master Jan 15, 2018
@Maurice-Betzel
Copy link
Contributor Author

Still a lot to learn in native land, thanks for your patience. I have put up a blog post mentioning the JavaCPP project and your name if that is OK with you.

@saudet
Copy link
Member

saudet commented Jan 15, 2018

Yes of course, please do! And thanks for the contribution!

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