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

Rbenv directory search order with brew #3808

Closed
wants to merge 3 commits into from

Conversation

mmorga
Copy link

@mmorga mmorga commented Apr 24, 2015

The bug is that as-written, rbenv (if installed by brew) always uses the brew prefix directory and the RBENV_ROOT. But the default desired behavior of rbenv is to use the $HOME/.rbenv to store installed rubies. As a result, you get the system ruby only.

My change builds upon the #3275 pull request from @mbologna and reverses the order of the search directories for rbenv to put the brew prefix last.

Since this bug only occurs for brew users, non-OSX users will not be affected.

@ncanceill
Copy link
Contributor

Hi, and thanks for contributing.

You will have to git rebase -i @~2 in order to get rid of the empty merge commit.

Then please find a few people to test and +1 this.

@gchan
Copy link

gchan commented Apr 29, 2015

+1

I haven't tested this but I have encountered the same problem on OSX and brew installed rbenv. Traced the issue with the brew directory taking priority over the $HOME directories for rubies.

@mbologna
Copy link
Contributor

mbologna commented Sep 7, 2015

Is the bug still present? @mmorga @mcornella

@mcornella
Copy link
Member

I have no idea... @gchan ?

@gchan
Copy link

gchan commented Sep 9, 2015

Yes. I just enabled the rbenv plugin and this still happens.

@mbologna
Copy link
Contributor

mbologna commented Sep 9, 2015

Probably this is why I opened #3275

@gchan
Copy link

gchan commented Sep 9, 2015

Yes @mbologna, the code changes in #3275 are very similar to this PR.

Unsure about the reason for line 12 (https://github.com/robbyrussell/oh-my-zsh/pull/3808/files#diff-22fdd1fcfb5b98380f344442393e1f6fL12) however.

@mbologna
Copy link
Contributor

@gchan @mcornella @mmorga please check with #4384 if it solves your problem

@caesarsol
Copy link

@mbologna I don't think #4384 would fix for me, since I have no bin directory in ~/.rbenv/.

I'm getting a bit lost between these PRs...
my edit would just be like the one in your #3275, specifically this line.

@mbologna
Copy link
Contributor

@caesarsol #4384 does not infer anything from ~/.rbenv/bin. Please check the diff.

@caesarsol
Copy link

I'm sorry, isn't the diff only deleting lines 7~10?
I still see $rbenvdir/bin in the file...

@caesarsol
Copy link

@mbologna sorry for the misunderstanding, what I'm saying is:
this PR doesn't solve the issue for me.
I didn't realize you were splitting two different issues.

@mbologna
Copy link
Contributor

@caesarsol this PR is another PR. I did not write it.

@eugenesvk
Copy link

Thank for the fix!
Although in my case I had to make both changes (one by @caesarsol and the other one by @mbologna) to make it work. Whenever I revert to implementing only one of the changes, it all breaks down again.

@robbyrussell robbyrussell added the Status: testers needed Pull Requests that are waiting for testers to merge label Aug 15, 2016
@mcornella mcornella added Area: plugin Issue or PR related to a plugin Status: conflicts Pull Request that has conflicts with the master branch labels Mar 24, 2019
@mcornella mcornella removed the Status: testers needed Pull Requests that are waiting for testers to merge label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: plugin Issue or PR related to a plugin Status: conflicts Pull Request that has conflicts with the master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants