You are here

[Fixed] CommandPromptPortable: logic error in CheckForJava, CheckForAppTools and CheckForDataTools

4 posts / 0 new
Last post
WindyPigeon
Offline
Last seen: 2 years 6 months ago
Joined: 2021-05-23 05:26
[Fixed] CommandPromptPortable: logic error in CheckForJava, CheckForAppTools and CheckForDataTools

I really like the Command Prompt Portable made by PortableApps.com.
Generally, Command Prompt Portable will check for the existing of installed Java Portable in PortableApps.com Platform and update the PATH environment variable if so. But recently I found that Java Portable doesn't always work properly in Command Prompt Portable.
The following is a part from the Command Prompt Portable's source code (line 115 to line 129):

CheckForJava:
	${GetParent} $EXEDIR $0
	${GetParent} $EXEDIR $0	;==> This line is duplicated
	IfFileExists "$0\CommonFiles\Java\bin\java.exe" "" CheckForAppTools
		ReadEnvStr $1 "PATH"	;line 119
		System::Call 'Kernel32::SetEnvironmentVariable(t, t) i("PATH", "$0\CommonFiles\Java\bin\;$1").r0'	;line 120

CheckForAppTools:
	IfFileExists "$EXEDIR\App\tools\*.*" "" CheckForDataTools
		System::Call 'Kernel32::SetEnvironmentVariable(t, t) i("PATH", "$EXEDIR\App\tools\;$1").r0'	;line 124

CheckForDataTools:
	IfFileExists "$EXEDIR\Data\tools\*.*" "" CheckForData
		System::Call 'Kernel32::SetEnvironmentVariable(t, t) i("PATH", "$EXEDIR\Data\tools\;$1").r0'	;line 128

The code is wrong in line 124 and line 128, it updates PATH environment variable with the value retrieve at line 119 and not the PATH environment after updated at line 120.
That means if there is any item in App\tools folder then the Java in CommonFiles will be ignored, and if there is any item in Data\tools folder then both Java Portable and tools in App\tools will also be ignored, which is probably not the expected result.

Here's my solution to fix this error:

...

ReadEnvStr $1 "PATH" ;Retrieve PATH environment variable (add this line before the line 92)

...

CheckForJava:
	${GetParent} $EXEDIR $0
	IfFileExists "$0\CommonFiles\Java\bin\java.exe" "" CheckForAppTools
		StrCpy $1 "$0\CommonFiles\Java\bin\;$1"

CheckForAppTools:
	IfFileExists "$EXEDIR\App\tools\*.*" "" CheckForDataTools
		StrCpy $1 "$EXEDIR\App\tools\;$1"

CheckForDataTools:
	IfFileExists "$EXEDIR\Data\tools\*.*" "" SetEnvironmentVariable
		StrCpy $1 "$EXEDIR\Data\tools\;$1"
		

SetEnvironmentVariable:
	System::Call 'Kernel32::SetEnvironmentVariable(t, t) i("PATH", "$1").r0'

...

 
The alternative solution is adding a line to retrieve value of PATH before both line 124 and line 128 under CheckForAppTools and CheckForDataTools.

CheckForAppTools:
	IfFileExists "$EXEDIR\App\tools\*.*" "" CheckForDataTools
		ReadEnvStr $1 "PATH"
		System::Call 'Kernel32::SetEnvironmentVariable(t, t) i("PATH", "$EXEDIR\App\tools\;$1").r0'

CheckForDataTools:
	IfFileExists "$EXEDIR\Data\tools\*.*" "" CheckForData
		ReadEnvStr $1 "PATH"
		System::Call 'Kernel32::SetEnvironmentVariable(t, t) i("PATH", "$EXEDIR\Data\tools\;$1").r0'

Nevertheless, this is not actually a big error after all, since you can still add your custom path by editing the commandprompt.bat batch file in Data\Batch directory.

If administrator sees this post, can administrator help to make this fix to the next update of the Command Prompt Portable?

John T. Haller
John T. Haller's picture
Offline
Last seen: 2 hours 29 min ago
AdminDeveloperModeratorTranslator
Joined: 2005-11-28 22:21
Dev Test

Please test this out to ensure it's working as expected: Download Command Prompt Portable 2.5 Dev Test 1.

Sometimes, the impossible can become possible, if you're awesome!

WindyPigeon
Offline
Last seen: 2 years 6 months ago
Joined: 2021-05-23 05:26
Yes, it work properly. Thanks

Yes, it work properly. Thanks.

John T. Haller
John T. Haller's picture
Offline
Last seen: 2 hours 29 min ago
AdminDeveloperModeratorTranslator
Joined: 2005-11-28 22:21
Fixed in 2.5

This fix is included in today's 2.5 release. I also added in taskbar pinning support with the PA.c Platform.

Sometimes, the impossible can become possible, if you're awesome!

Log in or register to post comments