Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-82909: Update PC/pyconfig.h to allow disabling pragma based auto-linking#19740

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
zooba merged 5 commits intopython:mainfromjcfr:fix-issue-38728
Mar 10, 2025

Conversation

@jcfr
Copy link
Contributor

@jcfrjcfr commentedApr 27, 2020
edited by bedevere-bot
Loading

DefinePY_NO_LINK_LIB to build extension disabling pragma based
auto-linking. This is relevant when using build-system generator
(e.g CMake) where the linking is explicitly handled

https://bugs.python.org/issue38728

thewtex, kwryankrattiger, zhengzh, KerstinKeller, and Osyotr reacted with thumbs up emoji
@jcfrjcfr requested a review froma team as acode ownerApril 27, 2020 19:19
@the-knights-who-say-ni

This comment was marked as resolved.

@jcfr

This comment was marked as resolved.

@jcfr
Copy link
ContributorAuthor

jcfr commentedMay 6, 2020

@zooba Thanks for your patience, let me know if you need anything else to move forward with the integration 🙏

@mathstuf
Copy link

No news entry in Misc/NEWS.d/next/

@jcfr Seems like a release note could be added in the meantime.

jcfr reacted with thumbs up emoji

@mathstuf
Copy link

Ping? Any update here? It'd be nice to have this in 3.9.

@mathstuf
Copy link

Seems this will miss 3.10 as well.

@kwryankrattiger
Copy link

Ping. This would be really really nice to have sometime soon 👍

@zooba
Copy link
Member

I thought we'd already added this? I guess if it's not there, then we should.

Definitely needs a NEWS entry, probably deserves a What's New as well. Not sure if there's any place in the documentation that would suit, but if so, we should mention it for sure.

@jcfr
Copy link
ContributorAuthor

jcfr commentedDec 9, 2021

Definitely needs a NEWS entry

I can definitely add an entry, What about theC-API section ? Seehttps://github.com/python/cpython/tree/main/Misc/NEWS.d/next/C%20API

@arhadthedevarhadthedev changed the titlebpo-38728: Update PC/pyconfig.h to allow disabling pragma based auto-linkinggh-82909: Update PC/pyconfig.h to allow disabling pragma based auto-linkingApr 2, 2023
@arhadthedevarhadthedev added buildThe build process and cross-build OS-windows labelsApr 2, 2023
@arhadthedev
Copy link
Member

I added draft documentation and the NEWS entry for the macro. However, they need improvements.

@jcfr
Copy link
ContributorAuthor

jcfr commentedApr 3, 2023

@arhadthedev Thanks for your help moving this forward 🙏

arhadthedev reacted with thumbs up emoji

@zooba
Copy link
Member

I haven't tested the commands added to the docs, but if they do work as described and produce the files, then this looks fine to me.

@tasty0tomato
Copy link

It seems that if building Python by oneself, python's lib file will not be found in virtual environment. Hope this issue fixes as quickly as possible

@zooba
Copy link
Member

No, virtual environment is only for Python libraries. You'll need to look inbase_prefix to find native import libraries.

@tasty0tomato
Copy link

tasty0tomato commentedJun 26, 2023
edited
Loading

@zooba My question is like this:rosinality/stylegan2-pytorch#325 (comment)

So, is it more like to be a question about venv?

@zooba
Copy link
Member

It looks like Torch is deciding those settings, and it's deciding wrong about where to find the libs files.

There's a revamp ofsysconfig going on that will eventually provide a better option for them to find it, but for now, either they need to usesys.base_prefix instead ofsys.prefix, or if they already are (which they might be), then there's something wrong with your venv (maybe you've chained multiple venvs together in some unsupported way?)

@tasty0tomato
Copy link

@zooba I can give some additional informations. For exmaple,
The source code of python is on D:\Python-3.10.12,
The binary file of Windows and also the dll file are on D:\Python-3.10.12\PCbuild\amd64

And that's maybe why dll file cannot be found?

@zooba
Copy link
Member

Yeah, many tools are not going to work well when running from a CPython build directory. It's an unsupported configuration.

Try using the PC/layout tool (e.g.python PC\layout --copy <directory> --preset-default) to create an installed-shape directory. That will have thelibs at the usual place relative topython.exe. (You need to re-run the command if you rebuild Python, but it does incremental copying. Also, don't run the copied Python from the CPython source directory or it'll get the wrongLib dir.)

tasty0tomato reacted with thumbs up emoji

@mathstuf
Copy link

Any progress here? I see conflicts, so at least a rebase is in order. It'd be nice to get this into 3.13 at least…

@vstinner Interested in shepherding this C API change at all?

@zooba
Copy link
Member

A rebase ought to be enough, and when someone confirms that the build commands shown in the new documentation are correct then this can be merged (as per my last message).

@KerstinKeller
Copy link

This would potentially be extremly helpful, any chance that this will get merged at some point?

@zooba
Copy link
Member

It looks like the rebase isn't coming, so someone will need to submit a new PR.

It also looks like my last hesitation was that the commands in the docs weren't tested.

jcfrand others added5 commitsMarch 2, 2025 15:51
…linkingDefine PY_NO_LINK_LIB to build extension disabling pragma basedauto-linking. This is relevant when using build-system generator(e.g CMake) where the linking is explicitly handled
Copy link
Member

@zoobazooba left a comment

Choose a reason for hiding this comment

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

Nothing wrong with the PR as it is, but I think we can do a bit of tidying here if you want.

Comment on lines 311 to +314
#ifdefMS_COREDLL
# if !defined(Py_BUILD_CORE)&& !defined(Py_BUILD_CORE_BUILTIN)
/* not building the core - must be an ext */
# if defined(_MSC_VER)
# if defined(_MSC_VER)&& !defined(Py_NO_LINK_LIB)
Copy link
Member

Choose a reason for hiding this comment

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

This whole set of conditions is pretty ugly... any interest in simplifying?

Ideally in this order:

  • check if the feature is explicitly disabled (new variable)
  • check if we're in a context where it should be implicitly disabled (Py_BUILD_CORE[_BUILTIN])
  • check if the compiler supports it (_MSC_VER)

The order doesn't need to be spelled out to anyone reading it, but it should make it easiest to read (the first check has the best name) and easiest to maintain (alternate compilers can be supported in the "middle", not the edges).

It also really doesn't need to be indented by 8 characters. Four would be plenty.

/* Define Py_NO_LINK_LIB to build extension disabling pragma
based auto-linking.
This is relevant when using build-system generator (e.g CMake) where
the linking is explicitly handled */
Copy link
Member

Choose a reason for hiding this comment

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

This comment isn't important for this file, though a short header above the entire block (something like "automatically reference python3x.lib") might make it easier to understand the purpose without reading the whole way through.

@zooba
Copy link
Member

Guess we're not getting those improvements, so we'll leave them for later. Should be an easy first contribution for a sprint.

@zoobazooba merged commitc3487c9 intopython:mainMar 10, 2025
39 checks passed
@jcfr
Copy link
ContributorAuthor

Thanks everyone for the help moving this forward 🙏

Osyotr reacted with hooray emoji

Osyotr added a commit to Osyotr/vcpkg that referenced this pull requestApr 5, 2025
seehwan pushed a commit to seehwan/cpython that referenced this pull requestApr 16, 2025
…auto-linking (pythonGH-19740)Define Py_NO_LINK_LIB to build extension disabling pragma based auto-linking. This is relevant when using build-system generator (e.g CMake) where the linking is explicitly handled
:file:`Python.h` triggers an implicit, configure-aware link with the
library. The header file chooses:file:`pythonXY_d.lib` for Debug,
:file:`pythonXY.lib` for Release, and:file:`pythonX.lib` for Release with
the `Limited API<stable-application-binary-interface>`_ enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

See also#134830

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@vstinnervstinnervstinner left review comments

@zoobazoobazooba left review comments

+2 more reviewers

@mathstufmathstufmathstuf left review comments

@hroncokhroncokhroncok left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

buildThe build process and cross-buildOS-windows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

12 participants

@jcfr@the-knights-who-say-ni@mathstuf@kwryankrattiger@zooba@arhadthedev@tasty0tomato@KerstinKeller@vstinner@hroncok@ezio-melotti@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp