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

Filter redundant/incompatible rvm_gem_options #4705

Merged

Conversation

felixbuenemann
Copy link
Contributor

This removes any occurrence of --no-document, --no-ri and --no-rdoc from rvm_gem_options when merging with the auto-detected install_params to avoid duplicate, deprecated or incompatible options.

This should fix the current test failures for ruby-1.8.7 under Xenial on Travis CI, where the default Travis ~/.rvmrc contains rvm_gem_options='--no-ri --no-rdoc --no-document' breaking rvm gem installs with rubygems 1.8.25 which does not yet support the --no-document option.

@felixbuenemann
Copy link
Contributor Author

While it would be possible to fix the rvm CI failures simply by patching the existing ~/.rvmrc, I chose to fix this in rvm instead, since it's likely to help other rvm users.

The regex is fairly defensive, to guard against things like --no-rdoc-foo, but has no guards at the beginning of the string due to weird differences in how BSD and GNU sed handle matching. However unless some gem requires an absurd flag like --not-posix-style--no-document that shouldn't really be a problem.

A valid case that we wouldn't catch is rvm_gem_options with embedded quoting around params:

rvm_gem_options='"--no-rdoc" "--no-ri"'

That's agaib pretty unlikely to be used in rvmrc and IMHO not worth the trouble (especially given the limitations and portability of sed's extended REs).

@felixbuenemann
Copy link
Contributor Author

The remaining test failure is fixed by #4704.

scripts/functions/gemset Outdated Show resolved Hide resolved
This removes any occurrence of --no-document, --no-ri and --no-rdoc from
rvm_gem_options when merging with auto-detected install_params to avoid
duplicate or incompatible options (eg. --no-document with rubygems 1.8).
@felixbuenemann felixbuenemann force-pushed the autofix-incompatible-rvm-gem-options branch from 76046c0 to a788056 Compare May 28, 2019 21:03
@pkuczynski
Copy link
Member

Good work, thanks!

@pkuczynski pkuczynski merged commit 303a3ed into rvm:master May 28, 2019
@felixbuenemann felixbuenemann deleted the autofix-incompatible-rvm-gem-options branch May 28, 2019 23:16
@pkuczynski pkuczynski added this to the rvm-1.29.9 milestone Jun 19, 2019
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.

None yet

3 participants