Skip to content
This repository was archived by the owner on Apr 14, 2021. It is now read-only.

Add support for TruffleRuby#6693

Merged
bundlerbot merged 8 commits intorubygems:masterfrom
eregon:truffleruby
Sep 11, 2018
Merged

Add support for TruffleRuby#6693
bundlerbot merged 8 commits intorubygems:masterfrom
eregon:truffleruby

Conversation

@eregon
Copy link
Copy Markdown
Contributor

@eregon eregon commented Sep 9, 2018

Hello,
This PR adds support for TruffleRuby in Bundler.

I searched for rbx and jruby (case-insensitive) through the codebase to find all places which need changes and where tests use specific Ruby implementations. So hopefully this PR is complete, but please tell me if I missed something.

I'd also like to test TruffleRuby in Bundler's CI, but will do this as a separate PR, as we first need the next release of TruffleRuby so that it includes the fix for truffleruby/truffleruby#1413.

@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Sep 9, 2018

This should also be backported to 1.16.x.

when "jruby"
JRUBY_VERSION
else
raise BundlerError, "That RUBY_ENGINE is not recognized"
Copy link
Copy Markdown
Member

@colby-swandale colby-swandale Sep 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woo! This is awesome! 🎉

My first question though, why are you modifying this behavior instead of adding to it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might also want to check that RUBY_ENGINE_VERSION is indeed defined? I'm not sure if all Rubies define it

Copy link
Copy Markdown
Contributor Author

@eregon eregon Sep 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking was rather than adding a special case for truffleruby I'd make it general.
Every not ancient (=1.8) Ruby version should define RUBY_ENGINE_VERSION, it's a standard constant like RUBY_ENGINE, RUBY_VERSION, etc.
I can add a defined? if it causes a problem for an implementation but it seems unlikely as currently this code path was hardcoded to only support MRI, JRuby and Rubinius.

Copy link
Copy Markdown
Contributor Author

@eregon eregon Sep 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colby-swandale What do you think? Is this code fine as-is or should I change it to something else?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with the PR, so let's merge it

when "jruby"
JRUBY_VERSION
else
raise BundlerError, "That RUBY_ENGINE is not recognized"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might also want to check that RUBY_ENGINE_VERSION is indeed defined? I'm not sure if all Rubies define it

@colby-swandale colby-swandale added this to the 1.16.5 milestone Sep 10, 2018
@colby-swandale
Copy link
Copy Markdown
Member

@bundlerbot r+

@bundlerbot
Copy link
Copy Markdown
Collaborator

📌 Commit 6a7b2c1 has been approved by colby-swandale

@bundlerbot
Copy link
Copy Markdown
Collaborator

⌛ Testing commit 6a7b2c1 with merge 7a33207...

bundlerbot added a commit that referenced this pull request Sep 10, 2018
Add support for TruffleRuby

Hello,
This PR adds support for TruffleRuby in Bundler.

I searched for `rbx` and `jruby` (case-insensitive) through the codebase to find all places which need changes and where tests use specific Ruby implementations. So hopefully this PR is complete, but please tell me if I missed something.

I'd also like to test TruffleRuby in Bundler's CI, but will do this as a separate PR, as we first need the next release of TruffleRuby so that it includes the fix for truffleruby/truffleruby#1413.
@deivid-rodriguez
Copy link
Copy Markdown
Contributor

@eregon This is great, thanks!

This PR I opened to rubygems a while ago is somehow related. Maybe you want to have a look since you have experience accross several implementations and the constants they define.

@bundlerbot
Copy link
Copy Markdown
Collaborator

💥 Test timed out

@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Sep 11, 2018

@bundlerbot timed out, should it be retried or is there an issue with the PR?

@colby-swandale
Copy link
Copy Markdown
Member

@bundlerbot retry

@bundlerbot
Copy link
Copy Markdown
Collaborator

⌛ Testing commit 6a7b2c1 with merge 368d759...

bundlerbot added a commit that referenced this pull request Sep 11, 2018
Add support for TruffleRuby

Hello,
This PR adds support for TruffleRuby in Bundler.

I searched for `rbx` and `jruby` (case-insensitive) through the codebase to find all places which need changes and where tests use specific Ruby implementations. So hopefully this PR is complete, but please tell me if I missed something.

I'd also like to test TruffleRuby in Bundler's CI, but will do this as a separate PR, as we first need the next release of TruffleRuby so that it includes the fix for truffleruby/truffleruby#1413.
@bundlerbot
Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-travis
Approved by: colby-swandale
Pushing 368d759 to master...

@bundlerbot bundlerbot merged commit 6a7b2c1 into rubygems:master Sep 11, 2018
@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Sep 11, 2018

Thanks for review and merging!

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Sep 12, 2018

This should also be backported to 1.16.x.

This pull-request is not bugfix. I think we need to bump 1.17.x maybe.

Can we merge non-bugfix feature with tiny versionup?

@colby-swandale
Copy link
Copy Markdown
Member

@hsbt We have allowed non-bugfixes to go into minor releases before if it made sense. And this isn't a huge change - so i'm happy to backport this into the next patch release.

@eregon
Copy link
Copy Markdown
Contributor Author

eregon commented Sep 12, 2018

1.16.x or 1.17.x is fine, what TruffleRuby needs is a (non-pre) release of Bundler so the patches in TruffleRuby can be removed and TruffleRuby can use vanilla Bundler.

Is there a schedule for the next Bundler release?
The next release of TruffleRuby has feature freeze on September 21.
That might be quite close, but it would be ideal so there is no overlap between patches in TruffleRuby and support for TruffleRuby in Bundler.

@colby-swandale
Copy link
Copy Markdown
Member

There is no scheduled time for a release, but i will try and get your changes released before the 21st.

colby-swandale pushed a commit that referenced this pull request Sep 14, 2018
Add support for TruffleRuby

Hello,
This PR adds support for TruffleRuby in Bundler.

I searched for `rbx` and `jruby` (case-insensitive) through the codebase to find all places which need changes and where tests use specific Ruby implementations. So hopefully this PR is complete, but please tell me if I missed something.

I'd also like to test TruffleRuby in Bundler's CI, but will do this as a separate PR, as we first need the next release of TruffleRuby so that it includes the fix for truffleruby/truffleruby#1413.

(cherry picked from commit 368d759)
colby-swandale pushed a commit that referenced this pull request Sep 18, 2018
* 1-16-stable:
  Version 1.16.5 with changelog
  scope TruffleRuby platform specs to be RubyGems >= 2.1.0
  Auto merge of #6689 - bundler:colby/fix-bundler-load-error, r=colby-swandale
  Auto merge of #6695 - bundler:segiddins/6684-gvp-prefer-non-pres, r=colby-swandale
  Auto merge of #6693 - eregon:truffleruby, r=colby-swandale
  Auto merge of #6692 - eregon:simplify-autoload-require-deprecate, r=segiddins
  Auto merge of #6688 - voxik:check-search, r=colby-swandale
  Auto merge of #6682 - bundler:bundle_env_formatting, r=colby-swandale
  Auto merge of #6675 - MaxLap:master, r=greysteil
  Auto merge of #6669 - ChrisBr:fix_dep_proxy, r=segiddins
  Auto merge of #6664 - greysteil:avoid-printing-git-error, r=colby-swandale
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants