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

Open3#popen3 should use leading Hash as additional environment variables #1668

Closed
wants to merge 1 commit into from

Conversation

atambo
Copy link
Member

@atambo atambo commented Apr 27, 2014

Fixes #1547
Fixes #1290

@atambo
Copy link
Member Author

atambo commented Apr 29, 2014

This breaks some tests in test/externals/ruby1.9/test_open3.rb 😠

@enebo
Copy link
Member

enebo commented May 6, 2014

@atambo You still figuring out those new failed tests in test_open3.rb?

@atambo
Copy link
Member Author

atambo commented May 7, 2014

Ya, I'll give it another shot soonish.

@enebo
Copy link
Member

enebo commented Jun 14, 2014

@headius Is this something @atambo should still work on with your mega branch of IO rework? I do not think we have plans to backport that to 1.7 but you might have some partial backports for logic like this?

@enebo enebo added this to the JRuby 1.7.17 milestone Oct 6, 2014
@enebo
Copy link
Member

enebo commented Oct 6, 2014

@atambo I probably derailed this issue by asking about master. 1.7.x is long-lived and this is causing enough real-world problems to need fixing.

@headius
Copy link
Member

headius commented Nov 2, 2014

JRuby master incorporates an unmodified open3 from MRI, since we now have better support for the features involved. We can still patch for 1.7.17 if this patch is still valid or can be updated.

@headius
Copy link
Member

headius commented Dec 2, 2014

I'm on it.

@headius
Copy link
Member

headius commented Dec 2, 2014

I have a lighter-weight fix that appears to be working. Will push shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants