[release/2.1] Add support for building CoreCLR using python3 #28087
Conversation
This is a combined fix from 3 different PRs and one additional fix for 2.1: - dotnet#19043 - dotnet#19356 - dotnet#22145 The 2.1-specific fix is that python is is only used as a fallback, in case all other python program names dont work. As for the original PRs, they perform these changes: build.sh and build.cmd contain logic to identify a working version of python to use. System.Private.CoreLib ignores that and directly invokes 'python', which may not work, or even execute a different program. Fix that. The windows build scripts try finding python in order of python3, python2 and then python. The unix build scripts dont. They just try python2 variants and then fail. This change makes brings them closer together by letting users build using only python3. Use the same logic in CMakeLists.txt that's used in build.sh/build.cmd to lookup python.
|
@jkoritzinsky @tommcdon @hoyosjs can you please review this one ? |
There was a problem hiding this comment.
@silvioguiso has already done a 2.1 build with Python 3, so I expect no major exceptions from any script between a Python 2 and Python 3 break.
fyi: @sywhang
| <CMakeDefinitionSaveFile>$(IntermediateOutputPath)..\cmake.definitions</CMakeDefinitionSaveFile> | ||
| </PropertyGroup> | ||
| <Exec Command="python $(MSBuildThisFileDirectory)..\scripts\check-definitions.py "$(CMakeDefinitionSaveFile)" "$(DefineConstants)" "$(IgnoreDefineConstants)" " /> | ||
| <Exec Command=""$(PYTHON)" $(MSBuildThisFileDirectory)..\scripts\check-definitions.py "$(CMakeDefinitionSaveFile)" "$(DefineConstants)" "$(IgnoreDefineConstants)" " /> |
There was a problem hiding this comment.
@silvioguiso this is the line that broke stuff. Now that python is an env var and not a literal you need to do this everywhere else it gets called, OR make it conditional based using MSbuild conditions.
There was a problem hiding this comment.
Yikes. Sorry about that.
Should I revert the entire patch or should I go for targeted fixes?
If you want me to go for more targeted fixes, is there some way I can access logs that show where this PR breaks things?
There was a problem hiding this comment.
We fixed this in this PR, no need to revert.
#28102
This is a combined fix from 3 different PRs and one additional fix for 2.1:
The 2.1-specific fix is that python3 is is only used as a fallback, in case all other python program names dont work.
As for the original PRs, they perform these changes:
build.sh and build.cmd contain logic to identify a working version of python to use. System.Private.CoreLib ignores that and directly invokes'python', which may not work, or even execute a different program. Fix that.
The windows build scripts try finding python in order of python3, python2 and then python. The unix build scripts dont. They just try python2 variants and then fail. This change makes brings them closer together by letting users build using only python3.
Use the same logic in CMakeLists.txt that's used in build.sh/build.cmd to lookup python.