Skip to content

Conversation

@zware
Copy link
Member

@zwarezware commented Feb 8, 2024

@zwarezware requested review from a team and rhettinger as code ownersFebruary 8, 2024 20:49
@bedevere-appbedevere-appbot mentioned this pull request Feb 8, 2024
15 tasks
@zwarezwareforce-pushed the add_mpdecimal_external branch from 3cfe39b to 7fc2f57CompareFebruary 8, 2024 20:50
@zwarezware marked this pull request as draft February 8, 2024 21:14
@rhettingerrhettinger removed their request for review February 8, 2024 21:19
@zware
Copy link
MemberAuthor

zware commented Feb 8, 2024

I missed that mpdecimal.h does not actually exist in the source distribution, but is created (or copied) by the building of the library. This will need to be handled somehow, either by adding a Windows-specific shim that picks the right mpdecimal{32,64}vc.h to include somewhere, or by building the library separately using its own build scripts.

@zwarezwareforce-pushed the add_mpdecimal_external branch from 22bfcc4 to c128618CompareFebruary 25, 2024 20:25
@zwarezware marked this pull request as ready for review February 25, 2024 22:01
@zware
Copy link
MemberAuthor

Made it back around to this and fixed the build issue. I don't like the added Modules/_decimal/windows/mpdecimal.h as a permanent solution, but I think it's a decent stopgap to avoid changing the sources somehow. Ideally we'll eventually switch to using mpdecimal's own PGO build script to produce binaries, but that will be a bigger change that we can make happen later. It will also require some attention on ARM, unless 4.0.0 already has handling for that.

@zware
Copy link
MemberAuthor

!buildbot Windows

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @zware for commit c128618 🤖

The command will test the builders whose names match following regular expression: Windows

The builders matched are:

  • AMD64 Windows11 Bigmem PR
  • AMD64 Windows11 Refleaks PR
  • ARM64 Windows Non-Debug PR
  • AMD64 Windows10 PR
  • ARM64 Windows PR
  • AMD64 Windows11 Non-Debug PR
  • AMD64 Windows Server 2022 NoGIL PR

@zwarezware requested a review from sethmlarson as a code ownerMarch 15, 2024 21:08
@zooba
Copy link
Member

@zware This looks like it's good enough shape - any reason to not merge yet?

@zware
Copy link
MemberAuthor

The limitation of 24h/day, mostly :)

@zwarezware merged commit 849e071 into python:mainMar 18, 2024
@zwarezware deleted the add_mpdecimal_external branch March 18, 2024 17:07
@bedevere-bot

This comment was marked as off-topic.

vstinner pushed a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
…-115182) This includes adding what should be a relatively temporary `Modules/_decimal/windows/mpdecimal.h` shim to choose between `mpdecimal32vc.h` or `mpdecimal64vc.h` based on which of `CONFIG_64` or `CONFIG_32` is defined.
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
…-115182) This includes adding what should be a relatively temporary `Modules/_decimal/windows/mpdecimal.h` shim to choose between `mpdecimal32vc.h` or `mpdecimal64vc.h` based on which of `CONFIG_64` or `CONFIG_32` is defined.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
…-115182) This includes adding what should be a relatively temporary `Modules/_decimal/windows/mpdecimal.h` shim to choose between `mpdecimal32vc.h` or `mpdecimal64vc.h` based on which of `CONFIG_64` or `CONFIG_32` is defined.
Sign up for freeto 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

@zware@bedevere-bot@zooba