Quote the file path to the Ruby bin directory when starting fluentd as a windows service

[iantalarico]

The the file path is passed to win32-service (which calls windows functions CreateService) and to Process.spawn. Both require spaces to be wrapped in quotes or windows will not properly handle them.

Without this fix fluentd will fail fail when initializing or when starting with a ruby directory that contains spaces.

[repeatedly]

@nurse How about this?
Does ruby have the function for this case?

[iantalarico]

It does not have a function that I am aware of.

[nurse]

@repeatedly instead of passing argv as single string, just pass them as array.
see also https://ruby-doc.org/core-2.4.1/Process.html#method-c-spawn

[repeatedly]

@iantalarico Could you try convert single string to array?

[iantalarico]

@repeatedly @nurse I gave this a try but was unable to make it work. Looking the documentation it looks like if I don't just pass a command line string it does not use the shell and will try to execute a built in command. I'm guessing this may not work if we are passing a path to an exe as the command.

[nurse]

@iantalarico Where and How it failed. I thought both ruby_path and rubybin_dir + "/ruby.exe " are full path.

[iantalarico]

@nurse It fails on Process.spawn.

If the path is quoted we get the error "Invalid argument - "C:/Program Files (x86)/TestDir/bin/ruby.exe""

If the path is unquoted we get the error "No such file or directory - OpenEvent"

I don't think it's and issue with the path I think it's an issue of what Process.spawn expects when we pass the command and an array.

[nurse]

@iantalarico How did you quote?
It seems you quoted twice, because Invalid argument is raised when path includes double quote.

[iantalarico]

@nurse I tried:
""#{rubybin_dir}/ruby.exe""
and
"#{rubybin_dir}/ruby.exe"

[nurse]

@iantalarico Could you show with Process.spawn(... )?

[iantalarico]

@nurse
Process.spawn("#{rubybin_dir}/ruby.exe", "#{rubybin_dir}/fluentd", opt, "-x #{INTEVENTOBJ_NAME}")
and
Process.spawn(""#{rubybin_dir}/ruby.exe"", ""#{rubybin_dir}/fluentd"", opt, "-x #{INTEVENTOBJ_NAME}")

[nurse]

Process.spawn("#{rubybin_dir}/ruby.exe", "#{rubybin_dir}/fluentd", opt, "-x #{INTEVENTOBJ_NAME}")

is correct.
And it shows "No such file or directory - OpenEvent" at Process.spawn?
The term "OpenEvent" is shown at that point sounds strange.

[iantalarico]

@nurse Yea I was wrong only the first one was failing on process spawn the second one fails on event open:

"C:/Program Files (x86)/TestDir/lib/ruby/gems/2.3.0/gems/win32-event-0.6.3/lib/win32/event.rb:125:in open'" "winsvc.rb:59:in service_stop'", "C:/Program Files (x86)/TestDir/lib/ruby/gems/2.3.0/gems/win32-service-0.8.10/lib/win32/daemon.rb:309:in `block in mainloop'"

It looks like the service is failing in some way in the daemon.

I can dig more into this if need be but I don't see a problem with just passing a command line string?

[nurse]

@iantalarico
Maybe Process.spawn("#{rubybin_dir}/ruby.exe", "#{rubybin_dir}/fluentd", opt, "-x", INTEVENTOBJ_NAME) is correct.
or maybe just it reached an another issue on your environment/setting.

Anyway I need full backtrace of the error.

[iantalarico]

@nurse I think that was the full backtrace. I did "err.backtrace.inspect"

[repeatedly]

After fixed this problem, I will release new v0.14.

[iantalarico]

@nurse Is the original backtrace I sent okay? Are you okay with the fix as is? This is essentially what was being done before. If not I can dig into this more.

[nurse]

@iantalarico Backtrace is OK.
Did you try Process.spawn("#{rubybin_dir}/ruby.exe", "#{rubybin_dir}/fluentd", opt, "-x", INTEVENTOBJ_NAME)?

[iantalarico]

@nurse It looks like I get the same backtrace:

C:/Program Files (x86)/TestDir/lib/ruby/gems/2.3.0/gems/win32-event-0.6.3/lib/win32/event.rb:125:in open'" "winsvc.rb:59:inservice_stop'", "C:/Program Files (x86)/TestDir/lib/ruby/gems/2.3.0/gems/win32-service-0.8.10/lib/win32/daemon.rb:309:in `block in mainloop'"

[nurse]

Hmm, it is not sufficient to investigate the issue. It seems to need more digging...

[iantalarico]

@nurse I'm happy to dig more into this but why is the current solution not acceptable? This is what the current code is doing.

[nurse]

@iantalarico Ah, I wrongly saw CI is failing but it looks false-positive.
And both part looks affect only Windows.
Therefore though array is better, current patch is acceptable.

@repeatedly Could you merge this if CI is ok.

[iantalarico]

@nurse Alright thanks. As I said I am happy to look into the array solution more if you feel strongly going that route.

[repeatedly]

Thanks for your work 👍