Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Build separate artifacts for library using CPack #123

Merged
merged 11 commits into from
May 26, 2020

Conversation

jmazanec15
Copy link
Member

@jmazanec15 jmazanec15 commented May 20, 2020

Issue #, if available:
#122

Description of changes:
This change uses CPack to generate separate RPM/DEB artifacts for the KNN JNI Library and adds the library as a dependency of the KNN Plugin.

The following steps will build the RPM and DEB packages in the jni/packages folder:

cd jni
cmake .
make package

2 packages are generated:

  1. opendistro-knnlib-1.8.0.0-0.1_linux.x86_64.deb
  2. opendistro-knnlib-1.8.0.0-0.1_linux.x86_64.rpm

Additionally, I changed the directory layout of JNI to follow a more standard C++ convention. The JNI directory now looks like this:

CMakeLists.txt
external/nmslib
src/com_amazon_opendistroforelasticsearch_knn_index_v1736_KNNIndex.cpp
src/CMakeLists.txt
include/com_amazon_opendistroforelasticsearch_knn_index_v1736_KNNIndex.h
release/
packages/

Now, when running make, it will build the library in jni/release instead of buildSrc. ./gradlew build will then build the library and move it to buildSrc.

Updated documentation as well.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jmazanec15 jmazanec15 changed the title Build separate artifacts for library using CPack[WIP] [WIP] Build separate artifacts for library using CPack May 20, 2020
dependsOn buildJniLib
from "$buildDir/jni/release"
include "*"
Copy link
Member

Choose a reason for hiding this comment

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

Minor: Should we specify to just copy the library? Making sure nothing else is copied

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the library extension is dependent on Operating System, I used wildcard here. Because only the library is placed in jni/release in the current build process, I believe this operation is okay.

Copy link
Member

@vamshin vamshin left a comment

Choose a reason for hiding this comment

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

LGMT! Thanks for pulling knn library to a separate rpm so that it could be pulled from rpm repo with out having to download from other sources. This was an issue for the users who does not have internet connectivity in their production servers. Thanks for innovating here.

@jmazanec15 jmazanec15 changed the title [WIP] Build separate artifacts for library using CPack Build separate artifacts for library using CPack May 26, 2020
@jmazanec15 jmazanec15 merged commit 52dd13b into opendistro-for-elasticsearch:master May 26, 2020
chenqi0805 added a commit to chenqi0805/k-NN that referenced this pull request Jun 9, 2020
* master:
  added github action to build library artifacts (opendistro-for-elasticsearch#132)
  FIX: buildDir->rootDir
  Build separate artifacts for library using CPack (opendistro-for-elasticsearch#123)
  Fix test structure (opendistro-for-elasticsearch#125)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants