You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by xiaozhongwang <gi...@git.apache.org> on 2018/01/24 07:29:00 UTC
[GitHub] trafodion pull request #1413: [TRAFODION-2802 ] source the .bashrc before ac...
GitHub user xiaozhongwang opened a pull request:
https://github.com/apache/trafodion/pull/1413
[TRAFODION-2802 ] source the .bashrc before access JAVA_HOME
modify for running on a new server
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xiaozhongwang/trafodion TRAFODION-2802
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1413.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 #1413
----
commit d593e31c85a6ce9922948a79824b8e4de28145cf
Author: wxzkenny <wx...@...>
Date: 2018-01-24T07:25:27Z
source the .bashrc before access JAVA_HOME
----
---
[GitHub] trafodion pull request #1413: [TRAFODION-2802 ] source the .bashrc before ac...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1413#discussion_r163951143
--- Diff: install/traf_checkset_env.sh ---
@@ -244,11 +244,14 @@ if [ ! -d ${javapath} ]; then
exit 1
fi
+source $HOME/.bashrc
javahome=`grep JAVA_HOME ~/.bashrc | wc -l`
if [ "x${javahome}" = "x0" ]; then
echo -en "\n# Added by traf_checkset_env.sh of trafodion\n" >> $HOME/.bashrc
echo -en "export JAVA_HOME=${javapath}\n" >> $HOME/.bashrc
echo -en "export PATH=\$PATH:\$JAVA_HOME/bin\n" >> $HOME/.bashrc
--- End diff --
See comment above. Writing into the user's .bashrc is probably not a very good idea. You can create a file ~/.trafodion instead.
---
[GitHub] trafodion pull request #1413: [TRAFODION-2802 ] source the .bashrc before ac...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1413#discussion_r163950904
--- Diff: install/traf_checkset_env.sh ---
@@ -244,11 +244,14 @@ if [ ! -d ${javapath} ]; then
exit 1
fi
+source $HOME/.bashrc
javahome=`grep JAVA_HOME ~/.bashrc | wc -l`
--- End diff --
See comment above
---
[GitHub] trafodion pull request #1413: [TRAFODION-2802 ] source the .bashrc before ac...
Posted by xiaozhongwang <gi...@git.apache.org>.
Github user xiaozhongwang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1413#discussion_r164020994
--- Diff: install/traf_checkset_env.sh ---
@@ -244,11 +244,14 @@ if [ ! -d ${javapath} ]; then
exit 1
fi
+source $HOME/.bashrc
--- End diff --
Before, we must prepare environment step by step as following document: [https://cwiki.apache.org/confluence/display/TRAFODION/Create+Build+Environment]
I only write them in one command.
Yes. I think this is not a good way too.
---
[GitHub] trafodion pull request #1413: [TRAFODION-2802 ] source the .bashrc before ac...
Posted by zellerh <gi...@git.apache.org>.
Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1413#discussion_r163950816
--- Diff: install/traf_checkset_env.sh ---
@@ -244,11 +244,14 @@ if [ ! -d ${javapath} ]; then
exit 1
fi
+source $HOME/.bashrc
--- End diff --
Sourcing ~/.bashrc and looking into this file is not a very good solution, IMHO. There are so many other ways we can set JAVA_HOME. For example: /etc/bashrc, ~/.trafodion, $TRAF_HOME/.sqenvcom.sh.
I would suggest to just check whether JAVA_HOME is set, otherwise duplicate the code in $TRAF_HOME/.sqenvcom.sh that sets JAVA_HOME. If JAVA_HOME is not already set, then you can then add a line
export JAVA_HOME=...
To the file ~/.trafodion.
---