rbenv / ruby-build

A tool to download, compile, and install Ruby on Unix-like systems.
https://rbenv.org/man/ruby-build.1
MIT License
3.89k stars 785 forks source link

Clarify RUBY_BUILD_ROOT env var #2392

Closed jasonkarns closed 5 months ago

jasonkarns commented 6 months ago

The RUBY_BUILD_ROOT itself does not default to share/ruby-build/ but rather must point to a directory that has definitions under share/ruby-build/.

The misleading verbiage was introduced: https://github.com/rbenv/ruby-build/commit/af2df4e74a305e4018331531e43e5d9276bb6875

Original phrasing introduced: https://github.com/rbenv/ruby-build/commit/ff75ca72045ce54b4c47d1b5c51e3d921cc63c96

Could still use some rephrasing I think. It's awkward to explain the env var must point to a directory that itself contains a particular path; without implying that the env var itself contains the "share/ruby-build/" path.

fixes #2391

mislav commented 5 months ago

Agreed that RUBY_BUILD_ROOT is confusing and awkward to document. In fact, I do not see a reason to keep promoting this feature considering the existence of RUBY_BUILD_DEFINITIONS. I proposed a commit to soft-deprecate the feature (I do not think we need to deprecate the env var at runtime)

jasonkarns commented 5 months ago

FWIW, I've only recently started using that var. My approach for contributing to rbenv repos has changed over time, but presently I have rbenv and ruby-build installed via homebrew. And then separately (in my ~/code dir), I have them cloned for development/contributing.

One thing that RUBY_BUILD_ROOT makes useful, is the ability to force a homebrew-installed ruby-build to act as if it's running from, say, ~/code/rbenv/ruby-build. Soft-deprecation seems fine for that to continue.

jasonkarns commented 5 months ago

edit as the original PR author, I can't approve. But I do 👍 your changes @mislav .

mislav commented 5 months ago

One thing that RUBY_BUILD_ROOT makes useful, is the ability to force a homebrew-installed ruby-build to act as if it's running from, say, ~/code/rbenv/ruby-build

Could export RUBY_BUILD_DEFINITIONS=~/code/rbenv/ruby-build/share/ruby-build accomplish the same?

jasonkarns commented 5 months ago

I would say not, because the intention when testing things is to operate as if the "working clone" of ruby-build were the only source of definitions. (setting RUBY_BUILD_DEFINITIONS would still include the homebrew-installed ruby-build source, IIRC)

Granted, there are arguably better setups for testing; I'm not holding this approach as a model. But I don't think that appending to the definitions path fits the same use case (depending on what exactly is being tested).

eregon commented 5 months ago

You can see in https://github.com/rbenv/ruby-build/blob/db6c17529bf14de3985c098123345c15aa3d6a8b/bin/ruby-build#L1367 the only usage of RUBY_BUILD_ROOT in code. So it does nothing more than RUBY_BUILD_DEFINITIONS (AFAIK). So I think we should deprecate it and remove it, aka remove it from the README as a way to deprecate.

My approach for contributing to rbenv repos has changed over time, but presently I have rbenv and ruby-build installed via homebrew. And then separately (in my ~/code dir), I have them cloned for development/contributing.

What I do for ruby-build is I have ~/code/ruby-build/bin/ruby-build in PATH, that's much simpler IMO.

eregon commented 5 months ago

Ah, you're right both set of definitions would be available when setting RUBY_BUILD_DEFINITIONS. I think it's much cleaner to use a single clone/installation of ruby-build than trying to mix them though.

mislav commented 5 months ago

Granted, there are arguably better setups for testing; I'm not holding this approach as a model. But I don't think that appending to the definitions path fits the same use case

I'd say: let's keep the current RUBY_BUILD_ROOT feature for use-cases like yours, but instead of better documenting it, lets deprecate it in documentation so that other people don't use it.

I was tempted to remove it from documentation completely, but I think it's better to leave it visible for posterity.