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.

Sign up for GitHub

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

feat: install script for logging on installation #739

Closed
wants to merge 6 commits into from
Closed

feat: install script for logging on installation #739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
270b941
feat: install script for logging on installation
jthegedus Jun 10, 2020
baf85b7
docs: improve install wording
jthegedus Jun 10, 2020
4e37e1e
fix: let git clone create dir
jthegedus Jun 10, 2020
dd7de05
docs: improve output steps in install script log
jthegedus Jun 10, 2020
a646265
fix: log format, git checkout and use $HOME over tilde
jthegedus Jun 10, 2020
e3fa70e
Merge branch 'master' into feat/install-script
jthegedus Jun 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions docs/core-manage-asdf-vm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,12 @@ No match for _Operating System_ and _Installation Method_ selections. Please try

### --Git--

Clone only the latest branch:
Run the install script to `git clone` the latest release:

```shell
git clone https://github.com/asdf-vm/asdf.git ~/.asdf --branch v0.7.8
```

Alternately, you can clone the whole repo and checkout the latest branch:

```shell
git clone https://github.com/asdf-vm/asdf.git ~/.asdf
cd ~/.asdf
git checkout "$(git describe --abbrev=0 --tags)"
curl -o- https://raw.githubusercontent.com/asdf-vm/asdf/v0.7.8/install.bash | bash
# or
wget -qO- https://raw.githubusercontent.com/asdf-vm/asdf/v0.7.8/install.bash | bash
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should adopt this pattern. I know it's popular but it has some risks and I don't think we should push our users in this direction.

See:

Copy link
Contributor Author

@jthegedus jthegedus Jun 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with all of this. Given it is the install method of choice for Homebrew & OMZSH users I didn't think it was too much an ask, especially since others can still just git clone. Additionally, my understanding of prior discussion of package manager code in this repo was that we didn't want to maintain those either.

But, we don't necessarily need to use this method for people who are using the docs site - see #739 (comment)

```

### --Homebrew--
Expand Down
20 changes: 20 additions & 0 deletions install.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash

# Unoffical Bash "strict mode"
# http://redsymbol.net/articles/unofficial-bash-strict-mode/
set -euo pipefail
#ORIGINAL_IFS=$IFS
IFS=$'\t\n' # Stricter IFS settings

asdf_install_dir=${ASDF_INSTALL_DIR:-"$HOME/.asdf"}

git clone https://github.com/asdf-vm/asdf.git "${asdf_install_dir}"
# checkout latest tag in repo in another dir
# credit: https://stackoverflow.com/a/6073628/7911479 and https://stackoverflow.com/a/31811385/7911479
git --git-dir "${asdf_install_dir}/.git" --work-tree "${asdf_install_dir}" checkout "$(git describe --abbrev=0 --tags)" --quiet

printf "\n%s\n" "asdf setup"
printf "%s\t\t\t%s\n" "1: install asdf" "completed!"
printf "%s\t%s\n" "2: add asdf to your shell" "https://asdf-vm.com/#/core-manage-asdf-vm"
printf "%s\t\t\t%s\n" "3: add a plugin" "https://asdf-vm.com/#/core-manage-plugins"
printf "%s\t%s\n" "4: install a tool version" "https://asdf-vm.com/#/core-manage-versions"
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that providing links provides much benefit to the user. Why can't we just tell the user what do to add asdf to their shell? If this script is invoked by the package manager we should be able to tell the user exactly what do to for their specific OS/package manager combination.

Copy link
Contributor Author

@jthegedus jthegedus Jun 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Why can't we just tell the user what do to add asdf to their shell

I think that solution is unmaintainable for us. The number of permutations is making the docs difficult to update as is. nvm does this and their script is 100s of lines of conditional shell scripting which it almost certainly doesn't work in all cases. I'd rather maintain the docs and get users to them instead of automatically editing peoples shell configs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure that providing links provides much benefit to the user

The idea was to capture users who hear about asdf and just run their package manager of choices installation, eg brew install asdf, and haven't ever come to the documentation site. These users have find that it doesn't work, they were given no further instructions, and instead of looking for a documentation site to see if there is something they missed, they check GitHub for issues where people used the same package manager and report they themselves are having issues.

I would like to show these people 2 things:

  1. there's more steps before you can use asdf
  2. these steps are on our documentation site

Copy link
Contributor Author

@jthegedus jthegedus Jun 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Given we don't need to inform users who are installing from the documentation site about the existence of the site, we could leave the docs as is, and only use the install script in Homebrew and co after they clone the repo as they currently do.

8 changes: 4 additions & 4 deletions release/tag.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ if ! git diff-index --cached --exit-code -r --ignore-submodules HEAD -- >&2; the
exit 1
fi

# Update version in README
sed -i.bak "s|^\\(git clone.*--branch \\).*$|\\1$new_tag_name|" README.md
rm README.md.bak
# Update version in install.bash
sed -i.bak "s|^\\(git clone.*--branch \\).*$|\\1$new_tag_name|" install.bash
rm install.bash.bak

# Update version in docs/core-manage-asdf-vm.md
sed -i.bak "s|^\\(git clone.*--branch \\).*$|\\1$new_tag_name|" docs/core-manage-asdf-vm.md
sed -i.bak "s~\\(https://raw.githubusercontent.com/asdf-vm/asdf/\\).*/install.bash | bash~\\1${VERSION}/install.bash | bash~" docs/core-manage-asdf-vm.md
rm docs/core-manage-asdf-vm.md.bak

# Update version in the VERSION file
Expand Down