New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issues found while trying to run download-env #373
Conversation
Can you do a |
@@ -31,6 +31,20 @@ if [ ! -z "$SETTINGS_FILE" -o ! -z "$XILINX" ]; then | |||
exit 1 | |||
fi | |||
|
|||
# Conda does not support ' ' in the path (it bails early). | |||
if echo "${SETUP_DIR}" | grep -q ' '; then |
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.
Can you indent this to match the existing code?
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.
Fixed.
scripts/download-env.sh
Outdated
./Miniconda3-latest-Linux-x86_64.sh -p $CONDA_DIR -b | ||
# -p to specify the install location | ||
# -b to enable batch mode (no prompts) | ||
# -f to not return an error if the location specified by -p already exists |
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.
Indenting.
scripts/download-env.sh
Outdated
cd $BUILD_DIR | ||
wget --no-verbose -c https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh | ||
wget --no-verbose --continue https://repo.continuum.io/miniconda/Miniconda3-latest-Linux-x86_64.sh |
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.
Can you add a # FIXME: Get the miniconda people to add a "self check" mode
scripts/download-env.sh
Outdated
echo "You appear to have ':' characters in the path to this script." | ||
echo "Please move this repository to another path that does not contain this character." | ||
exit 1 | ||
fi |
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.
Could you add these checks to enter-env.sh
as well? You should be able to do; vimdiff download-env.sh enter-env.sh
and see how they are mostly trying to be the same.
This allows qemu to be build on arch linux, it should not break other systems where this is the default.
We now ensure that a conda binary actually exists, rather than just the directory we asked it to be installed in. The install script will now attempt to install conda on subsequent runs, if a previous install attempt failed.
Provide the user with a "nice" error message if the repository exists in a directory that is known to not work (ones with whitespace or with colon's in the path).
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.
Rebased - removed the linux related commit that I pulled from your repo (you can pull that in yourself if you want it).
Looks great! |
Fix some of the issues I found while trying to run the linux image on qemu.
Specifically: