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

Windows support for signal handling and dealing with IO #26

Closed
wants to merge 9 commits into from

Conversation

naritta
Copy link
Contributor

@naritta naritta commented Sep 30, 2015

In windows,
There is no support for:
・signal except SIGTERM
(Some windows version have, but it is too different in version.)
・read_nonblock IO

Then, I added condition branch.

@@ -47,6 +47,8 @@ module ServerEngine
require File.join(here, v)
}

$platformwin = /mswin|mingw/ === RUBY_PLATFORM
Copy link
Member

Choose a reason for hiding this comment

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

using global variable is not good idea because its name is not in a namespace of ServerEngine.
Why don't you use this way?:

module ServerEngine
  def self.windows?
    IS_WINDOWS
  end

  IS_WINDOWS = /mswin|mingw/ === RUBY_PLATFORM
  private_constant :IS_WINDOWS
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed it.
naritta@5bf96ee

@@ -20,6 +20,9 @@ Gem::Specification.new do |gem|
gem.required_ruby_version = ">= 1.9.3"

gem.add_dependency "sigdump", ["~> 0.2.2"]
if /mswin|mingw/ =~ RUBY_PLATFORM
Copy link
Member

Choose a reason for hiding this comment

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

Does this if work when user install serverengine.gem on a windows machine?
To make this work, Rakefile need to create 2 gems (serverengine-VERSION.gem and serverengine-VERSION-mingw.gem), right? I think that is unnecessary optimization because installing win32-pipe does nothing on non-windows platform. So, why don't you simply depend on win32-pipe without if?

@frsyuki
Copy link
Member

frsyuki commented Dec 9, 2015

Merged with #29.

@frsyuki frsyuki closed this Dec 9, 2015
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.

3 participants