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

INCLUDEPY is incorrect #374

Closed
ngoldbaum opened this issue Oct 18, 2024 · 13 comments
Closed

INCLUDEPY is incorrect #374

ngoldbaum opened this issue Oct 18, 2024 · 13 comments
Assignees

Comments

@ngoldbaum
Copy link

ngoldbaum commented Oct 18, 2024

I'm experimenting with using a python-build-standalone python executable in CI on some projects like PyO3 and NumPy and am running into some issues.

The biggest one so far is that INCLUDEPY points to a non-existent path:

$  python -msysconfig | grep INCLUDEPY
	CONFINCLUDEPY = "/install/include/python3.13t"
	INCLUDEPY = "/install/include/python3.13t"

This makes it difficult to build C extensions against this version of Python, since it looks like libraries tend to check INCLUDEPY.

Interestingly, include is fine:

$ python -msysconfig | grep include
	include = "/Users/goldbaum/.local/share/uv/python/cpython-3.13.0+freethreaded-macos-aarch64-none/include/python3.13t"
	platinclude = "/Users/goldbaum/.local/share/uv/python/cpython-3.13.0+freethreaded-macos-aarch64-none/include/python3.13t"

I see that there is prior discussion about these /install paths being hard to update, although maybe since one path is updated we can update INCLUDEPY?

@ngoldbaum
Copy link
Author

We could also update the libraries to get the include path from 'include', but it would be nice if libraries that introspect Python to try to ask questions like this didn't have to get updates to work with python-build-standalone builds.

@ngoldbaum
Copy link
Author

@zanieb
Copy link
Member

zanieb commented Oct 18, 2024

Thanks for the report. I wonder how include gets derived? It's not like the build knows it'll be unpacked there. I worry this is an upstream CPython behavior?

@zanieb
Copy link
Member

zanieb commented Oct 18, 2024

@ngoldbaum
Copy link
Author

ngoldbaum commented Oct 18, 2024

I stepped through the standard library in a debugger. It looks like sys.get_path('include') ends up going into _expand_vars here, which then substitutes the correct path from the templates in the _INSTALL_SCHEMES dictionary.

> /Users/goldbaum/.local/share/uv/python/cpython-3.13.0+freethreaded-macos-aarch64-none/lib/python3.13t/sysconfig/__init__.py(277)_expand_vars()
-> value = os.path.expanduser(value)
(Pdb) p _INSTALL_SCHEMES[scheme]['include']
'{installed_base}/include/{implementation_lower}{py_version_short}{abiflags}'
(Pdb) p _subst_vars('{installed_base}/include/{implementation_lower}{py_version_short}{abiflags}', vars)
'/Users/goldbaum/.local/share/uv/python/cpython-3.13.0+freethreaded-macos-aarch64-none/include/python3.13t'

INCLUDEPY on the other hand it looks like is directly written to the binary and comes from get_config_vars(). So it looks like this is the intended behavior and maybe the downstream libraries are doing the incorrect thing by checking INCLUDEPY?

I can certainly make a PyO3 patch that checks sysconfig.get_path('include') if INCLUDEPY isn't a valid path. I already did that for NumPy in numpy/numpy#27581.

@zanieb
Copy link
Member

zanieb commented Oct 18, 2024

Thank you! That makes sense to me. The alternative is to change the INCLUDEPY value when the standalone distributions are installed — but I don't think we can do anything at build time.

@ngoldbaum
Copy link
Author

If it can be changed at install time I think that would be best, there's a lot of code out there checking for INCLUDEPY: https://github.com/search?q=INCLUDEPY&type=code

@zanieb
Copy link
Member

zanieb commented Oct 18, 2024

Related astral-sh/uv#7369

@zanieb
Copy link
Member

zanieb commented Oct 21, 2024

Opened astral-sh/uv#8429 to track this

@charliermarsh
Copy link
Member

If we rewrote INCLUDEPY at install time, would it be such that we point to an absolute path, referencing the destination to which we unpacked the Python?

@zanieb
Copy link
Member

zanieb commented Dec 11, 2024

I think so, yes.

@charliermarsh
Copy link
Member

This is now fixed at install-time in uv -- should we close out?

@charliermarsh charliermarsh self-assigned this Dec 17, 2024
@zanieb zanieb closed this as completed Dec 17, 2024
@zanieb
Copy link
Member

zanieb commented Dec 17, 2024

I think we may want to make it easier to apply the patches that uv does outside of uv, but perhaps we should track that separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants