You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/06/11 19:50:12 UTC
[GitHub] [hbase-native-client] bharathv opened a new pull request #4: HBASE-24539: Fix the classpath for mini-cluster
bharathv opened a new pull request #4:
URL: https://github.com/apache/hbase-native-client/pull/4
Fixes the classpath and HBaseTestingUtility c'tor invocation.
Promoted some local JNI refs to global to avoid GC cleaning them up.
Tested locally with a single unit-test that can bring-up the minicluster
up cleanly. There are other minicluster lifecycle issues that need to
be fixed (follow on patches) before we can fully use it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase-native-client] phrocker commented on a change in pull request #4: HBASE-24539: Fix the classpath for mini-cluster
Posted by GitBox <gi...@apache.org>.
phrocker commented on a change in pull request #4:
URL: https://github.com/apache/hbase-native-client/pull/4#discussion_r439368476
##########
File path: src/hbase/test-util/mini-cluster.cc
##########
@@ -20,163 +20,147 @@
#include "hbase/test-util/mini-cluster.h"
#include <fcntl.h>
#include <glog/logging.h>
-#include <boost/filesystem/fstream.hpp>
-#include <boost/filesystem/operations.hpp>
#include <fstream>
using hbase::MiniCluster;
+using std::string;
+using std::ifstream;
JNIEnv *MiniCluster::CreateVM(JavaVM **jvm) {
JavaVMInitArgs args;
JavaVMOption jvm_options;
args.version = JNI_VERSION_1_6;
args.nOptions = 1;
+ // Sets the Java classpath to load the test jars and dependencies.
+ // Either set the CLASSPATH environment variable before running the test or the test loads it from the default
+ // location, if it exists.
char *classpath = getenv("CLASSPATH");
- std::string clspath;
- if (classpath == NULL || strstr(classpath, "-tests.jar") == NULL) {
- std::string clsPathFilePath("../../../hbase-build-configuration/target/cached_classpath.txt");
- std::ifstream fd(clsPathFilePath);
- std::string prefix("");
- if (fd.is_open()) {
- if (classpath == NULL) {
- LOG(INFO) << "got empty classpath";
- } else {
- // prefix bootstrapper.jar
- prefix.assign(classpath);
- }
- std::string line;
- if (getline(fd, line)) {
- clspath = prefix + ":" + line;
- int ret = setenv("CLASSPATH", clspath.c_str(), 1);
- LOG(INFO) << "set clspath " << ret;
- } else {
- LOG(INFO) << "nothing read from " << clsPathFilePath;
- exit(-1);
- }
- } else {
- LOG(INFO) << "nothing read from " << clsPathFilePath;
- exit(-1);
+ // Copy it to a string so that we don't inadverdently change the environment variable.
+ string final_classpath;
+ if (classpath == nullptr || strstr(classpath, "-tests.jar") == nullptr) {
+ if (classpath != nullptr) {
+ final_classpath.assign(classpath);
}
- fd.close();
- } else {
- clspath.assign(classpath, strlen(classpath));
+ // Default classpath loaded from downloaded HBase src (paths defined in CMakeLists.txt)
+ string clspath_file_path("./apachehbase-src/hbase-build-configuration/target/cached_classpath.txt");
Review comment:
In the case where they provide HBASE_HOME this might get a little tricky. Could have cmake pass HBASE_HOME as an option for compilation here so the path needn't be hardcoded in the future ( as a follow up ).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase-native-client] bharathv merged pull request #4: HBASE-24539: Fix the classpath for mini-cluster
Posted by GitBox <gi...@apache.org>.
bharathv merged pull request #4:
URL: https://github.com/apache/hbase-native-client/pull/4
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase-native-client] bharathv commented on pull request #4: HBASE-24539: Fix the classpath for mini-cluster
Posted by GitBox <gi...@apache.org>.
bharathv commented on pull request #4:
URL: https://github.com/apache/hbase-native-client/pull/4#issuecomment-643501522
> I think I encountered some of those life cycle issues
Plan to redo the lifecycle as a part of https://issues.apache.org/jira/browse/HBASE-24538
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase-native-client] bharathv commented on pull request #4: HBASE-24539: Fix the classpath for mini-cluster
Posted by GitBox <gi...@apache.org>.
bharathv commented on pull request #4:
URL: https://github.com/apache/hbase-native-client/pull/4#issuecomment-642894184
@phrocker FYI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase-native-client] bharathv commented on a change in pull request #4: HBASE-24539: Fix the classpath for mini-cluster
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #4:
URL: https://github.com/apache/hbase-native-client/pull/4#discussion_r439660116
##########
File path: src/hbase/test-util/mini-cluster.cc
##########
@@ -20,163 +20,147 @@
#include "hbase/test-util/mini-cluster.h"
#include <fcntl.h>
#include <glog/logging.h>
-#include <boost/filesystem/fstream.hpp>
-#include <boost/filesystem/operations.hpp>
#include <fstream>
using hbase::MiniCluster;
+using std::string;
+using std::ifstream;
JNIEnv *MiniCluster::CreateVM(JavaVM **jvm) {
JavaVMInitArgs args;
JavaVMOption jvm_options;
args.version = JNI_VERSION_1_6;
args.nOptions = 1;
+ // Sets the Java classpath to load the test jars and dependencies.
+ // Either set the CLASSPATH environment variable before running the test or the test loads it from the default
+ // location, if it exists.
char *classpath = getenv("CLASSPATH");
- std::string clspath;
- if (classpath == NULL || strstr(classpath, "-tests.jar") == NULL) {
- std::string clsPathFilePath("../../../hbase-build-configuration/target/cached_classpath.txt");
- std::ifstream fd(clsPathFilePath);
- std::string prefix("");
- if (fd.is_open()) {
- if (classpath == NULL) {
- LOG(INFO) << "got empty classpath";
- } else {
- // prefix bootstrapper.jar
- prefix.assign(classpath);
- }
- std::string line;
- if (getline(fd, line)) {
- clspath = prefix + ":" + line;
- int ret = setenv("CLASSPATH", clspath.c_str(), 1);
- LOG(INFO) << "set clspath " << ret;
- } else {
- LOG(INFO) << "nothing read from " << clsPathFilePath;
- exit(-1);
- }
- } else {
- LOG(INFO) << "nothing read from " << clsPathFilePath;
- exit(-1);
+ // Copy it to a string so that we don't inadverdently change the environment variable.
+ string final_classpath;
+ if (classpath == nullptr || strstr(classpath, "-tests.jar") == nullptr) {
+ if (classpath != nullptr) {
+ final_classpath.assign(classpath);
}
- fd.close();
- } else {
- clspath.assign(classpath, strlen(classpath));
+ // Default classpath loaded from downloaded HBase src (paths defined in CMakeLists.txt)
+ string clspath_file_path("./apachehbase-src/hbase-build-configuration/target/cached_classpath.txt");
Review comment:
It also takes CLASSPATH env variable (L37). If its set, everything from the classpath is passed over to java.classpath.
CLASSPATH="/path/to/cached_classpath.txt" ./test-binary also works if the users want to pass a custom HBASE_HOME based classpath.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org