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

Remove ffi conditional install on Windows platforms #160

Merged
merged 4 commits into from
Sep 20, 2019

Conversation

sds
Copy link
Collaborator

@sds sds commented Sep 19, 2019

This functionality has caused more problems than it's solved.

From the original issue in #158, we're removing this logic for the following reasons:

  1. Regardless of platform, you always see a Building native extensions. This could take a while... message when this is not actually the case. ChildProcess is just piggybacking off of the RubyGems extension facilities to conditionally install a gem. This is confusing.
  2. When installing ffi via this workaround, it isn't clear to the user that it is happening. You see the aforementioned Building native extensions message, but aren't told that the ffi gem was installed. Again, this is potentially confusing, but also deceptive in that it hides the dependency.
  3. The workaround requires us to specify a dependency constraint that doesn't respect the constraints defined in the user's own gem dependencies. Users then need to align their dependency constraints with what we've defined in mkrf_conf.rb, duplicating them in their Gemfile/etc. See cannot load such file -- ffi on windows #150 for an example where this confusion occurs.
  4. It introduces a dependency on the rake gem (see Fix #143 - Childprocess v1.0.0 failing to install. #144), unnecessarily coupling us to Rake's release cycle. See Relax rake gem constraint from <=11.x to <=12.x. #147 for an example where we had to cut a new release when Rake issued a new release.

Given the changes to the behavior, this necessitates a major version bump from 2 to 3.

We originally introduced this logic in #132 without understanding all
the implications.

Issue #158 raised a challenge with this approach.
@sds sds changed the title Remove ffi conditional install windows Remove ffi conditional install on Windows platforms Sep 19, 2019
@sds sds force-pushed the remove-ffi-conditional-install-windows branch 3 times, most recently from dc4f9b2 to ee8da9b Compare September 19, 2019 09:57
Given the removal of the extension logic that conditionally installed
the `ffi` gem on Windows platforms, this fundamentally changes the
assumptions around what the gem installs and thus its backwards
compatibility, hence necessitating a major version upgrade.
@sds sds force-pushed the remove-ffi-conditional-install-windows branch from ee8da9b to f72bccc Compare September 20, 2019 12:26
@sds sds merged commit efde7cc into master Sep 20, 2019
@sds sds deleted the remove-ffi-conditional-install-windows branch September 20, 2019 12:58
Repository owner deleted a comment from coveralls Sep 20, 2019
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Mar 24, 2020
Update ruby-childprocessto 3.0.0.


### Version 3.0.0 / 2019-09-20

* [#156](enkessler/childprocess#156 unused `rubyforge_project` from gemspec
* [#160](enkessler/childprocess#160): Remove extension to conditionally install `ffi` gem on Windows platforms
* [#160](enkessler/childprocess#160): Remove runtime dependency on `rake` gem

### Version 2.0.0 / 2019-07-11

* [#148](enkessler/childprocess#148): Drop support for Ruby 2.0, 2.1, and 2.2
* [#149](enkessler/childprocess#149): Fix Unix fork reopen to be compatible with Ruby 2.6
* [#152](enkessler/childprocess#152): Fix hangs and permission errors introduced in Ruby 2.6 for leader processes of process groups

### Version 1.0.1 / 2019-02-03

* [#143](enkessler/childprocess#144): Fix installs by adding `rake` gem as runtime dependency
* [#147](enkessler/childprocess#147): Relax `rake` gem constraint from `< 12` to `< 13`

### Version 1.0.0 / 2019-01-28

* [#134](enkessler/childprocess#134): Add support for non-ASCII characters on Windows
* [#132](enkessler/childprocess#132): Install `ffi` gem requirement on Windows only
* [#128](enkessler/childprocess#128): Convert environment variable values to strings when `posix_spawn` enabled
* [#141](enkessler/childprocess#141): Support JRuby on Java >= 9
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

1 participant