classification
Title: [2.7] PCbuild/pythoncore.vcxproj kills the wrong program
Type: Stage: resolved
Components: Build Versions: Python 2.7
process
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: steve.dower, vstinner, zach.ware
Priority: normal Keywords: patch

Created on 2019-06-25 01:23 by vstinner, last changed 2019-06-25 23:39 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 14370 merged vstinner, 2019-06-25 10:37
Messages (9)
msg346471 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-25 01:23
x86 Windows XP 2.7:
https://buildbot.python.org/all/#/builders/45/builds/429

Killing any running python.exe instances...
...
C:\Program Files\MSBuild\Microsoft.Cpp\v4.0\Microsoft.CppClean.targets(74,5): warning : Access to the path 'd:\cygwin\home\db3l\buildarea\2.7.bolen-windows\build\PCBuild\python_d.exe' is denied. [d:\cygwin\home\db3l\buildarea\2.7.bolen-windows\build\PCbuild\python.vcxproj]

It's a debug build: it should kill python_d.exe, not python.exe.

--

PCbuild/pythoncore.vcxproj uses:

  <PropertyGroup>
    <MakeVersionInfoBeforeTarget>ClCompile</MakeVersionInfoBeforeTarget>
    <KillPython>true</KillPython>
  </PropertyGroup>

which comes from PCbuild/pyproject.vcxproj:

  <Target Name="KillPython" BeforeTargets="PrepareForBuild" Condition="'$(KillPython)' == 'true'">
    <Message Text="Killing any running python.exe instances..." Importance="high" />
    <KillPython FileName="$(OutDir)python$(PyDebugExt).exe" />
  </Target>

Problem: it seems like $(PyDebugExt) is not set to "_d" in PCbuild/pyproject.vcxproj.

Should PCbuild/pyproject.vcxproj explicitly import python.props which defines it?

    <!-- Suffix for all binaries when building for debug -->
    <PyDebugExt Condition="'$(PyDebugExt)' == '' and $(Configuration) == 'Debug'">_d</PyDebugExt>

Note: this issue is about Python 2.7.
msg346477 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2019-06-25 02:38
How can you tell that $(PyDebugExt) is not set?  Note that the "Killing any running python.exe instances..." line is hard-coded, it does not currently include $(PyDebugExt) (though it probably should).  Adding that to the message would be a good first step; backporting GH-9901 to 2.7 would probably also be good even though it was solving a different issue.
msg346506 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-25 10:38
> How can you tell that $(PyDebugExt) is not set?  Note that the "Killing any running python.exe instances..." line is hard-coded, it does not currently include $(PyDebugExt) (though it probably should).  Adding that to the message would be a good first step; ...

Oh... I didn't notice :-) I wrote PR 14370 to fix that.


> ...; backporting GH-9901 to 2.7 would probably also be good even though it was solving a different issue.

I'm not brave enough yet to make such large changes into Python 2.7 build system on Windows. I'm unable to build Python 2.7 for Windows for at least one year...
msg346514 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-25 11:37
New changeset 49032fa7e1cc1b6dc12b898daac24b75bae7572c by Victor Stinner in branch '2.7':
bpo-37396, PCbuild: Include "_d" in "Killing any running ..." message (GH-14370)
https://github.com/python/cpython/commit/49032fa7e1cc1b6dc12b898daac24b75bae7572c
msg346530 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-06-25 14:44
> Should PCbuild/pyproject.vcxproj explicitly import python.props which defines it?

It already does :)

This is more likely because the previous build on that buildbot crashed(?) or because it was aborted during test_multiprocessing:

https://buildbot.python.org/all/#/builders/45/builds/428
msg346533 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-25 15:14
Aaaah, CIs are funny.
msg346554 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-25 19:57
Me:
> Should PCbuild/pyproject.vcxproj explicitly import python.props which defines it?

Steve:
> It already does :) 


Well, I was confused by the message. I just fixed the message.

I suggest to close the issue and see if the bug comes back.
msg346566 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-06-25 22:41
> I suggest to close the issue and see if the bug comes back.

You opened it, so sure :)
msg346572 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-25 23:39
My question was more for Zachary who proposed: "backporting GH-9901 to 2.7 would probably also be good even though it was solving a different issue".

But as I wrote, I'm not comfortable to backport such change in Python 2.7. I prefer to avoid changes except if we really have to fix it.

So yeah, let's see if the bug strikes back. At least, we should now see if the build systems attempt to kill python.exe or python_d.exe.
History
Date User Action Args
2019-06-25 23:39:32vstinnersetmessages: + msg346572
2019-06-25 22:41:26steve.dowersetstatus: open -> closed

messages: + msg346566
stage: resolved
2019-06-25 19:57:03vstinnersetmessages: + msg346554
2019-06-25 15:14:06vstinnersetmessages: + msg346533
2019-06-25 14:44:26steve.dowersetmessages: + msg346530
2019-06-25 11:37:21vstinnersetmessages: + msg346514
2019-06-25 10:38:36vstinnersetmessages: + msg346506
stage: patch review -> (no value)
2019-06-25 10:37:03vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request14187
2019-06-25 02:38:15zach.waresetmessages: + msg346477
2019-06-25 01:23:57vstinnercreate