[bootstrap] Set JAVA_HOME and PATH for building zookeeper

[brirams]

While trying to reproduce #4870, I was unable to setup a vagrant environment from a fresh master without setting these two environment variables since ant depends on them.

[deepthi]

I don't understand this. Does the Vagrant setup even call bootstrap.sh?
I would have thought that any changes for vagrant need to go into vagrant/bootstrap_vm.sh.

[rafael]

@deepthi - vagrant/bootstrap_vm.sh is to provision the vagrant machine. Once the machine is provisioned, it uses this script. In a way, one the Vagrant setup does is automated the instructions provided in:

https://vitess.io/docs/tutorials/local/

[rafael]

@brirams , could you fix the DCO?

[deepthi]

@brirams when you fix the DCO, also fix the typo in the commit message: any -> ant

[deepthi]

@deepthi - vagrant/bootstrap_vm.sh is to provision the vagrant machine. Once the machine is provisioned, it uses this script. In a way, one the Vagrant setup does is automated the instructions provided in:

https://vitess.io/docs/tutorials/local/

Ok, now I see the call to bootstrap.sh inside build.sh. I missed it at first glance.
Won't this break anyone who is running a different version of the jvm or has it installed in a different place? In which case it should go into build.sh before the call to bootstrap.sh so that it is done only for the vagrant build.

[st-cyrill]

Same problem.
I think it should go into vitess/vagrant-scripts/vitess/build.sh before call ./bootstrap.sh

export JAVA_HOME=$(readlink -f /usr/bin/javac | sed "s@/bin/javac@@")

It works ok.

[rafael]

@deepthi - How about we don't set it if it's already set in the path? That should cover both cases.

[deepthi]

@deepthi - How about we don't set it if it's already set in the path? That should cover both cases.

If I understand correctly, you are suggesting that if java is already in the path, then you don't set JAVA_HOME? That should work. Though it would be preferable to isolate this in the vagrant setup.

[dkhenry]

This will break anyone who isn't using Java-7

[brirams]

yup -- will give this another look this week. thx!

[deepthi]

@brirams can this be closed since #4995?

[brirams]

yes! zookeeper now builds correctly but the vagrant start script fails because it's trying to use the etcd topo instead of zk2, as it's been configured. Think that broke here: #5003. I'll close this out and create another PR with the appropriate fix. thanks!

edit: #5029