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 hardcoded platform specific dependencies. #54

Closed
wants to merge 1 commit into from

Conversation

voxik
Copy link
Contributor

@voxik voxik commented Jul 24, 2012

Hi,

This gem works independently of its dependencies. Is not good idea to force people on some platform to install other platform specific code. More over, it explodes the amount of gems I need on my system, it pollutes my load path as is shown in example bellow and as least but not last, it makes packaging for Linux distribution pain. Thank you for consideration.

irb
irb(main):001:0> puts $:
/usr/local/share/ruby/site_ruby
/usr/local/lib64/ruby/site_ruby
/usr/share/ruby/vendor_ruby
/usr/lib64/ruby/vendor_ruby
/usr/share/rubygems
/usr/share/ruby
/usr/lib64/ruby
=> nil
irb(main):002:0> require 'listen'
=> true
irb(main):003:0> puts $:
/home/vondruch/gem_home/listen/gems/rb-fsevent-0.9.1/lib
/usr/share/gems/gems/ffi-1.0.9/lib
/usr/share/gems/gems/ffi-1.0.9/ext
/usr/lib64/gems/exts/ffi-1.0.9/lib
/home/vondruch/gem_home/listen/gems/rb-inotify-0.8.8/lib
/home/vondruch/gem_home/listen/gems/rb-fchange-0.0.5/lib
/home/vondruch/gem_home/listen/gems/listen-0.4.7/lib
/usr/local/share/ruby/site_ruby
/usr/local/lib64/ruby/site_ruby
/usr/share/ruby/vendor_ruby
/usr/lib64/ruby/vendor_ruby
/usr/share/rubygems
/usr/share/ruby
/usr/lib64/ruby
=> nil

@voxik
Copy link
Contributor Author

voxik commented Jul 24, 2012

The hardcoded dependencies even don't let me used polling if I want to use them for whatever reason.

@voxik
Copy link
Contributor Author

voxik commented Jul 24, 2012

Also note that FSSM gem never hardcoded such dependencies.

@travisbot
Copy link

This pull request fails (merged 9926921 into e985b25).

@rymai
Copy link
Member

rymai commented Jul 24, 2012

Hi,

this might interest you on why the core team made this choice: #5

Also, if you want to use polling explicitly, you can use force_polling(true) (see README.

@voxik
Copy link
Contributor Author

voxik commented Jul 24, 2012

@rymai

Thank you for pointers. Yes, I am aware that there is no way to install platform specific dependencies. But I still prefer to remove the dependencies. And there was no objections at that time, so I have to come with them later :)

To put things a bit into context. I come to the listen gem, because it is used by the sass gem. So I am not in control if polling will be used or not. And I am running now the test suite on Fedora and with rb-inotify installed, it just randomly crashes. If it would be optional, I would simply not install the platform specific bits and stayed with polling. Also, you are forcing me to package 3 more gems for Fedora where 2 of them has no meaning on my system and I am afraid they will be not easy to package. So at the end, everybody will be just disappointed.

I decided that I will patch the gem for Fedora that it will not require the dependencies, however that will break even more systems, because there will be application shipped with Gemfile.lock which will specify some dependencies which are not needed by Fedora.

Please, reconsider your decision and accept this pull request if you don't want to make life harder to us, packagers for various distros and to our users. Thank you.

@rymai
Copy link
Member

rymai commented Jul 24, 2012

Thanks for these info, however I'd like to have the opinion of @Maher4Ever and @thibaudgg (he's in vacation right now but he'll be online next week) about that and anyways, I'll let them choose to merge or not this pull-request.

Regarding rb-inotify crashes, this probably need to be fixed (either in listen or in rb-inotify) in order for Fedora users to take advantage of it instead of polling. If you think that's a Listen issue, please feel free to open a new issue ticket.

@voxik
Copy link
Contributor Author

voxik commented Jul 24, 2012

@rymai NP, thank you.

@Maher4Ever
Copy link
Contributor

Personally I'm not in favor of the fact that our users have to install some gems which they don't need. It works fine, but It's not elegant. Listen doesn't really need the platform-specific gems to function. It can always work with the polling adapter, so I don't think the gems need to be specified in the gemspec as dependencies.

We can treat the platform-specific gems as extra functionality, just like Guard does with the interactors. We can even go a step further and inform the user about the best choice for his environment at runtime.

@thibaudgg
Copy link
Member

Ok, so it would be polling by default if there's no system gem present with a nice invitation message to install the gem if force-polling message isn't used? I also think it could be a better approach.

@Maher4Ever
Copy link
Contributor

Great! I'll try to get this done tonight.

@voxik
Copy link
Contributor Author

voxik commented Jul 30, 2012

👍

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

5 participants