Skip to content
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

Jump to bottom

bpo-47103: Copy pgort140.dll into output directory when building PGInstrument on Windows #32083

Merged
merged 4 commits into from Apr 6, 2022
Merged

bpo-47103: Copy pgort140.dll into output directory when building PGInstrument on Windows #32083

merged 4 commits into from Apr 6, 2022
+18 −18

Conversation

zooba
Copy link
Member

@zooba zooba commented Mar 23, 2022 •
edited by bedevere-bot

@zooba zooba requested a review from a team as a code owner Mar 23, 2022
@zooba zooba closed this Mar 24, 2022
@zooba zooba reopened this Mar 24, 2022
@neonene
Copy link
Contributor

neonene commented Mar 25, 2022

The following 2 tests in test_embed.py failed with the same error:

test_init_pybuilddir_win32
test_init_pyvenv_cfg

Traceback (most recent call last):
  File "C:\cp\Lib\contextlib.py", line 155, in __exit__
    self.gen.throw(typ, value, traceback)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\cp\Lib\test\test_embed.py", line 1225, in tmpdir_with_python
    yield tmpdir
    ^^^^^^^^^^^^
  File "C:\cp\Lib\test\test_embed.py", line 1347, in test_init_pybuilddir_win32
    self.check_all_configs("test_init_compat_config", config,
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\cp\Lib\test\test_embed.py", line 738, in check_all_configs
    out, err = self.run_embedded_interpreter(testname,
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\cp\Lib\test\test_embed.py", line 117, in run_embedded_interpreter
    self.assertEqual(p.returncode, returncode,
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 3221225781 != 0 : bad returncode 3221225781, stderr is ''

@neonene neonene mentioned this pull request Mar 28, 2022
Closed
neonene
neonene reviewed Apr 5, 2022
View changes
PCbuild/python.vcxproj
@@ -132,9 +132,6 @@ set PYTHONPATH=$(PySourcePath)Lib
</Target>
<Target Name="GeneratePythonBat" AfterTargets="AfterBuild">
<PropertyGroup>
Copy link
Contributor

@neonene neonene Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<PropertyGroup>
<PropertyGroup>
<_PGOPath Condition="$(Configuration) == 'PGInstrument'">@set PATH=%PATH%%3B$(OutDir)</_PGOPath>

Copy link
Contributor

@neonene neonene Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving $(_PGOPath) like this fixes my errors. The way is more generic than #32146.

Copy link
Member Author

@zooba zooba Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, your other PR is better. We shouldn't rely on PATH for resolving DLLs, but should copy the right one alongside the executable.

Copy link
Member Author

@zooba zooba Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I see you've closed it, so I'll add a similar change to this PR.

Copy link
Contributor

@neonene neonene Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The additional change is perfect for PGO/non-PGO.
I can confirm PGInstrument builds work with this.

@zooba zooba merged commit 074da78 into python:main Apr 6, 2022
12 checks passed
@zooba zooba deleted the bpo-47103 branch Apr 6, 2022
@miss-islington
Copy link
Contributor

miss-islington commented Apr 6, 2022

Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Apr 6, 2022

Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Apr 6, 2022

Sorry, @zooba, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 074da788028c1f1e867dc81698efdcdc263f2288 3.9

@miss-islington
Copy link
Contributor

miss-islington commented Apr 6, 2022

Sorry @zooba, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 074da788028c1f1e867dc81698efdcdc263f2288 3.10

zooba added a commit to zooba/cpython that referenced this pull request Apr 6, 2022
@zooba
bpo-47103: Copy pgort140.dll into output directory when building PGIn…
2bcb6ca 
…strument on Windows (pythonGH-32083)
@bedevere-bot
Copy link

bedevere-bot commented Apr 6, 2022

GH-32366 is a backport of this pull request to the 3.10 branch.

zooba added a commit that referenced this pull request Apr 7, 2022
@zooba
bpo-47103: Copy pgort140.dll into output directory when building PGIn…
b0ec17b 
…strument on Windows (GH-32083)
@bedevere-bot
Copy link

bedevere-bot commented Apr 7, 2022

GH-32407 is a backport of this pull request to the 3.9 branch.

zooba added a commit that referenced this pull request Apr 8, 2022
@zooba
bpo-47103: Copy pgort140.dll into output directory when building PGIn…
80c1153 
…strument on Windows (GH-32083)
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
@zooba @hello-adam
bpo-47103: Copy pgort140.dll into output directory when building PGIn…
d5d4f91 
…strument on Windows (pythonGH-32083)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neonene neonene

@python/windows-team windows-team

Assignees

@zooba zooba

Labels
OS-windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@zooba @neonene @miss-islington @bedevere-bot @the-knights-who-say-ni