You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by mikewalch <gi...@git.apache.org> on 2017/03/07 15:52:17 UTC
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
GitHub user mikewalch opened a pull request:
https://github.com/apache/accumulo/pull/230
Accumulo-4599 Simplify how accumulo script adds jars to classpath
* Removed ACCUMULO_XTRAJARS variable
* Include '-add' option in accumulo script usage
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mikewalch/accumulo accumulo-4599
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/accumulo/pull/230.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #230
----
commit 00f64a8e48818401543b0a596af3121df4cda60a
Author: Mike Walch <mw...@apache.org>
Date: 2017-03-07T15:29:29Z
Accumulo-4599 Simplify how accumulo script adds jars to classpath
* Removed ACCUMULO_XTRAJARS variable
* Include '-add' option in accumulo script usage
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104786398
--- Diff: start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java ---
@@ -171,22 +171,10 @@ public void run() {
return classpath.toArray(new FileObject[classpath.size()]);
}
- private static ReloadingClassLoader createDynamicClassloader(final ClassLoader parent) throws FileSystemException, IOException {
+ private static ReloadingClassLoader createDynamicClassloader(final ClassLoader parent) throws IOException {
String dynamicCPath = AccumuloClassLoader.getAccumuloString(DYNAMIC_CLASSPATH_PROPERTY_NAME, DEFAULT_DYNAMIC_CLASSPATH_VALUE);
- String envJars = System.getenv("ACCUMULO_XTRAJARS");
- if (null != envJars && !envJars.equals(""))
- if (dynamicCPath != null && !dynamicCPath.equals(""))
- dynamicCPath = dynamicCPath + "," + envJars;
- else
- dynamicCPath = envJars;
-
- ReloadingClassLoader wrapper = new ReloadingClassLoader() {
- @Override
- public ClassLoader getClassLoader() {
- return parent;
- }
- };
+ ReloadingClassLoader wrapper = () -> parent;
--- End diff --
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104792661
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
--- End diff --
What about the case of running a program once and not wanting to change the env script?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104796515
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
- if [[ $2 =~ ^/ ]]; then
- jardir="$(dirname "$2")"
- jarfile="$2"
- else
- jardir="$(pwd)"
- jarfile="${jardir}/${2}"
- fi
- if jar tf "$jarfile" | grep -q META-INF/MANIFEST.MF ; then
- cp="$(unzip -p "$jarfile" META-INF/MANIFEST.MF | grep ^Class-Path: | sed 's/^Class-Path: *//')"
--- End diff --
https://issues.apache.org/jira/browse/ACCUMULO-1494
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r105063169
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
- if [[ $2 =~ ^/ ]]; then
- jardir="$(dirname "$2")"
- jarfile="$2"
- else
- jardir="$(pwd)"
- jarfile="${jardir}/${2}"
- fi
- if jar tf "$jarfile" | grep -q META-INF/MANIFEST.MF ; then
- cp="$(unzip -p "$jarfile" META-INF/MANIFEST.MF | grep ^Class-Path: | sed 's/^Class-Path: *//')"
--- End diff --
The only "standard" I would expect is: 1) set classpath, 2) run java.
I think of anything more as "extra". Other projects may have converged on a de facto standard for those "extras", but I'd prefer we just avoid them entirely whenever possible.
It's the "keep it simple, keep it extensible" principle.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104761530
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
- if [[ $2 =~ ^/ ]]; then
- jardir="$(dirname "$2")"
- jarfile="$2"
- else
- jardir="$(pwd)"
- jarfile="${jardir}/${2}"
- fi
- if jar tf "$jarfile" | grep -q META-INF/MANIFEST.MF ; then
- cp="$(unzip -p "$jarfile" META-INF/MANIFEST.MF | grep ^Class-Path: | sed 's/^Class-Path: *//')"
--- End diff --
I agree, this would be better handled externally.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104793591
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
--- End diff --
It's nice to be able to add a jar quickly to the classpath without modifying accumulo-env.sh like below:
```bash
accumulo -add /path/to/my.jar com.my.class
```
But I guess users could just do the following instead (after your PR is merged)
```bash
CLASSPATH=/path/to/my.jar; accumulo com.my.class
```
In the end, I am ok removing the `-add` and `jar` command if I can rename the current `accumulo classpath` command to be `classpath-info` and create a `accumulo classpath` that just prints the classpath created from accumulo-env.sh so the following works:
```bash
java -cp /path/to/my.jar:$(accumulo classpath) com.my.class
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104800443
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
- if [[ $2 =~ ^/ ]]; then
- jardir="$(dirname "$2")"
- jarfile="$2"
- else
- jardir="$(pwd)"
- jarfile="${jardir}/${2}"
- fi
- if jar tf "$jarfile" | grep -q META-INF/MANIFEST.MF ; then
- cp="$(unzip -p "$jarfile" META-INF/MANIFEST.MF | grep ^Class-Path: | sed 's/^Class-Path: *//')"
--- End diff --
I took a quick look at ACCUMULO-1494 and I think one intention of that change was to allow running a main class specified in a jar.
> @mikewalch has previously suggested that we should easily expose a classpath which a user can use to launch their own processes.
I like that approach. If we make it easy to construct a classpath, then a user can use `java -jar` to run a main class from a manifest file? So it seems like the same functionality can be achieved in a more standard way? I think this would all work, but not completely sure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104797448
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
- if [[ $2 =~ ^/ ]]; then
- jardir="$(dirname "$2")"
- jarfile="$2"
- else
- jardir="$(pwd)"
- jarfile="${jardir}/${2}"
- fi
- if jar tf "$jarfile" | grep -q META-INF/MANIFEST.MF ; then
- cp="$(unzip -p "$jarfile" META-INF/MANIFEST.MF | grep ^Class-Path: | sed 's/^Class-Path: *//')"
--- End diff --
@mikewalch has previously suggested that we should easily expose a classpath which a user can use to launch their own processes. This makes a lot more sense to me than baking this into our own launch scripts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104965493
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
--- End diff --
Create new JIRA issue for modifying `accumulo classpath` command: https://issues.apache.org/jira/browse/ACCUMULO-4604
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104785603
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
--- End diff --
Why not get rid of this `-add` thingy also? It doesn't seem like it's useful if you can set CLASSPATH in the env script.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104950540
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
--- End diff --
I removed the `jar` command in a66cb2888454e9994
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104803563
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
--- End diff --
I like the idea of adding options to the classpath command
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104800761
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
--- End diff --
If removing, It would be useful to look at the patch in ACCUMULO-1494. There may be more that needs to be removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104786227
--- Diff: start/src/main/java/org/apache/accumulo/start/Main.java ---
@@ -212,6 +212,7 @@ public static void printUsage() {
printCommand(executableMap.remove(cmd));
}
System.out.println(" <main class> args Runs Java <main class> located on Accumulo classpath");
+ System.out.println(" Add jars to the classpath using '-add jar1:jar2'");
--- End diff --
I don't think the java code should be documenting the wrapper script, which may or may not be used.
I also think this feature can be removed, since it's useless.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104952630
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
--- End diff --
I removed the `-add` option in a66cb2888454e9994 and documented how users can set the `CLASSPATH` variable before calling accumulo command.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104759592
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
- if [[ $2 =~ ^/ ]]; then
- jardir="$(dirname "$2")"
- jarfile="$2"
- else
- jardir="$(pwd)"
- jarfile="${jardir}/${2}"
- fi
- if jar tf "$jarfile" | grep -q META-INF/MANIFEST.MF ; then
- cp="$(unzip -p "$jarfile" META-INF/MANIFEST.MF | grep ^Class-Path: | sed 's/^Class-Path: *//')"
--- End diff --
Not sure why it was put there. It looks like it's adding the classpath in the manifest to the classpath which is strange. A better way would be to create an uber jar or use `accumulo -add <extra-classpath> <main-class>` to run.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104707884
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
- if [[ $2 =~ ^/ ]]; then
- jardir="$(dirname "$2")"
- jarfile="$2"
- else
- jardir="$(pwd)"
- jarfile="${jardir}/${2}"
- fi
- if jar tf "$jarfile" | grep -q META-INF/MANIFEST.MF ; then
- cp="$(unzip -p "$jarfile" META-INF/MANIFEST.MF | grep ^Class-Path: | sed 's/^Class-Path: *//')"
--- End diff --
what was this for?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104796382
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
--- End diff --
Users can already add items with `export CLASSPATH=...` before running the existing scripts, without editing any files. This isn't new.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104801111
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
--- End diff --
We can add a separate command, but rather than that, I think we should just improve the output of our existing classpath command, so it's in a more usable format, with flags to toggle the classloader hierarchies to include, and with a sensible default.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/230#discussion_r104785958
--- Diff: assemble/bin/accumulo ---
@@ -59,43 +59,22 @@ function main() {
export ACCUMULO_CONF_DIR="$conf"
# Verify setting in accumulo-env.sh
-
: "${JAVA_OPTS:?"variable is not set in accumulo-env.sh"}"
: "${ACCUMULO_LOG_DIR:?"variable is not set in accumulo-env.sh"}"
mkdir -p "${ACCUMULO_LOG_DIR}" 2>/dev/null
- verify_env_dir "ACCUMULO_LOG_DIR" "${ACCUMULO_LOG_DIR}"
verify_env_dir "HADOOP_PREFIX" "${HADOOP_PREFIX}"
verify_env_dir "HADOOP_CONF_DIR" "${HADOOP_CONF_DIR}"
verify_env_dir "ZOOKEEPER_HOME" "${ZOOKEEPER_HOME}"
: "${MALLOC_ARENA_MAX:?"variable is not set in accumulo-env.sh"}"
- # ACCUMULO_XTRAJARS is where all of the commandline -add items go into for reading by accumulo.
- # It also holds the JAR run with the jar command and, if possible, any items in the JAR manifest's Class-Path.
+ CLASSPATH="${lib}/accumulo-start.jar:${conf}:${lib}/slf4j-api.jar:${lib}/slf4j-log4j12.jar:${lib}/log4j.jar:${CLASSPATH}"
+
if [[ "$cmd" = "-add" ]]; then
- export ACCUMULO_XTRAJARS="$2"
+ CLASSPATH="${2}:${CLASSPATH}"
shift 2
- else
- export ACCUMULO_XTRAJARS=""
fi
if [[ "$cmd" = "jar" && -f "$2" ]]; then
--- End diff --
I think this `jar` command can be removed also. It's not useful if it's just yet-another-way to add items to CLASSPATH. Again, we have the env config file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #230: Accumulo-4599 Simplify how accumulo script adds jars to...
Posted by mikewalch <gi...@git.apache.org>.
Github user mikewalch commented on the issue:
https://github.com/apache/accumulo/pull/230
I am going to merge today if there are no objections.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #230: Accumulo-4599 Simplify how accumulo script adds ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/accumulo/pull/230
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---