-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add Windows support #674
Add Windows support #674
Conversation
cool.io 1.2.0
(use in future)
(use in future)
(use in future)
(use in future)
(use in future)
Singleton class seems to prevent GC and make TailWatcher to be explicitly closed, which is dangerous because of multithreads as the comment of close_watcher says.
@sodabrew thanks for info. |
@tagomoris @sonots @frsyuki Check this PR. I have a plan to merge this PR and master branch will be for v0.14 development, e.g. nanosecond, SocketManager, ServerEngine. |
@@ -15,6 +15,8 @@ | |||
# ignore setup error on Win or similar platform which doesn't support signal | |||
end | |||
require 'cool.io' | |||
|
|||
$platformwin = /mswin|mingw/ === RUBY_PLATFORM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From sada's comment on serverengine, how about using this approach like Fluent.on_windows?
? > treasure-data/serverengine#26 (comment)
We have fluent/env.rb so this file seems good place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
@@ -65,8 +66,20 @@ def open(&block) | |||
end | |||
|
|||
def mv(path) | |||
File.rename(@path, path) | |||
@path = path | |||
if defined?(Windows) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fluent.windows?
is better?
Commented. The correctness of windows related code relies on patch! |
@tagomoris @frsyuki @sonots Are there any concerns? I want to merge this PR before merging nano-second support. |
op.on('--reg-winsvc-fluentdopt OPTION', "specify fluentd option paramters for Windows Service. (Windows only)") {|s| | ||
opts[:fluentdopt] = s | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about showing above windows-only options only when Fluentd.windows?
is true?
Great job 👍 |
If there is no concerns, I will merge this PR tomorrow! |
Hi, the docs lead me to believe that fluentd does not support Windows, see here: If that is wrong, you might want to correct the documentation. If it's true, then i don't understand this pull request. |
That document is about Fluentd v0.12. Fluentd v0.14 (and master branch) supports Windows environment. |
I will add windows installation article on fluentd.org, not docs.fluentd.org. |
@repeatedly I do not see any details on documentation yet? All I find as of now is https://docs.fluentd.org/v0.12/articles/windows |
To use this see http://qiita.com/nurse/items/8bbb194982c685f1e795