AcademySoftwareFoundation / rez

An integrated package configuration, build and deployment system for software
https://rez.readthedocs.io
Apache License 2.0
960 stars 338 forks source link

CMake breaks due to windows penchant for escape characters as path separators #1321

Open anderslanglands opened 2 years ago

anderslanglands commented 2 years ago

3.0.5.zip

Trying to build Imath with the attached setup gives me:

Executing: c:\program files (x86)\microsoft visual studio\2017\community\common7\ide\commonextensions\microsoft\cmake\cmake\bin\cmake.exe C:\Users\anders\AppData\Local\Temp\rez-cook\imath\3.0.5 -Wno-dev -DCMAKE_ECLIPSE_GENERATE_SOURCE_PROJECT=TRUE -D_ECLIPSE_VERSION=4.3 --no-warn-unused-cli -DCMAKE_INSTALL_PREFIX=c:\users\anders\packages\imath\3.0.5 -DCMAKE_MODULE_PATH=%CMAKE_MODULE_PATH% -DCMAKE_BUILD_TYPE=Release -DREZ_BUILD_TYPE=local -DREZ_BUILD_INSTALL=0 -G NMake Makefiles
Initialize visual studio developer cmd env..
Not searching for unused variables given on the command line.
-- The C compiler identification is MSVC 19.16.27034.0
-- The CXX compiler identification is MSVC 19.16.27034.0
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Tools/MSVC/14.16.27023/bin/Hostx86/x86/cl.exe
CMake Error at C:/Users/anders/AppData/Local/Temp/rez-cook/imath/3.0.5/build/CMakeFiles/CMakeTmp/CMakeLists.txt:2 (set):
  Syntax error in cmake code at

    C:/Users/anders/AppData/Local/Temp/rez-cook/imath/3.0.5/build/CMakeFiles/CMakeTmp/CMakeLists.txt:2

  when parsing string

    c:\opt\rez\lib\site-packages\rezplugins\build_system\cmake_files

  Invalid character escape '\o'.

Environment

To Reproduce

  1. Extract attached zip somewhere and cd
  2. Remove package 'vs' from build_requires in package.py and make sure you're in a visual studio dev env
  3. run rez-build
anderslanglands commented 2 years ago

This is with cmd.exe as the shell by the way. With pwsh I get an even more baffling error:

CMake Error: The source directory "C:/Users/anders/AppData/Local/Temp/rez-cook/imath/3.0.5/build/platform-windows/arch-AMD64/.3" does not exist.

I've just been having a poke at it and I'm stumped. Even if I force the path to posix-style (with '/') when setting it in the env in cmake._add_build_actions() then it still gets written to the tmp CMakeLists.txt with '\'. Of course CMake helpfully deletes that temporary file so can't actually inspect it, but still I guess the path's getting reverted to windows-style in the -DCMAKE_MODULE_PATH=%CMAKE_MODULE_PATH% expansion in the cmake command line.

anderslanglands commented 2 years ago

Just for fun I tried changing the command line setting in cmake.build() to:

        cmake_path = os.path.join(os.path.dirname(__file__), "cmake_files")
        cmd.append("-DCMAKE_MODULE_PATH=%s" %
                   pathlib.Path(cmake_path).as_posix())

which seems to work. Of course now I'm blowing away anything else that might be set in CMAKE_MODULE_PATH so it's not a real solution.

anderslanglands commented 2 years ago

OK so the fix is I think to change cmd's normalize_path():

    def normalize_path(self, path):
        # Don't convert to windows path here because windows paths are evil and
        # CMake can't handle them
        return path
anderslanglands commented 2 years ago

Actually I would go further than this and do:

    def normalize_path(self, path):
        # Use POSIX paths because Windows understands them as well and CMake ONLY
        # understands POSIX
        import pathlib
        return pathlib.Path(path).as_posix()

reason being that without this, if I set an environment variable as "{root}/blah" then {root} may get exapnded to windows-style which also breaks CMake. A great example is setting env.CMAKE_PREFIX_PATH = "{root}"

nerdvegas commented 2 years ago

Ok, I don't see a problem with doing this. In terms of action items:

For clarity, pathlib's as_posix converts (eg) C:\foo\bah.txt to C:/foo/bah.txt.

anderslanglands commented 2 years ago

Here's a simpler test case. Again, the build_requires = ["vs"] just sets up the visual studio environment so cl, cmake etc can be found rez_path_test.zip .

anderslanglands commented 2 years ago

Ugh of course it's not that simple as other windows programs (e.g. move) will interpret the "/" as a switch. Will have to try escaped slash "\" now...

Vochsel commented 2 years ago

Hi! Any other fixes found here? Also getting this when using powerhsell CMake Error: The source directory ".../build/.3" does not exist. Thanks!

JeanChristopheMorinPerso commented 2 years ago

Unfortunately, I don't think any substantial progress has been made. Or at least I haven't heard of anything recently.

The thing is that we can patch stuff here and there, do hacks, etc but the problem is that it becomes hard to maintain. We need more investigation and a solid plan before we can move forward.

The issue is mainly tracked in https://github.com/AcademySoftwareFoundation/rez/issues/1302. Feel free to chip in.

We are also extremely limited in terms of resources right now and we also need to get the TSC up and running to get things rolling again.