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

fix library dedection for pypy 3.9 #673

Merged
merged 5 commits into from
Mar 14, 2022
Merged

fix library dedection for pypy 3.9 #673

merged 5 commits into from
Mar 14, 2022

Conversation

rkaminsk
Copy link
Contributor

@rkaminsk rkaminsk commented Mar 5, 2022

The library name in pypy 3.9 has changed. The '-c' moved to the end. This PR patches the candidate matrix to "make it work". It doesn't really fit nicely in the current matrix, though.

@mattip
Copy link
Contributor

mattip commented Mar 6, 2022

Where does sysconfig.get_config_var('LDLIBRARY') fail to return the correct link library?

@rkaminsk
Copy link
Contributor Author

rkaminsk commented Mar 6, 2022

Where does sysconfig.get_config_var('LDLIBRARY') fail to return the correct link library?

scikit-build is not using LDLIBRARY. But I also tried if it would work using it. Unfortunately, the value reported by the sysconfig module does not include the version number and I could not use it.

$ ./pypy3.9-v7.3.8-linux64/bin/pypy -m sysconfig | grep LDLIB
	LDLIBRARY = "libpypy3-c.so"

$ ls -1 ./pypy3.9-v7.3.8-linux64/bin/lib*
./pypy3.9-v7.3.8-linux64/bin/libpypy3.9-c.so
./pypy3.9-v7.3.8-linux64/bin/libpypy3.9-c.so.debug

@mattip
Copy link
Contributor

mattip commented Mar 6, 2022

Ahh, right, that was a bug in the release and was fixed. It will be part of this week's release.

@rkaminsk
Copy link
Contributor Author

rkaminsk commented Mar 6, 2022

Ahh, right, that was a bug in the release and was fixed. It will be part of this week's release.

Okay, then we should wait to merge this PR. I will have a look again once the new release is out. Even though the current state should also work with the upcoming release.

@rkaminsk
Copy link
Contributor Author

@mattip, I can confirm that LDLIBRARY is set correctly in the nightly build. But there is no updated release yet.

@henryiii, this pact tries to avoid guessing the library path by constructing it from config vars. If this fails, it falls back to the matrix to guess the path. The matrix has also been adjusted to work with newer pypy releases. Just in case. What do you think?

@mattip
Copy link
Contributor

mattip commented Mar 13, 2022

But there is no updated release yet.

I will be releasing a new version once CPython releases the upcoming security fix release (March 15?). I want to make sure I got all the fixes into PyPy.

@henryiii henryiii merged commit e6a72ce into scikit-build:master Mar 14, 2022
@henryiii
Copy link
Contributor

Fantastic, thank you very much!

@henryiii henryiii added this to the 0.14.0 milestone Mar 30, 2022
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

Successfully merging this pull request may close these issues.

3 participants