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

Configure pkg-config during CPython builds #460

Merged
merged 1 commit into from
Jan 1, 2025
Merged

Configure pkg-config during CPython builds #460

merged 1 commit into from
Jan 1, 2025

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Dec 30, 2024

Closes #449

@zanieb zanieb added platform:darwin Specific to the macOS platform platform:linux Specific to the Linux platform labels Dec 30, 2024
@zanieb
Copy link
Member Author

zanieb commented Dec 30, 2024

I need to validate this works on other Python versions too, I only tested 3.14 in #457.

A bit separately, I'm not sure how the SQLite checks passed without the patch at python/cpython#128322 since that was required in a test outside of python-build-standalone (as reported in the issue linked there).

Copy link
Collaborator

@indygreg indygreg left a comment

Choose a reason for hiding this comment

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

This looks correct. I'm surprised a non-working pkg-config survived this long in the project's history!

Although TBH I'm not sure how much it mattered before the C extensions stuff was ported to configure. And even post configure, our approach has historically been to spit out a Modules file explicitly defining how to compile extension modules, bypassing the CPython build system nearly entirely.

@indygreg
Copy link
Collaborator

Running just diff-python-json against this branch yields the following changes on a 3.13 Linux build:

  • HAVE_EDITLINE_READLINE_H 1 -> 0
  • HAVE_LIBSQLITE3 0 -> 1
  • HAVE_LZMA_H 1 -> 0
  • HAVE_RL_APPEND_HISTORY 0 -> 1
  • HAVE_RL_CATCH_SIGNAL 0 -> 1
  • HAVE_RL_COMPLETION_APPEND_CHARACTER 0 -> 1
  • HAVE_RL_COMPLETION_DISPLAY_MATCHES_HOOK 0 -> 1
  • HAVE_RL_COMPLETION_MATCHES 0 -> 1
  • HAVE_RL_COMPLETION_SUPPRESS_APPEND 0 -> 1
  • HAVE_RL_PRE_INPUT_HOOK 0 -> 1
  • HAVE_RL_RESIZE_TERMINAL 0 -> 1
  • PY_SQLITE_HAVE_SERIALIZE 0 -> 1
  • Py_RL_STARTUP_HOOK_TAKES_ARGS 0 -> 1
  • WITH_EDITLINE 0 -> 1

So it looks like some readline/libedit config changes snuck in here as well. I want to say the new state is completely correct.

The only concerning change here is HAVE_LZMA_H going to off. Maybe the configure script was pulling in the Docker image's libxz via its pkg-config? As long as we build a functional xz extension module, I think we're good here though.

@indygreg
Copy link
Collaborator

And if we run a full just diff, we see changes in the following object files:

  • python/build/Modules/_sqlite/connection.o
  • python/build/Modules/readline.o

No changes to anything libxz related, which is good.

If you look at the raw changes to the ELF binaries, it looks like a handful of new symbol references are introduced, which means Python is taking advantage of features that weren't detected/enabled before.

So this all looks good to me.

(It might be a good idea to have CI run diffoscope against the base branch and post a summary to the PR to make this kind of analysis more turnkey.)

@zanieb
Copy link
Member Author

zanieb commented Dec 30, 2024

Thanks for doing those comparisons Greg! I was thinking it'd be wise to check what else changed but wasn't sure what the best way to do so is yet. I was going to diff the logs as a starting point 😭

(It might be a good idea to have CI run diffoscope against the base branch and post a summary to the PR to make this kind of analysis more turnkey.)

That'd be sweet. I'll look into that.

@zanieb zanieb merged commit 4c4d347 into main Jan 1, 2025
297 checks passed
@zanieb zanieb deleted the zb/sqlite-fix branch January 1, 2025 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform:darwin Specific to the macOS platform platform:linux Specific to the Linux platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

python-build-standalone ships sqlite3 module missing Connection.serialize, deserialize
3 participants