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
MSBuild Extensions in CPython NuGet Package has Bad Expression #84339
Comments
CPython provides a NuGet package as a mechanism to support non-installed Python distributions. The package includes MSBuild support to integrate with its build process. The expressions on lines 32 and 33 in the file: https://github.com/python/cpython/blob/master/PC/layout/support/props.py are both missing closing parentheses, which results in literal text instead of the resolve file paths. This appears to be introduced in version 3.7.2 of the package onward, including the current pre-release 3.9.0-a5. In addition, several build conditions use the form " If there is no further discussion or objection, I can submit a PR with the required fixes. |
The closing parentheses are needed - a PR would be appreciated for that. The quotes around a variable reference are unnecessary. At a parser level, it just changes it from a variable reference to a string literal with substitutions (unlike most shells, which do a textual substitution before parsing). |
In testing the fix, another issue has arisen. It appears the specified expression will never yield a usable path. Expression 1: Expression 2: The package has the abridged structure of: ┌─ build Based on this hierachy, neither exe will resolve because they do not have the same common ancestor. Additionally, Interestingly, the property specified in versions 3.7.1 and earlier appear to define the correct path as: <PythonHome>$(MSBuildThisFileDirectory)\..\..\tools</PythonHome> My suggestion is to revert back to this older variant. If "python_d.exe" isn't needed, then it should be removed. <PythonHome>$([MSBuild]::NormalizePath($(MSBuildThisFileDirectory)\..\..\tools))</PythonHome> OR <PythonHome>$([System.IO.Path]::GetFullPath($(MSBuildThisFileDirectory)\..\..\tools))</PythonHome> In my particular case, the tooling I'm plugging this into wasn't happy unless the path was absolute. There As soon as I know what the final form of the property should be, I'll submit the PR and update this issue with the link |
Either of those fixes look good. I normally use IO.Path personally, which might be because it's been around longer, but anything that works on VS 2017 and later should be fine. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: