Fix path generation to internal mingw

[scrouthtv]

Related Issues

I recently installed OpenModelica and when first trying to compile a model, I encountered this issue:

d:/Program Files/OpenModelica1.19.2-64bit/share/omc/scripts/Compile.bat Motor1 gcc mingw64 parallel dynamic 32 0
PATH = "d:\Program Files\OpenModelica1.19.2-64bit\tools\msys\mingw64\bin;d:\Program Files\OpenModelica1.19.2-64bit\tools\msys\mingw64\bin\..\..\usr\bin;"
'""d:\Program' is not recognized as an internal or external command,
operable program or batch file.

Upon further investigation, I found this issue when generating the path to the packaged mingw's make:

1. The path to the mingw folder is generated using set MINGW="%OPENMODELICAHOME%\tools\msys\%OM_PLATFORM%". Now, %MINGW% holds the path including double quotes around the entire path:

echo %MINGW%
"d:\Program Files\OpenModelica1.19.2-64bit\\tools\msys\mingw64"

2. This is later on concatenated as "%MINGW%\bin\mingw32-make" (L112)

Purpose

make is badly called as (e.g. for my system):

""d:\Program Files\OpenModelica1.19.2-64bit\\tools\msys\mingw64"\bin\mingw32-make"

It should instead be

"d:\Program Files\OpenModelica1.19.2-64bit\tools\msys\mingw64\bin\mingw32-make"

Note the extra quotes + the extra backslash.

This results in a failing compilation run.

Approach

1. Don't include the quotes in the %MINGW% variable (https://stackoverflow.com/a/34717386)
2. Remove the extra backslash

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[scrouthtv]

Ok there are even more issues than I've expected:

• race condition with CONVERT_CD_TO_SHORT_PATH_NAME & CONVERT_OPENMODELICAHOME_TO_SHORT_PATH_NAME (if this routine takes longer than the next command, %CURRENT_DIR% is unset for the next command)
• maybe race condition with call "%MSVCHOME\vcvarsall.bat, if the variables set there are used in MSVCCOMPILE, though I can't test that as don't have access to MSVC

Fix for these would be to simply remove the call and inline the variable settings.

[AnHeuermann]

@scrouthtv Thank you for your contribution!

Fix for these would be to simply remove the call and inline the variable settings.

Do you want to add these to your PR or open a new one for that problem? In the later case I'll merge this PR.

[adrpo]

I'm not really sure about this one: b309373
as for example myself I set OPENMODELICAHOME without the trailing slash.

[adrpo]

Maybe we should remove the trailing \ from the definition of OPENMODELICAHOME in: https://github.com/OpenModelica/OpenModelicaSetup/blob/master/OpenModelicaSetup.nsi#L321
and
https://github.com/OpenModelica/OpenModelicaSetup/blob/master/OpenModelicaSetup.nsi#L493

[AnHeuermann]

I guess we could leave the \ in this PR. Two backslashes shouldn't break anything, right?
The important part was the moved " anyway.

[scrouthtv]

Maybe we should remove the trailing \ from the definition of OPENMODELICAHOME

Makes more sense to me as well. On unix it's normal to not have trailing / as far as I can tell.

On Windows, most env variables don't have one either (e. g. PATH, windir, etc.)

Do you want to add these to your PR

I tried doing that locally as well. The script now works when run from the command line, however, I still get errors when trying to simulate from OM directly. I'm not sure why that is.

[AnHeuermann]

Suggested change

	set "MINGW=%OPENMODELICAHOME%tools\msys\%OM_PLATFORM%"
	set "MINGW=%OPENMODELICAHOME%\tools\msys\%OM_PLATFORM%"

[scrouthtv]

I am also unsure what is going on here:

OpenModelica/OMCompiler/Compiler/scripts/Compile.bat

Lines 39 to 42 in b309373

	cd /D "%MINGW%\bin"
	set PATH=%CD%;%CD%\..\..\usr\bin;
	echo PATH = "%PATH%"
	cd /D "%CURRENT_DIR%"

and why they're not simply using

set "PATH=%MINGW%\bin;%MINGW%\..\usr\bin;"
echo PATH =  %PATH%

This weird construct is used again when exiting:

OpenModelica/OMCompiler/Compiler/scripts/Compile.bat

Lines 120 to 121 in b309373

	@%COMSPEC% /C exit %RESULT%
	EXIT /B %ERRORLEVEL%

I guess they're safe to fix, I can't tell however, why it is the way it is...

[scrouthtv]

This is what I'm currently using. The script is working from the command line, though I'm still getting an error when running from OM.

To test this I manually added debugging to the script:

1. print the current working directory at the beginning of the script
2. print "end" just before the script is about to close (just before EXIT)

Upper part of the image: If I run "Simulate" from OM the script seems to have finished, as it first says "end" and then gives the error. I'm not sure if this issue is at all related to this specific script.

Lower part of the image: If I run the same command line from the correct folder, the script seems to work & even calls make.

I fear that there are some internal issues with OM unrelated to this script, that I'm unable to assess.

[adrpo]

In the image you are missing a \ that you deleted so you have OpenModelica...64bittools instead of OpenModelica...64bit\tools. OMEdit and omc will read the OPENMODELICAHOME and change the \ to / and probably remove the trailing \. That's why I said is better to keep the \ as you can never be sure if you will get or not the trailing \ in OPENMODELICAHOME.

[adrpo]

I am also unsure what is going on here:

OpenModelica/OMCompiler/Compiler/scripts/Compile.bat

Lines 39 to 42 in b309373

	cd /D "%MINGW%\bin"
	set PATH=%CD%;%CD%\..\..\usr\bin;
	echo PATH = "%PATH%"
	cd /D "%CURRENT_DIR%"

and why they're not simply using

set "PATH=%MINGW%\bin;%MINGW%\..\usr\bin;"
echo PATH =  %PATH%

This weird construct is used again when exiting:

OpenModelica/OMCompiler/Compiler/scripts/Compile.bat

Lines 120 to 121 in b309373

	@%COMSPEC% /C exit %RESULT%
	EXIT /B %ERRORLEVEL%

I guess they're safe to fix, I can't tell however, why it is the way it is...

I think we do this to get the short names instead of the path names with special chars or spaces, but I am not sure.