python / cpython

The Python programming language
https://www.python.org
Other
63.36k stars 30.34k forks source link

"Building C and C++ Extensions on Windows" docs are very out-of-date #87970

Open 1e2b3793-5146-46c5-b55f-27b9e782b291 opened 3 years ago

1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago
BPO 43804
Nosy @pfmoore, @tjguk, @zware, @zooba, @WildCard65, @shreyanavigyan
PRs
  • python/cpython#25343
  • python/cpython#26020
  • Files
  • patch.patch: Patch File
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['3.8', '3.9', '3.10', '3.11', 'OS-windows', 'docs'] title = '"Building C and C++ Extensions on Windows" docs are very out-of-date' updated_at = user = 'https://github.com/shreyanavigyan' ``` bugs.python.org fields: ```python activity = actor = 'shreyanavigyan' assignee = 'docs@python' closed = False closed_date = None closer = None components = ['Documentation', 'Windows'] creation = creator = 'shreyanavigyan' dependencies = [] files = ['49973'] hgrepos = [] issue_num = 43804 keywords = ['patch'] message_count = 19.0 messages = ['390773', '390883', '391052', '391144', '391342', '391460', '391608', '391609', '391615', '391618', '391667', '393380', '393405', '393406', '393409', '393412', '393413', '393414', '393415'] nosy_count = 7.0 nosy_names = ['paul.moore', 'tim.golden', 'docs@python', 'zach.ware', 'steve.dower', 'WildCard65', 'shreyanavigyan'] pr_nums = ['25343', '26020'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = None url = 'https://bugs.python.org/issue43804' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    In the context of Docs/extending/windows.rst, the command provided, for compiling the source code and linking the libraries to create a DLL, only works for Python 32-bit versions. But the documentation doesn't inform anything about that. The PR linked with this issue adds a note section with the text :-

    """

    The above commands are only applicable for Python 32-bit versions.

    """

    This PR is against the master branch and if accepted, it needs to be backported to all the previous supported versions.

    Kindly have a review of my PR.

    Thanking you,

    With Regards

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Kindly have a review of my PR.

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Kindly have a review of my PR.

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Please have a review.

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Kindly have a review of my PR.

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Kindly have a review of my patch

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Ping

    zware commented 3 years ago

    Please have some patience. Constant requests for review or pings are frankly quite annoying. Everyone on this project is a volunteer, and as such things only get done when someone has the time and inclination to do them. If your issue/PR has sat idle for a month or so, then it may be time to consider a ping, but a ping every two days will quickly come to be seen as spam and ignored.

    zooba commented 3 years ago

    I agree with Zach's comment on the PR - this topic needs an overhaul.

    All the information required is going to be out there somewhere, and those of us on this issue can help fill in any gaps, but someone will need to spend a bit of time on research and laying it out nicely.

    This is a great contribution opportunity for someone who would rather contribute writing and research rather than just code. However, it is a tough area, and so any improvements will be pretty well scrutinised.

    Good docs here will be very impactful throughout the community, so let's make sure they're good.

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Both https://docs.python.org/3/extending/windows.html#building-on-windows and https://docs.python.org/3/extending/building.html recommends using distutils. As mentioned in PEP-632 distutils will soon be deprecated. I think it'll be a good idea to change the docs to recommend using setuptools and also change the source code for setup.py to use setuptools instead of distutils.

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    I'm attaching a patch for improving "Building C and C++ Extensions on Windows" and "Building C and C++ Extensions" docs.

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Ping

    cbbc42d0-1c72-4dd1-93d4-8553b09ac1c0 commented 3 years ago

    I'm quite familiar with MSVC's command line and I'm quite confused on what you mean "the above commands are specific to 32-bit Python"

    "/LD" is available for both 32-bit and 64-bit compilations, it implies "/MT" to the compiler and "/DLL" to the linker.

    "/I" is available for both 32-bit and 64-bit compilations.

    Python's lib files are named exactly the same between 32-bit and 64-bit versions.

    The only thing platform specific in MSVC is the compiler. Visual Studio's C/C++ build tools ships with 4 variants of MSVC:

    2 32-bit versions, 1 for targeting 32-bit and the other for targeting 64-bit (32-bit native, 32-bit cross compile to 64-bit)

    The same is true for the 64-bit versions (64-bit native and 64-bit cross compile to 32-bit)

    Internally, these are known as: x86, x86_x64, x64, x64_x86

    zware commented 3 years ago

    Since the removal of Rietveld from the tracker, it's not easy to review a patch as a patch file. Please open a PR instead.

    As I said on the first PR, and as William says above, I don't see any indication that your assertion that some commands are 32-bit only is true. Please provide some evidence for your claim.

    zooba commented 3 years ago

    I still don't think the patch goes anywhere near far enough to be useful.

    Most likely we should preface that section with a "we recommend using a third-party build backend for your package, such as ... and refer to packaging.python.org for up to date suggestions" and then the rest of the section shows literally the build steps to compile an extension module (so that build backend authors have a reference).

    We should *not have a section on using setuptools, or if we do, it should consist entirely of a link to the latest setuptools documentation. Their interface is not tied to our release schedule, and could change at any time, invalidating our docs (which is a *good thing, provided we haven't documented them).

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    Zachary:

    As I said on the first PR, and as William says above, I don't see any indication that your assertion that some commands are 32-bit only is true. Please provide some evidence for your claim.

    cl.exe by default points to the 32-bit cl.exe not the 64-bit cl.exe. And also the library used by cl.exe by default is also 32-bit. Therefore there seems to be a linker issue. And I've tested that. It's true.

    Steve:

    Most likely we should preface that section with a "we recommend using a third-party build backend for your package, such as ... and refer to packaging.python.org for up to date suggestions" and then the rest of the section shows literally the build steps to compile an extension module (so that build backend authors have a reference). We should *not have a section on using setuptools, or if we do, it should consist entirely of a link to the latest setuptools documentation. Their interface is not tied to our release schedule, and could change at any time, invalidating our docs (which is a *good thing, provided we haven't documented them).

    I'll remove the changes in Building C and C++ Extensions. I'll also make sure no changes are referencing setuptools. I'll be opening PR very soon.

    cbbc42d0-1c72-4dd1-93d4-8553b09ac1c0 commented 3 years ago

    Then it appears you're using a version of the compiler that is built for building 32-bit exes/dlls, you need to use the x64 version.

    For this you need to start either the x86 Cross Tools console or the x64 native console (VS 2019) or use vcvarsall.cmd/Enter-VsDevShell (PowerShell)

    1e2b3793-5146-46c5-b55f-27b9e782b291 commented 3 years ago

    I'm aware of that. But anyway I'm removing it since there are ways to overcome it.

    cbbc42d0-1c72-4dd1-93d4-8553b09ac1c0 commented 3 years ago

    Correction: You can use either VsDevCmd.bat/Enter-VsDevShell on VS versions that provide them (2017 and 2019 are known to include it), but to get the x64 tools you need to pass command line arguments (They default to x86 native tools).

    Otherwise you must use either vcvarsall.cmd and pass the appropriate command line arguments or vcvarsx86_amd64.bat/vcvars64.bat