-
Notifications
You must be signed in to change notification settings - Fork 152
[TRAFODION-2802 ] source the .bashrc before access JAVA_HOME #1413
base: master
Are you sure you want to change the base?
Conversation
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2381/ |
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2381/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 Looks OK to me. Just wondering though: Why was this change needed?
@@ -244,11 +244,14 @@ if [ ! -d ${javapath} ]; then | |||
exit 1 | |||
fi | |||
|
|||
source $HOME/.bashrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -244,11 +244,14 @@ if [ ! -d ${javapath} ]; then | |||
exit 1 | |||
fi | |||
|
|||
source $HOME/.bashrc | |||
javahome=`grep JAVA_HOME ~/.bashrc | wc -l` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above. Writing into the user's .bashrc is probably not a very good idea. You can create a file ~/.trafodion instead.
@DaveBirdsall I build environment on a new server. |
modify for running on a new server