search

fluent / fluent-package-builder

td-agent (Fluentd) Building and Packaging System
Apache License 2.0
22 stars 23 forks source link

msi: support path which contains space or parenthesis #589

Closed kenhys closed 9 months ago

kenhys commented 10 months ago

When installing fluent-package to c:\Program Files, the space character must be escaped correctly.

kenhys commented 10 months ago

This PR still not work correctly yet. checking log...

kenhys commented 10 months ago

Installed to c:\Program Files:

fluent-escape-space-fluent

Then check fluentdopt:

fluent-escape-space-fluentdopt

Could run manually:

fluent-escape-space-fluentdwinsvc

daipom commented 10 months ago

I'm checking the package behavior...

daipom commented 10 months ago

I failed to install the package to C:\Dummy Program Files (FOO)\. I'm checking the cause.

daipom commented 10 months ago

I checked the log. InstallFluentdWinSvc is failing.

$  msiexec /i fluent-package-5.0.1-x64.msi /l* installer.log
Action 12:39:40: InstallFluentdWinSvc. 
WixQuietExec64:  \fluent\..\ の使い方が誤っています。
WixQuietExec64:  Error 0x800700ff: Command line returned an error.
WixQuietExec64:  Error 0x800700ff: QuietExec64 Failed
WixQuietExec64:  Error 0x800700ff: Failed in ExecCommon method
CustomAction InstallFluentdWinSvc returned actual error code 1603 (note this may not be 100% accurate if translation happened inside sandbox)
Action ended 12:39:40: InstallFinalize. Return value 3.
Action 12:39:40: Rollback. Rolling back action:
Rollback: InstallFluentdWinSvc
Rollback: Copying new files
Rollback: Creating folders
Rollback: Updating component registration
Action ended 12:40:10: INSTALL. Return value 3.
Action ended 12:40:10: ExecuteAction. Return value 3.
Action 12:40:10: FatalError. 
Action start 12:40:10: FatalError.
Action 12:40:10: FatalError. Dialog created
Action ended 12:40:12: FatalError. Return value 2.
Action ended 12:40:12: INSTALL. Return value 3.
kenhys commented 10 months ago

Are there Executing op: log? /l*v option will enable such feature.

kenhys commented 10 months ago

Are there Executing op: log? /l*v option will enable such feature.

Reproduced. got same log.

kenhys commented 10 months ago

assets/fluentd.bat cause it.

daipom commented 10 months ago

I think this is completely another issue. I will create another issue later. We should handle this separately from this PR.

I think this is the smallest script to reproduce:

/path/to/foo(bar)/test.bat

@echo off
if "a" == "b" (
  echo %~dp0
)
$ .\test.bat
\ was unexpected at this time.
daipom commented 10 months ago

Continue to check the behavior when the installed path including spaces.

daipom commented 10 months ago

bca8ebf967b25d6097271cac35d468fcc088f7fd

I see! Certainly, " will solve this problem.

I think this is completely another issue. I will create another issue later. We should handle this separately from this PR.

* This rollback occurs when the path includes `()`.

* This is caused by the `delayed environment variable` mechanism of BATCH.

I think this is the smallest script to reproduce:

/path/to/foo(bar)/test.bat

@echo off
if "a" == "b" (
  echo %~dp0
)
$ .\test.bat
\ was unexpected at this time.

Thanks!

/path/to/foo(bar)/test.bat

@echo off
if "a" == "a" (
  echo "%~dp0"
)
$ .\test.bat
"C:\path\to\foo(bar)\"
daipom commented 10 months ago

Continue to check the behavior when the installed path including spaces.

Installed path: C:\Dummy Program Files\

Something seems wrong with fluent-gem. Looks like calling fluent-gem breaking the environment.

Fluent Package Command Prompt with administrative privileges:

C:\Windows\system32>fluentd --version
fluent-package 5.0.1 fluentd 1.16.2 (d5685ada81ac89a35a79965f1e94bbe5952a5d3a)

C:\Windows\system32>fluent-gem --version
'C:\Dummy' は、内部コマンドまたは外部コマンド、
操作可能なプログラムまたはバッチ ファイルとして認識されていません。
'C:\Dummy' は、内部コマンドまたは外部コマンド、
操作可能なプログラムまたはバッチ ファイルとして認識されていません。

C:\Windows\system32>fluent-gem --version
C:/Dummy Program Files/fluent/lib/ruby/3.2.0/rubygems.rb:263:in `find_spec_for_exe': can't find gem fluentd (>= 0.a) with executable fluent-gem (Gem::GemNotFoundException)
        from C:/Dummy Program Files/fluent/lib/ruby/3.2.0/rubygems.rb:282:in `activate_bin_path'
        from C:/Dummy Program Files/fluent/bin/fluent-gem:32:in `<main>'

C:\Windows\system32>fluentd --version
C:/Dummy Program Files/fluent/lib/ruby/3.2.0/rubygems.rb:263:in `find_spec_for_exe': can't find gem fluentd (>= 0.a) with executable fluentd (Gem::GemNotFoundException)
        from C:/Dummy Program Files/fluent/lib/ruby/3.2.0/rubygems.rb:282:in `activate_bin_path'
        from C:/Dummy Program Files/fluent/bin/fluentd:32:in `<main>'
kenhys commented 10 months ago

Yes fluent-gem also has same issue.

daipom commented 10 months ago

Oh, is it the same issue? I don't completely understand the mechanism... OK, I will check the behavior again after the new package is built!

kenhys commented 10 months ago

f1dd645 has still issues...

kenhys commented 10 months ago

It should be delims=.

kenhys commented 10 months ago

Now fluent-gem will work.

fluent-escape-space-fluent-gem

daipom commented 10 months ago

Thanks. I'm checking the package.

Finally, I understood the mechanism of \ was unexpected at this time. error.

/path/to/foo(bar)/test.bat

@echo off
if "a" == "b" (
  echo %~dp0
)
$ .\test.bat
\ was unexpected at this time.

delayed environment variable has nothing to do with this error.

Is is simply because the () of the path breaks the if () block.

In the above case, echo %~dp0 will be echo /path/to/foo(bar)/. So, the script will be equivalent to the following.

@echo off
if "a" == "b" (
  echo /path/to/foo(bar)/
)

The error occurs simlply becaues this script is broken. We can workaround this by ".

daipom commented 10 months ago

Hmm, I thought there was no problem, but I have noticed that I can not run Fluentd in Fluent Package Command Prompt... I'm checking the cause.

$ fluentd
C:/Dummy Program Files (FOO)/fluent/lib/ruby/gems/3.2.0/gems/fluentd-1.16.2/lib/fluent/config.rb:43:in `initialize': Invalid argument @ rb_sysopen - "C:/Dummy Program Files (FOO)/fluent//etc/fluent/fluentd.conf" (Errno::EINVAL)
        from C:/Dummy Program Files (FOO)/fluent/lib/ruby/gems/3.2.0/gems/fluentd-1.16.2/lib/fluent/config.rb:43:in `open'
        from C:/Dummy Program Files (FOO)/fluent/lib/ruby/gems/3.2.0/gems/fluentd-1.16.2/lib/fluent/config.rb:43:in `build'
        from C:/Dummy Program Files (FOO)/fluent/lib/ruby/gems/3.2.0/gems/fluentd-1.16.2/lib/fluent/supervisor.rb:682:in `setup_global_logger'
        from C:/Dummy Program Files (FOO)/fluent/lib/ruby/gems/3.2.0/gems/fluentd-1.16.2/lib/fluent/supervisor.rb:624:in `configure'
        from C:/Dummy Program Files (FOO)/fluent/lib/ruby/gems/3.2.0/gems/fluentd-1.16.2/lib/fluent/command/fluentd.rb:351:in `<top (required)>'
        from <internal:C:/Dummy Program Files (FOO)/fluent/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
        from <internal:C:/Dummy Program Files (FOO)/fluent/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:85:in `require'
        from C:/Dummy Program Files (FOO)/fluent/lib/ruby/gems/3.2.0/gems/fluentd-1.16.2/bin/fluentd:15:in `<top (required)>'
        from C:/Dummy Program Files (FOO)/fluent/bin/fluentd:32:in `load'
        from C:/Dummy Program Files (FOO)/fluent/bin/fluentd:32:in `<main>'
kenhys commented 10 months ago

Thanks! I'll check it.

kenhys commented 10 months ago

d9b8999 causes msi rollback unexpectedly.

daipom commented 10 months ago

Isn't it caused by not applying this?

https://github.com/fluent/fluent-package-builder/pull/589#discussion_r1335325513

kenhys commented 10 months ago

Isn't it caused by not applying this?

maybe that's it. Thanks.

kenhys commented 10 months ago

With https://github.com/fluent/fluent-package-builder/commit/e26df3fced2606fb60edc3fc7c35f1d4de338f4a, surely unexpected rollback was fixed.

image

And more, from fluent package prompt with admin also works.

image

daipom commented 10 months ago

What do you think about https://github.com/fluent/fluent-package-builder/pull/589#discussion_r1335315312?

kenhys commented 10 months ago

It seems work as expected.

daipom commented 10 months ago

Thanks! I will check the built package briefly, just in case.

kenhys commented 9 months ago

Thanks, I'll fix it.

kenhys commented 9 months ago

Revised commit message.

daipom commented 9 months ago

Thanks!