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

Simple, optional tarball cache support #232

Merged
merged 1 commit into from
Nov 13, 2012

Conversation

lloeki
Copy link
Contributor

@lloeki lloeki commented Nov 7, 2012

Rationale

Both in development and in production, some usage patterns of ruby-build
are slowed down by the download phase. In scenarios such as
troubleshooting failing builds or with provisioning situations (chef,
vagrant...) the repeated download is unnerving, bandwidth wasting and
simply against etiquette towards tarball hosters.

It also happens that some source sites happen to be down and in such
cases it is helpful to be able to sideload sources to rbenv.

Behavior

By default nothing changes.

If the variable CACHE_PATH is set, then ruby-build will use that
directory to store a successful download, and will check before
downloading if the tarball is already there, in which case downloading
is skipped.

The file is first downloaded as before in the tmp subdirectory and only
moved afterwards, thus ensuring consistency.

There is no default cache path and the optional variable is to be set by
hand, ensuring people know what they're doing when using ruby-build.

Additionnally, rbenv-install will helpfully set CACHE_PATH if and only
if a RBENV_ROOT/cache directory exists. Again, the directory has to be
created manually.

The CACHE_PATH variable internally ends with a slash to mutualize
non-cached cases. Still, consistency is ensured whether or not a slash
is provided externally.

Notes

I'm not quite sure CACHE_PATH is a good name, maybe
RUBY_BUILD_CACHE_PATH is better and less conflicting.

Rationale:

Both in development and in production, some usage patterns of ruby-build
are slowed down by the download phase. In scenarios such as
troubleshooting failing builds or with provisioning situations (chef,
vagrant...) the repeated download is unnerving, bandwidth wasting and
simply against etiquette towards tarball hosters.

It also happens that some source sites happen to be down and in such
cases it is helpful to be able to sideload sources to rbenv.

Behavior:

By default nothing changes.

If the variable CACHE_PATH is set, then ruby-build will use that
directory to store a successful download, and will check before
downloading if the tarball is already there, in which case downloading
is skipped.

The file is first downloaded as before in the tmp subdirectory and only
moved afterwards, thus ensuring consistency.

There is no default cache path and the optional variable is to be set by
hand, ensuring people know what they're doing when using ruby-build.

Additionnally, rbenv-install will helpfully set CACHE_PATH if and only
if a RBENV_ROOT/cache directory exists. Again, the directory has to be
created manually.

The CACHE_PATH variable internally ends with a slash to mutualize
non-cached cases. Still, consistency is ensured whether or not a slash
is provided externally.

Notes:

I'm not quite sure CACHE_PATH is a good name, maybe
RUBY_BUILD_CACHE_PATH is better and less conflicting.
@sstephenson
Copy link
Contributor

Beautiful patch, and fits right in with the ruby-build ethos. +1 from me.

Implementation-wise, I'd prefer CACHE_PATH to be normalized not to have a trailing slash, because "${CACHE_PATH}/${package_name}.tar.gz" reads clearer to me. But that's just a nit-picking detail.

And yeah, let's go with RUBY_BUILD_CACHE_PATH, to mirror RUBY_BUILD_BUILD_PATH. We'll also need to document it in the readme.

@lloeki
Copy link
Contributor Author

lloeki commented Nov 7, 2012

OK, I'll do the RUBY_BUILD_CACHE_PATH and a bit of readme doc later on.

I initially considered CACHE_PATH trailing slash normalization as you mentioned, as I agree it usually feels more readable, yet in this case it makes the code flow feel hairier, like too nested, when it actually doesn't need to be. Also, it makes duplicate tar or introduce an intermediate variable, again without need. Whatever, if you feel it's better normalizing it without a trailing slash, I'll do the change too.

@sstephenson
Copy link
Contributor

Thanks! Yeah, an intermediate variable sounds best.

@sstephenson sstephenson merged commit 776c6e1 into rbenv:master Nov 13, 2012
@sstephenson
Copy link
Contributor

I went ahead and merged your patch with the changes above. (We still need documentation.) Thank you!

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.

2 participants