pyenv / pyenv-installer

This tool is used to install `pyenv` and friends.
MIT License
3.96k stars 430 forks source link

fix error about wrong command #68

Closed lsflling closed 4 years ago

lsflling commented 6 years ago
joshfriend commented 6 years ago

Why did you close #67? It was exactly the same as this PR

obestwalter commented 5 years ago

Hi @lsflling What is this fixing? I don't get this error and there is no bug open about this.

lsflling commented 5 years ago

@obestwalter I got the error message. $(COMMAND) means make the output of the COMMAND as another COMMAND. The output of "git clone "$1"" is Cloning into ''''pyenv'''...'.

obestwalter commented 5 years ago

ah, ok - I had a proper look now what this is even about. It's about the offline installer, which I never used yet (because I didn't even know it exists and it is not also not documented :laughing:).

So the normal installer is not working for you and you want to do an offline install ...)

About the error and the fix: I don't quite understand why the code is that way, but I would propose to fix it by simplifiying it a bit more, so that the script crashes immediately with a helpful error. I would suggest something like this:

#!/usr/bin/env bash

set -e

if [[ -z "$PYENV_PACKAGE_ARCHIVE" ]]; then
  PYENV_PACKAGE_ARCHIVE="$(cd $(dirname "$0") && pwd)/pyenv-package.tar.gz"
fi

if [[ -n "${USE_HTTPS}" ]]; then
  GITHUB="https://github.com"
else
  GITHUB="git://github.com"
fi

TMP_DIR=$(mktemp -d)
cd ${TMP_DIR}

git clone "${GITHUB}/pyenv/pyenv.git"            "$TMP_DIR"
git clone "${GITHUB}/pyenv/pyenv-doctor.git"     "$TMP_DIR"
git clone "${GITHUB}/pyenv/pyenv-installer.git"  "$TMP_DIR"
git clone "${GITHUB}/pyenv/pyenv-update.git"     "$TMP_DIR"
git clone "${GITHUB}/pyenv/pyenv-virtualenv.git" "$TMP_DIR"
git clone "${GITHUB}/pyenv/pyenv-which-ext.git"  "$TMP_DIR"

tar -zcf "$PYENV_PACKAGE_ARCHIVE" -C "$TMP_DIR" .

rm -rf ${TMP_DIR}

Maybe the original contributor - @huntzhan - can also chip in?

If you like you could update your PR and maybe even add some information about the offline installer in the README?

obestwalter commented 5 years ago

Hi @lsflling, are you still interested in fixing this?

obestwalter commented 4 years ago

Let's just merge this simple fix for now then. Thanks @lsflling