Skip to content
This repository has been archived by the owner on Feb 9, 2023. It is now read-only.

Remove hardcoded path references #212

Open
wants to merge 1 commit into
base: aiyprojects
Choose a base branch
from

Conversation

RichardBronosky
Copy link

@RichardBronosky RichardBronosky commented Dec 27, 2017

Remove the assumed /home/pi/AIY-projects-python project path. This is
easily done because of the previous cd call changes the CWD to the
project path. This change is needed because of conflicting instructions
in HACKING.md This resolves
#211

Remove the assumed /home/pi/AIY-projects-python project path. This is
easily done because of the previous cd call change the CWD to the
project path. This changed is needed because of conflicting instructions
in HACKING.md This resolves
google#211
@ameer1234567890
Copy link
Contributor

This would not work very well in the distant case where a user runs the install-deps.sh while in the home directory or anywhere else for that matter. Ideally we should not be assuming anything here. A better approach would be to hard code a path somewhere in the root of the repo and the use references to that everywhere.

@RichardBronosky
Copy link
Author

@ameer1234567890 look at line 32. Where the script is run from is irrelevant. The script cds into the cloned project root. https://github.com/google/aiyprojects-raspbian/pull/212/files#diff-68a15966113a64d31b3e52ddc4c93227L32

Copy link
Member

@drigz drigz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I've been trying to avoid assuming /home/pi as it breaks DietPi.

@@ -32,8 +32,7 @@ sudo pip3 install --upgrade pip virtualenv
cd "${scripts_dir}/.."
virtualenv --system-site-packages -p python3 env
env/bin/pip install -r requirements.txt
echo "/home/pi/AIY-projects-python/src" > \
/home/pi/AIY-projects-python/env/lib/python3.5/site-packages/aiy.pth
echo "$(pwd)/src" > ./env/lib/python3.5/site-packages/aiy.pth
Copy link
Member

@drigz drigz Jan 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$PWD is not guaranteed to be the the repository directory. Could you change it to use "${scripts_dir}/.." instead?

Just saw the cd above. Forget this comment :)

@drigz
Copy link
Member

drigz commented Jan 4, 2018

@protovist, shall we wait until after the next image release to merge this, just in case it breaks something?

@mbrooksx
Copy link
Contributor

@RichardBronosky can you rebase with the latest changes we've submitted to the aiyprojects branch? Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants