Skip to content
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

update install script to better support non-PATH installations #1184

Merged
merged 10 commits into from
Aug 16, 2021

Conversation

williamboman
Copy link
Contributor

No description provided.

install_linux.sh Outdated
SUDO="sudo";
fi


$SUDO mkdir -p "$dest"
$SUDO install -b -c -v /tmp/tflint "$dest"
$SUDO install --backup=none -c -v /tmp/tflint "$dest"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps scope creeping a but - but I suggest we change this to not create any backup, to avoid polluting the PATH. What happens now if you run the install script more than once is that you'll end up with a tflint and tflint~ (the backup) in in your install destination.

install_linux.sh Outdated
@@ -68,13 +70,13 @@ else
dest="${TFLINT_INSTALL_PATH:-/usr/local/bin}/"
echo "Installing /tmp/tflint to ${dest}..."

if [[ "$(id -u)" == 0 ]]; then SUDO=""; else
if [[ "$(id -u)" == 0 ]] || [[ $TFLINT_INSTALL_NO_ROOT == 1 ]]; then SUDO=""; else
Copy link
Member

Choose a reason for hiding this comment

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

Seems like the script should just test whether it can access the directory as the non-root user and use sudo if not rather than adding an option

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call 👍 a80c763

install_linux.sh Outdated
@@ -4,6 +4,8 @@ processor=$(uname -m)

if [ "$processor" == "x86_64" ]; then
arch="amd64"
elif [ "$processor" == "arm64" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

If you could submit this separately happy to merge it right away

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 #1185

@bendrucker bendrucker merged commit fff9931 into terraform-linters:master Aug 16, 2021
@williamboman williamboman deleted the install-script branch August 17, 2021 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants