[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Steve Dower]

Steve Dower  added the comment:


New changeset e6685ad05385f8cb492e8e1c7c07889a94517f55 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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Steve Dower]

Steve Dower  added the comment:


New changeset 7f70456b92c9ff0bcc4df2a2cec213ab2a897591 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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Steve Dower]

Change by Steve Dower :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 7.0 -> 8.0
pull_requests: +18712
pull_request: https://github.com/python/cpython/pull/19349

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[miss-islington]

Change by miss-islington :


--
pull_requests: +18713
pull_request: https://github.com/python/cpython/pull/19350

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Steve Dower]

Steve Dower  added the comment:


New changeset 6e623ff9d251e0ce86e9b18a01bfd6f067079d7a by Chris Martinez in 
branch 'master':
bpo-40158: Fix CPython MSBuild Properties in NuGet Package (GH-19343)
https://github.com/python/cpython/commit/6e623ff9d251e0ce86e9b18a01bfd6f067079d7a


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Chris Martinez]

Change by Chris Martinez :


--
keywords: +patch
pull_requests: +18707
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/19343

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Steve Dower]

Steve Dower  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Chris Martinez]

Chris Martinez  added the comment:

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.exe


Based 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 in
the package (that I could find). I'm not sure if this means the path is wrong, 
"python_d.exe" was accidentally
omiitted, or this property assignment simply should not exist. This current 
behavior will ultimately result in
the 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:

$(MSBuildThisFileDirectory)\..\..\tools

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 this
path using one of the following forms:

$([MSBuild]::NormalizePath($(MSBuildThisFileDirectory)\..\..\tools))

OR

$([System.IO.Path]::GetFullPath($(MSBuildThisFileDirectory)\..\..\tools))


In my particular case, the tooling I'm plugging this into wasn't happy unless 
the path was absolute. There
doesn't appear to be any reason or downside to resolving the path ahead of 
time. I can easily workaround
that 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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Steve Dower]

Steve Dower  added the comment:

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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue40158] MSBuild Extensions in CPython NuGet Package has Bad Expression

[Chris Martinez]

New submission from Chris Martinez :

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 " $(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.

--
components: Build, Demos and Tools, Distutils, Installation, Windows
messages: 365610
nosy: dstufft, eric.araujo, paul.moore, steve.dower, sydefekt, tim.golden, 
zach.ware
priority: normal
severity: normal
status: open
title: MSBuild Extensions in CPython NuGet Package has Bad Expression
type: compile error
versions: Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com