Skip to content

gh-127405: Remove dead code in sysconfig._get_pybuilddir() #131935

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

Closed
wants to merge 17 commits into from

Conversation

XuehaiPan
Copy link
Contributor

@XuehaiPan XuehaiPan commented Mar 31, 2025

The return value for get_config_var('Py_DEBUG') is one of 0, 1, None. It cannot be a str.

@XuehaiPan
Copy link
Contributor Author

How can I add the "skip news" label?

@brianschubert
Copy link
Contributor

Hmm, I think something might be outdated here. When I build with --with-pydebug, the build dir is build/lib.<platform>-<version>, not build/lib.<platform>-<version>-pydebug:

$ rm -r build
$ make clean && ./configure --with-pydebug --prefix=$(pwd) && make -j

$ ./python -c 'import sysconfig; print(sysconfig.get_config_var("Py_DEBUG"))'
1

$ ls -l build 
total 16
drwxrwxr-x 3 brian brian 12288 Mar 31 09:17 lib.linux-x86_64-3.14
drwxrwxr-x 2 brian brian  4096 Mar 31 09:17 scripts-3.14

Maybe the right fix is to remove that branch altogether?

How can I add the "skip news" label?

If any user-facing behavior changes, there should be a news entry. Right now this PR changes where -m sysconfig --generate-posix-vars writes its output files, so that would require a news entry.

@XuehaiPan
Copy link
Contributor Author

Maybe the right fix is to remove that branch altogether?

Move to this approach.

@XuehaiPan XuehaiPan changed the title gh-127405: Fix get_config_var('Py_DEBUG') usage in sysconfig._get_pybuilddir() gh-127405: Remove dead code in sysconfig._get_pybuilddir() Mar 31, 2025
@@ -161,10 +161,7 @@ def _print_config_dict(d, stream):


def _get_pybuilddir():
pybuilddir = f'build/lib.{get_platform()}-{get_python_version()}'
if get_config_var('Py_DEBUG') == '1':
pybuilddir += '-pydebug'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any way for this to be called, but I note that this behavior is tied to

pydebug = lib_dir.endswith("-pydebug")

I think this aspect of the WASM build is broken, and if this code can be deleted then perhaps that trail can too.

@XuehaiPan
Copy link
Contributor Author

The complexity of fixing the WASM build with the correct path is beyond what I expected.

@XuehaiPan XuehaiPan closed this Apr 1, 2025
@FFY00
Copy link
Member

FFY00 commented Apr 1, 2025

I think you are trying to fix GH-131769. GH-131761 should address it if it gets merged.

GH-131844 should fix the debug WASM builds, and GH-131855 proposes a more reliable way to set the stack size limit.

@XuehaiPan XuehaiPan deleted the fix-py-debug-sysconfig branch April 2, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants