Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake plugin overhaul #703

Open
bfloch opened this issue Aug 29, 2019 · 10 comments
Open

CMake plugin overhaul #703

bfloch opened this issue Aug 29, 2019 · 10 comments
Labels

Comments

@bfloch
Copy link
Contributor

bfloch commented Aug 29, 2019

EDIT: The proposal might depend on later CMAKE version?! Have to verify how 2.8 could still work in case we need to support it for some reason.

In order to support cmake on windows (and to improve the plugin in general) I am proposing the following changes to the cmake plugin:

  1. Allow build_system and child_build_system to be None (new default)

    • In this case, instead of hard-coded logic, use cmake without specific generator and cmake --build build_path --target {ALL} where ALL is all in unix and ALL_BUILD on windows.
      • The all and ALL_BUILD distinction is unfortunate, but can be verified by listing all targets
    • This allows cmake to choose the best default for any platform (Visual Studio on Windows) and build without having to deal with Visual Studio and MSBuild discovery manually - which is really painful on windows. See the note below.
  2. Deprecate the hardcoded generators

    • We should support all generators that the underlying platform & cmake version supports
    • For backwards compatibility I will keep the current dictionary, but I will alternate it with a dynamic list coming from cmake --help that will always match a cmake-executable.
    • The only gotcha is that we don't have the context at class scope, so technically a different cmake version might run in context then what the arguments would show. I can deal with it by using the cmake bin path as key for the cached generators, but in this 10% case the bound arguments would not match the underlying generators. I will need to lift the validation in the command line parameter and let cmake deal with it. However platform and cmake version should be the only necessary inputs. Generators are compiled into cmake and can not be changed/extended otherwise.
      • If we really wanted to have accurate arguments we'd need to be creating a context for this part, but since the current method is not accurate either (hard-coded and may be wrong), I guess we could live with it. Also if you have an explicit cmake version it will match perfectly.

This would allow building on Windows (or other platforms?) with the default settings but still keep backwards compatibility.

I am already working towards a PR on this. Feedback appreciated.
Other necessary changes for better path handling will be in a separate issue.

Related note for the curious
Windows is very special.

In order to support other generators on windows I am looking at options for manual discovery of the visual studio installation and platform SDK - which is needlessly painful. This is not necessarily cmake related so I think its best to keep outside of core-rez/the plugin. It can be implemented as a package instead. My personal generic approach with least maintenance overhead is to rely on vswhere [1] to find vcvarsall.bat. Yes since they made Visual Studio only discoverable via COM in the latest releases you need a compiled executable to find the compiler to compile an executable ... 👏 Then I parse the difference of the resulting environment in terms of set, append and prepend env and reconstruct it within commands as @early()-binding. Since all this happens at build time and generates the right commands in the visual_studio and platform_sdk packages that evaluate instantly. Users with non default installation paths just need to rebuild the packages locally.
That's crazy you say? The benefit is you use Microsoft's official logic (thousands lines of horrifying batch files) to setup the environment but dodge the cmd related limitations [2] of the environment and just evaluate the resulting environment (fast!). This works correctly in any shell.
Other approaches are to reconstruct the partial/complete discovery and setup of the environment as seen in microsoft_crazyness.h [3] or CMake's Visual Studio generator [4] implementation. But this would mean maintaining it and so far there does not seem to be a pattern in the way both Visual Studio and the platform SDK change structure between releases. It's like they try hard to make you not use their platform for development ¯\_(ツ)_/¯
[1] https://github.com/microsoft/vswhere
[2] https://devblogs.microsoft.com/oldnewthing/20100203-00/?p=15083
[3] https://threader.app/thread/1125902959789740032
[4] https://github.com/Kitware/CMake/tree/master/Source

@Jordibastide
Copy link

Jordibastide commented Aug 30, 2019

I'm using a similar method to discover visual studio installation and platform SDK found in a "LLVM-based D compiler project" bat utility

(I've removed call command L52&57, it seemed to execute .bat in a subshell with call exec

Pros:

  • Discovers most recent visual studio install from 12.0 to 15+ (Visual Studio 2019)

Cons:

  • A bit long to execute, I will have a look on your suggested method (env diff bake)
  • No possibility to specify a max version for now

Thanks for making this moving forward

@bfloch
Copy link
Contributor Author

bfloch commented Aug 30, 2019

Thanks @Jordibastide!
Another one for my collection :) It also uses vswhere for newer visual studio versions, I see.

The whole point of my approach is to free the result from batch to not suffer from 8k/2k character limits.
So I guess one could convert this neat little script to PowerShell at least. I am just a little afraid that I might miss something like the standard library paths etc. that's why I currently stick to the vcvarsall.bat but interestingly this method will work with any other method of environment setup.

It's a bit sneaky but my results so far have been quite encouraging.

@bfloch
Copy link
Contributor Author

bfloch commented Sep 6, 2019

FYI I have a working implementation on this - but I'll wait for #707 since it relies on it to fix CMAKE_MODULE_PATH in windows.

@bfloch
Copy link
Contributor Author

bfloch commented Sep 9, 2019

#707 moved to #737

I've pushed my hacky WIP of the cmake overhaul to https://github.com/bfloch/rez/tree/feature/cmake_generators_overhaul
since it is being discussed on the mailing list:
https://groups.google.com/forum/#!topic/rez-config/9o3918_gzA0

(Not a PR. Please don't use this in production)

EDIT: To not confuse matters:
This branch only deals with cmake's default logic and the Visual Studio generators.
The vcvarsall.bat method discussed before that would support other generators like Ninja with msvc is not part of this branch.

@msercheli
Copy link

I am not sure if this information helps in any way, but I am dropping it here just in case.
VS 2019 Powershell environment for development:

For running x86 Powershell (call it from SysWOW64 folder.... I know... weird)
C:\Windows\SysWOW64\WindowsPowerShell\v1.0\powershell.exe -NoExit -Command "& { Import-Module C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\Tools\vsdevshell\Microsoft.VisualStudio.DevShell.dll; Enter-VsDevShell -InstanceId 77e464a7}"

For running x64 Powershell (call it from System32 folder..... weird again)
But unfortunately, I could not find the Microsoft.VisualStudio.DevShell.dll equivalent for x64.

This blog talks more about where 32 vs 64 Powershell live:
https://www.gregorystrike.com/2011/01/27/how-to-tell-if-powershell-is-32-bit-or-64-bit/

And this talks more about the DevShell.dll:
https://intellitect.com/enter-vsdevshell-powershell/

Again, not sure if this info helps at all.

Ps: All that in Windows 10

@bfloch
Copy link
Contributor Author

bfloch commented Nov 15, 2019

Thanks! Finally a powershell dev environment.
Here is an attached idiomatic solution that does not rely on "guessing" an instance id:

$vs2019_path = (@()+(&"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" -version 16.0 -property installationpath))[-1]
$vstarget_path = (@()+(&"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" -version 15.0 -property installationpath))[-1]
Import-Module (Join-Path $vs2019_path "Common7\Tools\Microsoft.VisualStudio.DevShell.dll")
Enter-VsDevShell -VsInstallPath $vstarget_path -SkipAutomaticLocation -DevCmdArguments -host_arch=amd64

Two things to notice:

  1. host_arch seems not to be respected - it always finds a x86 compiler (bug?!)
  2. It uses VsDevCmd.bat under the hood (!)

If you use the -SkipExistingEnvironmentVariables option, your variables will come through but it will not add new paths to PATH. So you won't get the cl.exe path. Not sure what the use case is here.

If you don't use the option that you get cl.exe but your variables are truncated and PATH is being completely replaced. The usual problems we have expressed with using VsDevCmd.bat all apply.

In my test I ran this before the lines above:

$Env:TESTVAR = (1..3000 -join ";C:\")
$Env:PATH += ";" + (1..3000 -join ";C:\")

My point stands: Microsoft fix you development environment - don't just wrap it in PowerShell.

For us it means nothing. My suggested workaround solution would still be superior as we would get the variables and append to exiting paths in PowerShell. This powershell dev prompt does not add anything since it is a batch file in disguise.

@bfloch
Copy link
Contributor Author

bfloch commented Nov 15, 2019

Just as a sanity test: All of this is to replace something as simple as CXX=/usr/bin/g++. Just saying ...

@dewyatt
Copy link

dewyatt commented Jan 21, 2020

Here's what I use for x64:

$vswhere = "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe"
$vspath = & "$vswhere" -latest -property installationPath
Import-Module "$vspath\Common7\Tools\Microsoft.VisualStudio.DevShell.dll"
Enter-VsDevShell -VsInstallPath "$vspath" -DevCmdArguments "-arch=x64 -no_logo" -SkipAutomaticLocation

(See parse_cmd.bat for other options to -DevCmdArguments, like -host_arch, etc)

EDIT: On a side note, for powershell core you may need Set-ItemProperty -Path 'HKLM:\SOFTWARE\Wow6432Node\Microsoft\VSCommon\16.0\SQM' -Name OptIn -Value 0 before calling Enter-VsDevShell.

@mxc123
Copy link

mxc123 commented May 22, 2021

rez-bind --quickstart
Binding platform into C:\Users\mcc\packages...
10:18:41 WARNING Skipping installation: Package variant already exists: c:\users\mcc\packages\platform\windows\package.py[]
Binding arch into C:\Users\mcc\packages...
10:18:41 WARNING Skipping installation: Package variant already exists: c:\users\mcc\packages\arch\AMD64\package.py[]
Binding os into C:\Users\mcc\packages...
10:18:41 WARNING Skipping installation: Package variant already exists: c:\users\mcc\packages\os\windows-10.0.17763\package.py[]
Binding python into C:\Users\mcc\packages...
命令语法不正确。
Traceback (most recent call last):
File "D:\Program Files\python27\Lib\runpy.py", line 174, in run_module_as_main
"main", fname, loader, pkg_name)
File "D:\Program Files\python27\Lib\runpy.py", line 72, in run_code
exec code in run_globals
File "D:\Program Files\rez\Scripts\rez\rez-bind.exe_main
.py", line 7, in
File "d:\program files\rez\lib\site-packages\rez\cli_entry_points.py", line 95, in run_rez_bind
return run("bind")
File "d:\program files\rez\lib\site-packages\rez\cli_main.py", line 184, in run
returncode = run_cmd()
File "d:\program files\rez\lib\site-packages\rez\cli_main.py", line 176, in run_cmd
return func(opts, opts.parser, extra_arg_groups)
File "d:\program files\rez\lib\site-packages\rez\cli\bind.py", line 77, in command
quiet=True)
File "d:\program files\rez\lib\site-packages\rez\package_bind.py", line 112, in bind_package
quiet=quiet)
File "d:\program files\rez\lib\site-packages\rez\package_bind.py", line 175, in bind_package
parser=bind_parser)
File "d:\program files\rez\lib\site-packages\rez\bind\python.py", line 86, in bind
pkg.post_commands = post_commands
File "D:\Program Files\python27\Lib\contextlib.py", line 24, in exit
self.gen.next()
File "d:\program files\rez\lib\site-packages\rez\package_maker.py", line 233, in make_package
make_root(variant
, root)
File "d:\program files\rez\lib\site-packages\rez\bind\python.py", line 70, in make_root
platform
.symlink(exepath, link)
File "d:\program files\rez\lib\site-packages\rez\utils\platform_.py", line 510, in symlink
subprocess.check_output("mklink %s %s" % (link_name, source), shell=True)
File "D:\Program Files\python27\Lib\subprocess.py", line 574, in check_output
raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command 'mklink c:\users\mcc\packages\python\2.7.12\platform-windows\arch-AMD64\os-windows-10.0.17763\bin\python d:\program files\python27\python.exe' returned non-zero exit status 1

E:\rez_packages\example_packages\hello_world>rez-bind python
searching d:\program files\rez\lib\site-packages\rez\bind...
creating package 'python' in C:\Users\mcc\packages...
10:23:54 WARNING Skipping installation: Package variant already exists: c:\users\mcc\packages\python\2.7.12\package.py[0]

@mxc123
Copy link

mxc123 commented May 22, 2021

I get the error when use the rez-bind python command line, i dont know how to relove it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants