msi: fix wrong env path for Fluent Package Prompt

fluent / fluent-package-builder

td-agent (Fluentd) Building and Packaging System

Apache License 2.0

22 stars 23 forks source link

msi: fix wrong env path for Fluent Package Prompt #606

Closed daipom closed 7 months ago

daipom commented 7 months ago

fix https://github.com/fluent/fluent-package-builder/issues/605
The current PATH includes ". It is unintended and causes some bugs. For example, fluent-diagtool does not work correctly because of this.

daipom commented 7 months ago

I will check the behavior with the package built on the CI.

kenhys commented 7 months ago

NOTE: Additionally, %~dp0\bin expanded to c:\fluent\\bin for example, so there is no need to add path separator for it.

diff --git a/fluent-package/msi/assets/fluent-package-prompt.bat b/fluent-package/msi/assets/fluent-package-prompt.bat
index 96ec212..077877e 100644
--- a/fluent-package/msi/assets/fluent-package-prompt.bat
+++ b/fluent-package/msi/assets/fluent-package-prompt.bat
@@ -1,4 +1,4 @@
 @echo off
-set "PATH=%~dp0\bin;%PATH%"
+set "PATH=%~dp0bin;%PATH%"
 set "PATH=%~dp0;%PATH%"
 title Fluent Package Command Prompt
-- 
2.43.0

daipom commented 7 months ago

NOTE: Additionally, %~dp0\bin expanded to c:\fluent\\bin for example, so there is no need to add path separator for it.

diff --git a/fluent-package/msi/assets/fluent-package-prompt.bat b/fluent-package/msi/assets/fluent-package-prompt.bat
index 96ec212..077877e 100644
--- a/fluent-package/msi/assets/fluent-package-prompt.bat
+++ b/fluent-package/msi/assets/fluent-package-prompt.bat
@@ -1,4 +1,4 @@
 @echo off
-set "PATH=%~dp0\bin;%PATH%"
+set "PATH=%~dp0bin;%PATH%"
 set "PATH=%~dp0;%PATH%"
 title Fluent Package Command Prompt
-- 
2.43.0

Thanks! I see! I will add it.

daipom commented 7 months ago

NOTE: Additionally, %~dp0\bin expanded to c:\fluent\\bin for example, so there is no need to add path separator for it.

diff --git a/fluent-package/msi/assets/fluent-package-prompt.bat b/fluent-package/msi/assets/fluent-package-prompt.bat
index 96ec212..077877e 100644
--- a/fluent-package/msi/assets/fluent-package-prompt.bat
+++ b/fluent-package/msi/assets/fluent-package-prompt.bat
@@ -1,4 +1,4 @@
 @echo off
-set "PATH=%~dp0\bin;%PATH%"
+set "PATH=%~dp0bin;%PATH%"
 set "PATH=%~dp0;%PATH%"
 title Fluent Package Command Prompt
-- 
2.43.0

Thanks! I see! I will add it.

I will fix this in another PR after this PR is merged because this is not related to #605.

daipom commented 7 months ago

I have confirmed the msi package works correctly.

daipom commented 7 months ago

The fixed content itself is reasonable, but it lacks the reason "why".
* PATH was set to "c:\opt\fluent";"c:\opt\fluent\bin";...

* There is a case that some tool can't spawn td-agent-gem correctly when  PATH contains quoted literals...

* In this commit...
Could you amend commit message?

Thanks for your review. I have added it.