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

Clarify RUBY_BUILD_ROOT env var #2392

Merged
merged 2 commits into from
May 30, 2024
Merged

Clarify RUBY_BUILD_ROOT env var #2392

merged 2 commits into from
May 30, 2024

Conversation

jasonkarns
Copy link
Member

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: af2df4e

Original phrasing introduced: ff75ca7

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

jasonkarns and others added 2 commits May 24, 2024 19:42
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/`.
It's a feature of questionable utility, it's difficult to describe in documentation, and has been superseded by RUBY_BUILD_DEFINITIONS.
@mislav
Copy link
Member

mislav commented May 29, 2024

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
Copy link
Member Author

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
Copy link
Member Author

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

@mislav
Copy link
Member

mislav commented May 29, 2024

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
Copy link
Member Author

jasonkarns commented May 29, 2024

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
Copy link
Member

eregon commented May 30, 2024

You can see in

IFS=: RUBY_BUILD_DEFINITIONS=($RUBY_BUILD_DEFINITIONS ${RUBY_BUILD_ROOT:-$RUBY_BUILD_INSTALL_PREFIX}/share/ruby-build)
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
Copy link
Member

eregon commented May 30, 2024

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
Copy link
Member

mislav commented May 30, 2024

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.

@mislav mislav merged commit 15bdff9 into master May 30, 2024
6 checks passed
@mislav mislav deleted the readme-rubybuildroot branch May 30, 2024 14:34
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.

RUBY_BUILD_ROOT docs misleading
3 participants