Fix CMake generation on Windows #536

object71 · 2021-03-23T09:52:01Z

On Windows, paths are mixed by os.path.join with different slash directions. This should generally be fine but when using the print function we print "C:\\mypath/myfile.cpp" as "C:\mypath/myfile.cpp" which is unescaped on the backward slash. To fix this I propose to replace "\\" with "/" just before printing it out for CMake usage. I would suggest using pathlib but it is not included by default on python versions <= 3.3 which may still be an issue.

o01eg · 2021-04-16T20:43:30Z

Catch this on our GDNative project. When this will be merged?

vnen · 2021-04-16T21:42:16Z

I don't really understand this change. If we want to use / everywhere isn't it better to just not use os.path.join at all and just hardcode the slashes?

Also, I don't think we need to worry about compatibility with EOL Python versions. Everyone should have a maintained Python 3 version installed.

object71 · 2021-04-17T10:03:13Z

In summary, the change fixes the following issue: when you create a path in python for some file the path will be with mixed slashes "/" and "\". Both python and cmake can handle those but... python as other programming languages will store back slashes escaped "C:\\Program Files\\Project/file.h" but when you call print so that they are provided to CMake it will actually print them unescaped as "C:\Program Files\Project/file.h" and then CMake will crash saying it doesn't know what the escape symbol "\P" means.

object71 · 2021-04-17T10:05:00Z

And to answer your question - yes it is better to not use os.path.join and instead use Pathlib but it is only recently added by default in python versions higher than 3.3 . If you're okay with that I will change this commit to instead use Pathlib for the paths in python and then call the as_posix() method when printing them out to CMake.

vnen · 2021-04-17T14:44:37Z

In summary, the change fixes the following issue: when you create a path in python for some file the path will be with mixed slashes "/" and "".

I do understand the issue, I just don't understand why there are paths with mixed slashes in the first place. I would prefer not have mixed slashes to begin with instead of fixing them with a post-hoc replace.

I'm okay with using pathlib. As I said, we should not need to keep compatibility with EOL Python versions (even 3.4 is EOL now). I'd rather have a proper fix instead of patching this, because otherwise it'll never be fixed properly.

It was causing issue on windows with escaped backwards slashes. This should now solve the issue as PosixPath will always be used which is understandable both by CMake and python and doesn't require backwards slashes.

object71 · 2021-04-17T15:22:18Z

@vnen I modified the generators file to now use pathlib and Path by default so that it doesn't cause issues on windows.

…re printing it out

o01eg · 2021-04-18T17:59:47Z

I've tried to use it in our project and get error:


2021-04-18T17:30:52.1922296Z          Cloning into 'godot-cpp'...
2021-04-18T17:30:52.1923291Z          HEAD is now at 0ad827f Modify to use Path instead of PosixPath and only call as_posix() before printing it out
2021-04-18T17:30:52.1924720Z          Submodule 'godot-headers' (https://github.com/godotengine/godot-headers) registered for path 'godot-headers'
2021-04-18T17:30:52.1999526Z          Cloning into 'D:/a/freeorion/freeorion/build/godot-cpp/godot-headers'...
2021-04-18T17:30:52.7010118Z          Submodule path 'godot-headers': checked out '815f34e1e96c09122449105c55aba501654da029'
2021-04-18T17:30:52.8675937Z          Performing update step for 'godotcpp'
2021-04-18T17:30:52.9946726Z          No patch step for 'godotcpp'
2021-04-18T17:30:53.0351565Z          Performing configure step for 'godotcpp'
2021-04-18T17:30:54.9735079Z          -- The C compiler identification is MSVC 19.28.29913.0
2021-04-18T17:30:56.0088804Z          -- The CXX compiler identification is MSVC 19.28.29913.0
2021-04-18T17:30:56.0334560Z          -- Detecting C compiler ABI info
2021-04-18T17:30:57.0380066Z          -- Detecting C compiler ABI info - done
2021-04-18T17:30:57.0406489Z          -- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.28.29910/bin/Hostx64/x86/cl.exe - skipped
2021-04-18T17:30:57.0422005Z          -- Detecting C compile features
2021-04-18T17:30:57.0423754Z          -- Detecting C compile features - done
2021-04-18T17:30:57.0527413Z          -- Detecting CXX compiler ABI info
2021-04-18T17:30:58.0438062Z          -- Detecting CXX compiler ABI info - done
2021-04-18T17:30:58.0470765Z          -- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.28.29910/bin/Hostx64/x86/cl.exe - skipped
2021-04-18T17:30:58.0472592Z          -- Detecting CXX compile features
2021-04-18T17:30:58.0479820Z          -- Detecting CXX compile features - done
2021-04-18T17:30:58.1906918Z          -- Found PythonInterp: D:/a/freeorion/bin/python3.5.exe (found version "3.5.7") 
2021-04-18T17:30:58.1908436Z          -- Generating Bindings
2021-04-18T17:31:01.3893953Z          -- Configuring done
2021-04-18T17:31:01.5003677Z          -- Generating done
2021-04-18T17:31:01.5080064Z      3>CUSTOMBUILD : CMake warning :  [D:\a\freeorion\freeorion\build\godotcpp.vcxproj]
2021-04-18T17:31:01.5081762Z            Manually-specified variables were not used by the project:
2021-04-18T17:31:01.5082566Z          
2021-04-18T17:31:01.5083141Z              BUILD_TESTING
2021-04-18T17:31:01.5084060Z          
2021-04-18T17:31:01.5084922Z          -- Build files have been written to: D:/a/freeorion/freeorion/build/godot-cpp-build
2021-04-18T17:31:01.5085940Z          
2021-04-18T17:31:01.5382723Z          Performing build step for 'godotcpp'
2021-04-18T17:31:01.6961280Z          Microsoft (R) Build Engine version 16.9.0+5e4b48a27 for .NET Framework
2021-04-18T17:31:01.6963130Z          Copyright (C) Microsoft Corporation. All rights reserved.
2021-04-18T17:31:01.6964328Z          
2021-04-18T17:31:02.2025374Z            Checking Build System
2021-04-18T17:31:02.5101951Z            Bindings
2021-04-18T17:31:03.9572799Z            Traceback (most recent call last):
2021-04-18T17:31:03.9574270Z              File "<string>", line 1, in <module>
2021-04-18T17:31:03.9576520Z              File "D:\a\freeorion\freeorion\build\godot-cpp\binding_generator.py", line 71, in generate_bindings
2021-04-18T17:31:03.9577695Z                with open(header_filename, "w+") as header_file:
2021-04-18T17:31:03.9578999Z            TypeError: invalid file: WindowsPath('D:/a/freeorion/freeorion/build/godot-cpp-build/include/gen/GlobalConstants.hpp')

I worked on cb837cb

object71 · 2021-04-18T19:17:23Z

i will check it out tomorrow

o01eg · 2021-04-18T19:25:01Z

I suppose it was better to accept fix as is and refactor it later especially after accepting CI changes.

object71 · 2021-04-18T21:21:44Z

Nah, my fix was merely the simplest solution and I thoutght it would be enough.

Anyway it is better to address the underlying problem and provide a good fix even if it causes a few more issues before its ideal.

…ors. Also now calling Path.open(mode) instead of open(Path, mode).

object71 · 2021-04-19T04:46:43Z

@o01eg Can you try and reproduce the issue with the latest commit? I wasn't able to reproduce it on my PC and the windows build seems to be successful.

o01eg · 2021-04-19T04:58:31Z

@object71 Yes, it works

         Performing build step for 'godotcpp'
         Microsoft (R) Build Engine version 16.9.0+5e4b48a27 for .NET Framework
         Copyright (C) Microsoft Corporation. All rights reserved.
         
           Checking Build System
           Bindings
           D:\a\freeorion\freeorion\build\godot-cpp-build\src\gen: File exists
           D:\a\freeorion\freeorion\build\godot-cpp-build\src\gen: File exists
           Building Custom Rule D:/a/freeorion/freeorion/build/godot-cpp/CMakeLists.txt
           AABB.cpp
           Array.cpp
           Basis.cpp
           CameraMatrix.cpp
           Color.cpp
           ...

kelteseth · 2021-05-10T17:12:57Z

I checked out @object71 code and reduced the hundreds of errors to one:


Running C:\Program Files\CMake\bin\cmake.exe -S D:/Backup/Code/Godot/Osm -B C:/Users/Eli/AppData/Local/Temp/QtCreator-wzByDP/qtc-cmake-fWFIrKtp -GNinja "-DCMAKE_BUILD_TYPE:STRING=Debug" "-DCMAKE_PROJECT_INCLUDE_BEFORE:PATH=C:/Qt/Tools/QtCreator/share/qtcreator/package-manager/auto-setup.cmake" "-DQT_QMAKE_EXECUTABLE:STRING=C:/Qt/5.15.2/msvc2019_64/bin/qmake.exe" "-DCMAKE_PREFIX_PATH:STRING=C:/Qt/5.15.2/msvc2019_64" "-DCMAKE_C_COMPILER:STRING=C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.28.29910/bin/HostX86/x64/cl.exe" "-DCMAKE_CXX_COMPILER:STRING=C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.28.29910/bin/HostX86/x64/cl.exe" in C:\Users\Eli\AppData\Local\Temp\QtCreator-wzByDP\qtc-cmake-fWFIrKtp.
-- The C compiler identification is MSVC 19.28.29913.0
-- The CXX compiler identification is MSVC 19.28.29913.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.28.29910/bin/HostX86/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.28.29910/bin/HostX86/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PythonInterp: C:/Users/Eli/AppData/Local/Programs/Python/Python39/python.exe (found version "3.9.5") 
-- Generating Bindings
-- Configuring done
-- Generating done
CMake Error:
  Running

   'C:/Users/Eli/Apps/ninja.exe' '-C' 'C:/Users/Eli/AppData/Local/Temp/QtCreator-wzByDP/qtc-cmake-fWFIrKtp' '-t' 'recompact'

  failed with:

   ninja: error: build.ninja:5960: multiple rules generate godot-cpp/CMakeFiles/godot-cpp.dir/src/gen/GlobalConstants.cpp.obj [-w dupbuild=err]



CMake Generate step failed.  Build files cannot be regenerated correctly.
CMake process exited with exit code 1.
Elapsed time: 00:02.

object71 · 2021-05-10T19:40:23Z

Well, I'll need to check that out. What hundreds of errors?

o01eg · 2021-05-12T00:38:32Z

Do we need to wait fix for Ninja generator to merge it?

object71 · 2021-05-13T08:36:12Z

I think this could be merged anyway - the error described above is just a warning. It is just that @kelteseth has made it into an error report which blocks the Ninja generation.

EDIT: and by looking at it, it should be also reproducible without my change. I couldn't find how I can produce the same issue but my change doesn't enumerate more files. It just enumerates them with the correct slashes.

o01eg · 2021-05-13T21:12:34Z

I've added CMake+Ninja test to Linux CI and it works: #518

object71 · 2021-05-15T16:44:16Z

Well I guess someone responsible for merging should make the decision. My opinion is that the error described above is not caused from my changes and I couldn't reproduce it neither on windows neither on linux machine.

Calinou

Thanks! Congratulations for your first merged pull request 🎉

zphensley42 · 2022-10-09T02:05:18Z

I know this is old and dead, but I encountered the issues described with duplicate build objects when running with CLion.

I've got a stop-gap fix here: https://github.com/godotengine/godot-cpp/compare/3.x...zphensley42:godot-cpp:fix/3.x_duplicate_ninja_error?expand=1

I hope it helps someone.

Calinou added the bug This has been identified as a bug label Mar 23, 2021

pseudofred approved these changes Apr 14, 2021

View reviewed changes

o01eg mentioned this pull request Apr 16, 2021

Test godot-cpp building with CMake and MSVS #544

Closed

Modify binding_generator.py to use pathlib instead of os.path.

bedb339

It was causing issue on windows with escaped backwards slashes. This should now solve the issue as PosixPath will always be used which is understandable both by CMake and python and doesn't require backwards slashes.

Modify to use Path instead of PosixPath and only call as_posix() befo…

0ad827f

…re printing it out

Returned error check with print as call to mkdir may ignore other err…

982bd30

…ors. Also now calling Path.open(mode) instead of open(Path, mode).

o01eg mentioned this pull request Apr 21, 2021

Normalize CMake paths #546

Closed

o01eg mentioned this pull request May 12, 2021

Start the server with Godot client freeorion/freeorion#3431

Merged

Calinou approved these changes May 15, 2021

View reviewed changes

Calinou merged commit 476a870 into godotengine:master May 15, 2021

Uh oh!

Fix CMake generation on Windows #536

Fix CMake generation on Windows #536

Uh oh!

Conversation

object71 commented Mar 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

o01eg commented Apr 16, 2021

Uh oh!

vnen commented Apr 16, 2021

Uh oh!

object71 commented Apr 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

object71 commented Apr 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vnen commented Apr 17, 2021

Uh oh!

object71 commented Apr 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

o01eg commented Apr 18, 2021

Uh oh!

object71 commented Apr 18, 2021

Uh oh!

o01eg commented Apr 18, 2021

Uh oh!

object71 commented Apr 18, 2021

Uh oh!

object71 commented Apr 19, 2021

Uh oh!

o01eg commented Apr 19, 2021

Uh oh!

kelteseth commented May 10, 2021

Uh oh!

object71 commented May 10, 2021

Uh oh!

o01eg commented May 12, 2021

Uh oh!

object71 commented May 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

o01eg commented May 13, 2021

Uh oh!

object71 commented May 15, 2021

Uh oh!

Calinou left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zphensley42 commented Oct 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

object71 commented Mar 23, 2021 •

edited

Loading

object71 commented Apr 17, 2021 •

edited

Loading

object71 commented Apr 17, 2021 •

edited

Loading

object71 commented Apr 17, 2021 •

edited

Loading

object71 commented May 13, 2021 •

edited

Loading

Calinou left a comment •

edited

Loading