Issue #3783: Fix CI check builds for windows platform #3785
Conversation
|
My personal grudge with the current setup is that all logs are lost. When builds fail, all the information I have is that the build failed. The errors (and warnings) aren't propagated back to where the user has access to them. At the moment, there are numerous warnings in C++ build that needs to be resolved. But nobody is seeing them and so nobody is fixing them. |
Signed-off-by: HS <[email protected]>
|
I have added two new workflows to use the native build system (cmake) to build cpp runtime so all the errors and warnings are shown. The configuration builds with both the cl and the clang (not clang-cl) compiler. @KvanTTT Resolved all the above comments. And, no, I haven't tried the swift builds. I am not educated in this language so if there are failures I wouldn't know what to do about them. I don't want to block this PR because of that potential failure. If needed, I will follow up with another PR with swift specific change. This PR is focused primarily on getting the Windows + cpp builds to work and be verified on CI. |
| @@ -1,3 +1,3 @@ | |||
| cd runtime-testsuite | |||
| mvn -Dantlr-python2-exec="C:\Python27\python.exe" -Dtest=python2.** test | |||
| mvn -Dantlr-python2-exec="%PYTHON_HOME%\python.exe" -Dtest=python2.** test | |||
There was a problem hiding this comment.
I disapprove this change.
This runs in an always on docker instance with both Python2 and Python3 installed
The python version at PYTHON_HOME is unknown
There was a problem hiding this comment.
Having a hard coded path is not portable. Other potential options -
- Get python in the %PATH% variable and use just
python.exei.e. without the qualifying full path - Update docker to setup the PYTHON_HOME variable
- Use different environment variables for Python2 and Python3, and update docker image to setup the variables accordingly
Feel free to just alternatives if you have any.
There was a problem hiding this comment.
We don't need this to be portable, currently it only runs on 4 docker instances on a physical box in my office
There was a problem hiding this comment.
A hardcoded path won't wok on Github cloud though. Python is not installed on that path.
Just may be - We might be on the different page here. Please follow comments on #3783 if you haven't yet.
There was a problem hiding this comment.
Is it possible to use just different environment variables %PYTHON2_HOME% and %PYTHON3_HOME% to make it workable everywhere?
There was a problem hiding this comment.
Sure. @ericvergnaud Would that resolve your concern?
There was a problem hiding this comment.
It might, but we don't need to solve problems we don't have. The existing scripts work just fine, they're just not portable, which is not something we need at this point.
Sorry for the tone, it's great that you've gotten the cpp build to run on Windows. I suggest you restrict this PR to just that.
| @@ -1,3 +1,3 @@ | |||
| cd runtime-testsuite | |||
| mvn -Dantlr-python3-exec="C:\Python310\python.exe" -Dtest=python3.** test | |||
| mvn -Dantlr-python3-exec="%PYTHON_HOME%\python.exe" -Dtest=python3.** test | |||
There was a problem hiding this comment.
I disapprove this change.
This runs in an always on docker instance with both Python2 and Python3 installed
The python version at PYTHON_HOME is unknown
|
Maybe an attempt to run on github servers will solve all of this or will this same problem occur? |
|
Abandoned in favor of #3792 |
Signed-off-by: HS [email protected]