
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2020-04-02 18:00 bysydefekt, last changed2022-04-11 14:59 byadmin. This issue is nowclosed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 19343 | merged | sydefekt,2020-04-03 17:42 | |
| PR 19349 | merged | miss-islington,2020-04-03 20:04 | |
| PR 19350 | merged | miss-islington,2020-04-03 20:04 | |
| Messages (7) | |||
|---|---|---|---|
| msg365610 -(view) | Author: Chris Martinez (sydefekt)* | Date: 2020-04-02 18:00 | |
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.pyare 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 " $(Property) == 'value' ", but should instead use " '$(Property)' == 'value' ". By not surrounding the property value with '', the condition may resolve as " == '' ", which is an invalid expression and will cause a build failure. This doesn't appear to have caused an issue yet, but it easily could.If there is no further discussion or objection, I can submit a PR with the required fixes. | |||
| msg365613 -(view) | Author: Steve Dower (steve.dower)*![]() | Date: 2020-04-02 18:16 | |
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). | |||
| msg365647 -(view) | Author: Chris Martinez (sydefekt)* | Date: 2020-04-02 23:24 | |
In testing the fix, another issue has arisen. It appears the specified expression will never yield a usable path.Expression 1:$([msbuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), "python_d.exe"))Expression 2:$([msbuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), "python.exe"))The package has the abridged structure of:┌─ build│ ││ └─ native│ ││ └─ python.props│└─ tools │ └─ python.exeBased on this hierachy, neither exe will resolve because they do not have the same common ancestor. Additionally,I found that "python_d.exe" is always assumed for "Configuration=Debug", but "python_d.exe" does not exist inthe package (that I could find). I'm not sure if this means the path is wrong, "python_d.exe" was accidentallyomiitted, or this property assignment simply should not exist. This current behavior will ultimately result inthe build integration failing because "python_d.exe" is resolved, but it doesn't exist.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.If it is needed, then the path needs to be fixed. To make the path more robust, I also recommend resolving thispath using one of the following forms:<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. Theredoesn't appear to be any reason or downside to resolving the path ahead of time. I can easily workaroundthat issue, but I suspect resolving an absolute path may be useful to other package consumers as well.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 | |||
| msg365685 -(view) | Author: Steve Dower (steve.dower)*![]() | Date: 2020-04-03 09:27 | |
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. | |||
| msg365721 -(view) | Author: Steve Dower (steve.dower)*![]() | Date: 2020-04-03 20:03 | |
New changeset6e623ff9d251e0ce86e9b18a01bfd6f067079d7a by Chris Martinez in branch 'master':bpo-40158: Fix CPython MSBuild Properties in NuGet Package (GH-19343)https://github.com/python/cpython/commit/6e623ff9d251e0ce86e9b18a01bfd6f067079d7a | |||
| msg365732 -(view) | Author: Steve Dower (steve.dower)*![]() | Date: 2020-04-03 22:18 | |
New changeset7f70456b92c9ff0bcc4df2a2cec213ab2a897591 by Miss Islington (bot) in branch '3.7':bpo-40158: Fix CPython MSBuild Properties in NuGet Package (GH-19343)https://github.com/python/cpython/commit/7f70456b92c9ff0bcc4df2a2cec213ab2a897591 | |||
| msg365733 -(view) | Author: Steve Dower (steve.dower)*![]() | Date: 2020-04-03 22:20 | |
New changesete6685ad05385f8cb492e8e1c7c07889a94517f55 by Miss Islington (bot) in branch '3.8':bpo-40158: Fix CPython MSBuild Properties in NuGet Package (GH-19343)https://github.com/python/cpython/commit/e6685ad05385f8cb492e8e1c7c07889a94517f55 | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:28 | admin | set | github: 84339 |
| 2020-04-03 22:20:16 | steve.dower | set | messages: +msg365733 |
| 2020-04-03 22:18:32 | steve.dower | set | messages: +msg365732 |
| 2020-04-03 20:05:25 | steve.dower | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2020-04-03 20:04:16 | miss-islington | set | pull_requests: +pull_request18713 |
| 2020-04-03 20:04:08 | miss-islington | set | nosy: +miss-islington pull_requests: +pull_request18712 |
| 2020-04-03 20:03:58 | steve.dower | set | messages: +msg365721 |
| 2020-04-03 17:42:39 | sydefekt | set | keywords: +patch stage: patch review pull_requests: +pull_request18707 |
| 2020-04-03 09:27:55 | steve.dower | set | messages: +msg365685 |
| 2020-04-02 23:24:48 | sydefekt | set | messages: +msg365647 |
| 2020-04-02 18:16:31 | steve.dower | set | messages: +msg365613 |
| 2020-04-02 18:00:31 | sydefekt | create | |