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

Add install from tag #4360

Merged
merged 4 commits into from
Jun 17, 2018
Merged

Add install from tag #4360

merged 4 commits into from
Jun 17, 2018

Conversation

saizai
Copy link
Contributor

@saizai saizai commented Apr 12, 2018

Fixes #4312.

Changes proposed in this pull request:

  • add --tag to rvm install
  • add tag to ruby string

@pkuczynski
Copy link
Member

@mpapis ?

@mpapis
Copy link
Member

mpapis commented Apr 15, 2018

all is fine, but wouldn't it be better to fix rvm_ruby_tag?

@saizai
Copy link
Contributor Author

saizai commented Apr 15, 2018 via email


__rvm_unset_ruby_variables

rvm_ruby_repo_url="${repo_url:-}"
rvm_ruby_repo_branch="${branch_name:-}"
rvm_ruby_name="$ruby_name"
rvm_ruby_repo_tag="${rvm_ruby_repo_tag:-}"
Copy link
Member

Choose a reason for hiding this comment

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

rvm_ruby_repo_tag="${tag_name}"

@@ -248,6 +263,8 @@ __rvm_fetch_from_git_revision_or_sha()
then __from="${rvm_ruby_sha}"
elif [[ -n "${rvm_ruby_repo_branch:-}" ]]
then __from="${rvm_ruby_repo_branch}"
elif [[ -n "${rvm_ruby_repo_tag:-}" ]]
then __from="${rvm_ruby_repo_tag}"
Copy link
Member

Choose a reason for hiding this comment

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

how this is different from the earlier line __from="${rvm_ruby_tag#t}"? why can't we use that?

@saizai
Copy link
Contributor Author

saizai commented May 12, 2018 via email

@mpapis
Copy link
Member

mpapis commented May 12, 2018

but they both end up in the __from variable which controls how we get the source code, I'm just having trouble understanding how it's different if finally they land in exactly the same variable.

@saizai
Copy link
Contributor Author

saizai commented May 14, 2018

__from gets used, here, as git checkout $__from. git checkout works equally well for tags, branches & commit IDs, so I just reused it.

In other parts, it makes a difference — e.g. the gem name & pulling the tag description from git. Or going around the tag-as-in-name lookup and just doing tag-as-in-git.

mpapis
mpapis previously approved these changes Jun 17, 2018
@mpapis mpapis merged commit 8c67f44 into rvm:master Jun 17, 2018
@pkuczynski pkuczynski added this to the rvm-1.29.4 milestone Sep 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants