GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-45582: Fix test_embed failure during a PGO build on Windows by neonene · Pull Request #30014 · python/cpython

Conversation

@neonene

@neonene neonene commented Dec 9, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

PGO-instrumented _testembed.exe and getpath.py often miscalculate the prefix path, when launched with all PYTHON* envvars removed.

The new path calculation uses VPATH, which is defined as "..\\.." in pythoncore.vcxproj. From PGO-instrumented folder, VPATH needs to be "..\\..\\.." to resolve prefix.

https://bugs.python.org/issue45582

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
neonene added 3 commits December 10, 2021 02:32
@neonene
VPATH definition exclusive for PGInstrument
1e6bb41
@neonene
Use VPATH for BUILD_LANDMARK
24d03eb
@neonene
Use different VPATH in test_init_pybuilddir_win32
0529d08
@neonene
neonene requested a review from a team as a code owner December 9, 2021 18:32
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Dec 9, 2021
@bedevere-bot bedevere-bot added the awaiting review label Dec 9, 2021
@neonene neonene mentioned this pull request Dec 9, 2021
bpo-45582: Change the folder location of PGO-instrumented builds on Windows #29941
Closed
@neonene
Merge branch 'python:main' into bpo-45582_pgo2
6662755
@zooba

zooba commented Dec 9, 2021

Copy link
Copy Markdown
Member

I like this change better, but maybe we should get VPATH into sysconfig so that it can be used in the test?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zooba

zooba commented Dec 9, 2021

Copy link
Copy Markdown
Member

maybe we should get VPATH into sysconfig so that it can be used in the test?

I think adding it as sys._vpath but only on Windows is fine for this. Then sysconfig can add it into its dict if it's there.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
neonene added 7 commits December 10, 2021 18:20
@neonene
VPATH definition for MSBuild
737cedb
@neonene
VPATH for sysmodule.c / getpath.c
78e1fdb
@neonene
VPATH for sysmodule.c (freezed)
678a408
@neonene
Implement sys._vpath
bf34c4f
@neonene
sysconfig.get_config_var('VPATH')
7f8e0e3
@neonene
Use VPATH in test_embed
ee17a52
@neonene
Fix indent in test_embed
f3ac09c
@neonene

neonene commented Dec 10, 2021

Copy link
Copy Markdown
Contributor Author

Thank you for the nice idea. I've finished the implementation.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@neonene
Fix a redundant definition
2513c67
@zooba zooba added the skip news label Dec 10, 2021
@zooba
zooba merged commit 3f398a7 into python:main Dec 10, 2021
@bedevere-bot bedevere-bot removed the awaiting review label Dec 10, 2021
@neonene
neonene deleted the bpo-45582_pgo2 branch December 11, 2021 01:42
@zooba zooba mentioned this pull request Apr 10, 2022
Rewrite getpath.c in Python #89745
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@neonene @zooba @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.