NVIDIA-AI-IOT / tf_trt_models

TensorFlow models accelerated with NVIDIA TensorRT
BSD 3-Clause "New" or "Revised" License
686 stars 241 forks source link

Don't use `sudo` during installation #17

Closed cosama closed 6 years ago

cosama commented 6 years ago

I just had to cleaning up my system directories after calling the install.sh script without looking into what it actually does. I had everything setup in a virtualenv and assumed it would just go there, I almost got a heart attack when I saw all the sudo calls in the script.

Please remove them in the installation script, it is bad practice and if a person calls something like sudo apt-get install just before calling the script it doesn't even tell the user what just happened. I think using the --user flag instead would be okay.

Also the sudo calls in install_protoc.sh are dangerous. If there is no other way I think ~/.local/bin would be a more appropriate location.

If this is not a solution please add a warning message above the installation instruction, best in capital red letters!!!

ghost commented 6 years ago

Hi cosama. Thanks for the feedback.

I agree with your suggestions.

  1. A local installation of protocol buffers under this repository (as it is used only to install the tensorflow object detection package)
  2. User level python package installations.

Will work on integrating these changes.

Thanks.

ghost commented 6 years ago

I just merged the changes into the master branch, going to close this for now.

Thanks for raising this issue, and sorry if it caused any inconvenience! Please feel to raise another issue if you have any other suggestions related to this repository.

Thanks!

cosama commented 6 years ago

Thanks for dealing with this so promptly. I will make sure later that everything is still working. Great job.

cosama commented 6 years ago

Just had a look at this and it seems to work just fine.

I'm not sure if I should be opening a new issue for this. I think the following would be the best solutions:

#!/bin/bash

INSTALL_PROTOC=$PWD/scripts/install_protoc.sh
MODELS_DIR=$PWD/third_party/models

PYTHON=python

if [ $# -eq 1 ]; then
  PYTHON=$1
fi

echo $PYTHON

# install protoc
echo "Downloading protoc"
source $INSTALL_PROTOC
PROTOC=$PWD/data/protoc/bin/protoc

# install tensorflow models
git submodule update --init

pushd $MODELS_DIR/research
echo $PWD
echo "Installing object detection library"
echo $PROTOC
$PROTOC object_detection/protos/*.proto --python_out=.
$PYTHON setup.py install $@
popd

pushd $MODELS_DIR/research/slim
echo $PWD
echo "Installing slim library"
$PYTHON setup.py install $@
popd

echo "Installing tf_trt_models"
echo $PWD
$PYTHON setup.py install $@

People could then call: 1) sudo ./install.sh 2) ./install.sh 3) ./install.sh --user

and it would work for all these situations.