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

Rocksdb compilation fails with IBM Java #1926

Closed
ayappanec opened this issue Feb 27, 2017 · 10 comments
Closed

Rocksdb compilation fails with IBM Java #1926

ayappanec opened this issue Feb 27, 2017 · 10 comments
Assignees
Labels

Comments

@ayappanec
Copy link
Contributor

ayappanec commented Feb 27, 2017

While doing make rocksdbjava with IBM Java, it fails with the following error.

In file included from java/rocksjni/backupenginejni.cc:14:0:
./java/./rocksjni/portal.h: In static member function 'static _jbyteArray* rocksdb::JniUtil::v_op(std::function<rocksdb::Status(rocksdb::Slice, std::basic_string<char>*)>, JNIEnv*, jbyteArray, jint)':
./java/./rocksjni/portal.h:929:78: error: invalid conversion from 'const jbyte* {aka const signed char*}' to 'jbyte* {aka signed char*}' [-fpermissive]
                                 reinterpret_cast<const jbyte*>(value.c_str()));
                                                                              ^
In file included from java/rocksjni/backupenginejni.cc:9:0:
/opt/ibm/java-x86_64-80/include/jni.h:627:7: error:   initializing argument 4 of 'void JNIEnv_::SetByteArrayRegion(jbyteArray, jsize, jsize, jbyte*)' [-fpermissive]
  void SetByteArrayRegion(jbyteArray array, jsize start, jsize len, jbyte *buf) { functions->SetByteArrayRegion(this, array, start, len, buf); }
       ^
In file included from java/rocksjni/backupablejni.cc:17:0:
./java/./rocksjni/portal.h: In static member function 'static _jbyteArray* rocksdb::JniUtil::v_op(std::function<rocksdb::Status(rocksdb::Slice, std::basic_string<char>*)>, JNIEnv*, jbyteArray, jint)':
./java/./rocksjni/portal.h:929:78: error: invalid conversion from 'const jbyte* {aka const signed char*}' to 'jbyte* {aka signed char*}' [-fpermissive]
                                 reinterpret_cast<const jbyte*>(value.c_str()));
                                                                              ^
In file included from java/rocksjni/backupablejni.cc:12:0:
/opt/ibm/java-x86_64-80/include/jni.h:627:7: error:   initializing argument 4 of 'void JNIEnv_::SetByteArrayRegion(jbyteArray, jsize, jsize, jbyte*)' [-fpermissive]
  void SetByteArrayRegion(jbyteArray array, jsize start, jsize len, jbyte *buf) { functions->SetByteArrayRegion(this, array, start, len, buf); }
       ^
In file included from java/rocksjni/checkpoint.cc:15:0:
./java/./rocksjni/portal.h: In static member function 'static _jbyteArray* rocksdb::JniUtil::v_op(std::function<rocksdb::Status(rocksdb::Slice, std::basic_string<char>*)>, JNIEnv*, jbyteArray, jint)':
./java/./rocksjni/portal.h:929:78: error: invalid conversion from 'const jbyte* {aka const signed char*}' to 'jbyte* {aka signed char*}' [-fpermissive]
                                 reinterpret_cast<const jbyte*>(value.c_str()));
                                                                              ^
....................
....................,

Checking out the jni header file in IBM Java, it has

void SetByteArrayRegion(jbyteArray array, jsize start, jsize len, **jbyte *buf**) { functions->SetByteArrayRegion(this, array, start, len, buf); }

whereas in Openjdk,

void (JNICALL *SetByteArrayRegion)
      (JNIEnv *env, jbyteArray array, jsize start, jsize len, **const jbyte *buf**);

The const qualifier is missing in Set* JNI calls for IBM Java. I made a small patch which fixes this issue.
ibmsoe@6663b40

If this okay, i can go for a pull request.

Thanks
Ayappan P

@adamretter
Copy link
Collaborator

@ayappanec That looks okay to me. @IslamAbdelRahman Any comments on this?

@siying
Copy link
Contributor

siying commented Feb 27, 2017

@ayappanec if the extra const is the problem, will removing the const in cast work? Do we need to move from reintepret_cast<> to C style cast? We are following Google C++ Style and C style casting is not recommended: https://google.github.io/styleguide/cppguide.html#Casting

@zdenekkorcak
Copy link

zdenekkorcak commented Feb 28, 2017

We used extra const_cast<jbyte*> to solve this problem:

const_cast<jbyte*>(reinterpret_cast<const jbyte*>(value_slice.data()))

It is a little bit longer than C style casting, but it follows Google C++ Style

@ayappanec
Copy link
Contributor Author

ayappanec commented Feb 28, 2017

@siying Removing just the const qualifier fails to compile.

 In file included from java/rocksjni/write_batch_with_index.cc:13:0:
./java/./rocksjni/portal.h: In static member function âstatic _jbyteArray* rocksdb::JniUtil::v_op(std::function<rocksdb::Status(rocksdb::Slice, std::basic_string<char>*)>, JNIEnv*, jbyteArray, jint          )â:
./java/./rocksjni/portal.h:929:71: error: reinterpret_cast from type âconst char*â to type âjbyte* {aka signed char*}â casts away qualifiers
                                 reinterpret_cast<jbyte*>(value.c_str()));

As mentioned by @zdenekkorcak , we need to add an extra const_cast to fix this.

@ayappanec
Copy link
Contributor Author

New changes at ibmsoe@7ff6d27

@ayappanec
Copy link
Contributor Author

Any update on this ?

@adamretter
Copy link
Collaborator

@ayappanec Switching to the C++ style I think likely addresses @siying's concerns. Can you open a PR?

I have a concern though, that we have no way of preventing this from creeping in again in future because there is no IBM Java CI

@ayappanec
Copy link
Contributor Author

We have a Jenkins CI machine for Open Source projects in PowerPC64 LE. I will include rocksdb project with IBM java & Openjdk in PowerPC64 LE.

@adamretter
Copy link
Collaborator

@ayappanec Okay but is the output of that public and can we somehow get notifications if we break the build like we do with Travis on GitHub?

@ayappanec
Copy link
Contributor Author

The output of that will be public. I will try to set notifications for facebook/rocksdb if there is any build break.

facebook-github-bot pushed a commit that referenced this issue Mar 23, 2017
Summary:
PR to fix this issue -> #1926
Closes #1965

Differential Revision: D4682411

Pulled By: siying

fbshipit-source-id: a519be1
sagar0 pushed a commit that referenced this issue May 19, 2017
Summary:
PR to fix this issue -> #1926
Closes #1965

Differential Revision: D4682411

Pulled By: siying

fbshipit-source-id: a519be1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants