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

Work correctly on jruby and when ruby is installed under another name #4

Closed
wants to merge 1 commit into from

Conversation

jeremyevans
Copy link

Borrow the code rake uses for FileUtils::RUBY to get the path to
the current ruby binary.

Force the mt argument to be an array of strings, as shelljoin does
not work if an element of the array is an integer on JRuby.

Borrow the code rake uses for FileUtils::RUBY to get the path to
the current ruby binary.

Force the mt argument to be an array of strings, as shelljoin does
not work if an element of the array is an integer on JRuby.
@zenspider
Copy link
Contributor

  1. Where is this RUBY constant defined?
  2. You're saying that jruby's shellword's shelljoin doesn't to_s? seems like a bug.

@zenspider
Copy link
Contributor

Verified it's a bug. Filing a ticket on jruby.

@zenspider
Copy link
Contributor

I'm not comfortable with the RUBY bit yet. The rest makes sense for the jruby bug.

10004 % ruby -rshellwords -e 'p RUBY'
-e:1:in `<main>': uninitialized constant RUBY (NameError)

@jeremyevans
Copy link
Author

The RUBY implementation I borrowed from rake is located at https://github.com/ruby/rake/blob/master/lib/rake/file_utils.rb#L9

I just copied the code and placed it inside Minitest::Bisect: https://github.com/seattlerb/minitest-bisect/pull/4/files#diff-ef8f6d7f169542f4c0a72cc6b77c42e7R12

Even though the JRuby bug may be fixed in JRuby's master branch, it still exists in the latest official release, so it may be worth including the workaround so that people using JRuby 1.6 or 1.7 can still use minitest-bisect.

@zenspider
Copy link
Contributor

NOPE. Ignore me. The patch partially rejected because I applied your previous patch and the hunk was in-between. Found the reject, carved it up and manually applied it.

@zenspider
Copy link
Contributor

merged. Thanks!

@zenspider zenspider closed this Apr 27, 2015
@minitest minitest locked and limited conversation to collaborators Jan 24, 2021
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.

None yet

2 participants