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

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

wants to merge 6 commits into from

Conversation

jthegedus
Copy link
Contributor

@jthegedus jthegedus commented Jun 10, 2020

Summary

Use an install script to perform the git clone installation process. This allows us to log on installation which is required for the Homebrew installation as outlined in #738 (Homebrew users expect brew install asdf to perform the complete setup of asdf and subsequently report a GitHub issue before going to the docs site to see further instructions)

Fixes: #738

Other Information

  • update tag regex (probably can be improved as I'm no sed expert)
  • we no longer have instructions in README so deprecate that tag update
  • change instructions to curl/wget the install script from the GitHub remote
  • use --git-dir and --work-tree to avoid filesystem traversal as git provides this functionality

Notes

  • the git checkout uses --quiet flag to supress "detached head" warning on tag checkout. Perhaps we let it log this?
  • potentially could reuse ASDF_DATA_DIR instead of introducing another env var

Testing output:

with --quiet:

asdf on  feat/install-script [!] 
➜ ./install.bash        
Cloning into '/home/jthegedus/projects/test'...
remote: Enumerating objects: 31, done.
remote: Counting objects: 100% (31/31), done.
remote: Compressing objects: 100% (26/26), done.
remote: Total 5350 (delta 12), reused 13 (delta 5), pack-reused 5319
Receiving objects: 100% (5350/5350), 1007.11 KiB | 808.00 KiB/s, done.
Resolving deltas: 100% (3010/3010), done.

asdf setup
1: install asdf                 completed!
2: add asdf to your shell       https://asdf-vm.com/#/core-manage-asdf-vm
3: add a plugin                 https://asdf-vm.com/#/core-manage-plugins
4: install a tool version       https://asdf-vm.com/#/core-manage-versions

without --quiet:

asdf on  feat/install-script [!] 
➜ ./install.bash        
Cloning into '/home/jthegedus/projects/test'...
remote: Enumerating objects: 31, done.
remote: Counting objects: 100% (31/31), done.
remote: Compressing objects: 100% (26/26), done.
remote: Total 5350 (delta 12), reused 13 (delta 5), pack-reused 5319
Receiving objects: 100% (5350/5350), 1007.11 KiB | 808.00 KiB/s, done.
Resolving deltas: 100% (3010/3010), done.
Note: switching to 'v0.7.8'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 4a3e3d6 Update version to 0.7.8

asdf setup
1: install asdf                 completed!
2: add asdf to your shell       https://asdf-vm.com/#/core-manage-asdf-vm
3: add a plugin                 https://asdf-vm.com/#/core-manage-plugins
4: install a tool version       https://asdf-vm.com/#/core-manage-versions

@jthegedus jthegedus requested a review from a team as a code owner June 10, 2020 07:57
@jthegedus jthegedus self-assigned this Jun 10, 2020
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

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)

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

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

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.

@jthegedus
Copy link
Contributor Author

jthegedus commented Jun 25, 2020

I will create an Issue in the repo to pin for Homebrew users to direct them to the documentation. As most Google the issue and end up finding old Issues and resolving the problem via an old method.

Edit: added with #785

@jthegedus jthegedus closed this Jun 25, 2020
@jthegedus jthegedus deleted the feat/install-script branch August 22, 2020 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installation script directing users to doc site
2 participants