python / cpython

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

Update to libmpdec-2.5.0 #85051

Closed 5531d0d8-2a9c-46ba-8b8b-ef76132a492c closed 4 years ago

5531d0d8-2a9c-46ba-8b8b-ef76132a492c commented 4 years ago
BPO 40874
Nosy @rhettinger, @doko42, @mdickinson, @pitrou, @skrah, @ambv, @asottile, @miss-islington
PRs
  • python/cpython#20652
  • python/cpython#20654
  • python/cpython#21202
  • python/cpython#21203
  • 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 = 'https://github.com/skrah' closed_at = created_at = labels = ['extension-modules', 'type-bug', '3.9', '3.10'] title = 'Update to libmpdec-2.5.0' updated_at = user = 'https://github.com/skrah' ``` bugs.python.org fields: ```python activity = actor = 'skrah' assignee = 'skrah' closed = True closed_date = closer = 'skrah' components = ['Extension Modules'] creation = creator = 'skrah' dependencies = [] files = [] hgrepos = [] issue_num = 40874 keywords = ['patch'] message_count = 58.0 messages = ['370766', '370774', '370780', '372530', '372532', '372534', '372579', '372580', '372581', '372584', '372585', '372586', '372587', '372589', '372591', '372593', '372597', '372598', '372608', '372609', '372613', '372614', '372615', '372616', '372617', '372618', '372620', '372623', '372626', '372629', '372632', '372635', '372917', '372918', '372919', '372920', '372921', '372922', '372924', '372926', '372928', '372930', '372931', '372932', '372935', '372943', '372944', '372945', '372946', '372947', '372948', '373001', '373003', '373005', '373021', '373093', '373094', '373096'] nosy_count = 8.0 nosy_names = ['rhettinger', 'doko', 'mark.dickinson', 'pitrou', 'skrah', 'lukasz.langa', 'Anthony Sottile', 'miss-islington'] pr_nums = ['20652', '20654', '21202', '21203'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue40874' versions = ['Python 3.9', 'Python 3.10'] ```

    5531d0d8-2a9c-46ba-8b8b-ef76132a492c commented 4 years ago

    How witty!

    pitrou commented 4 years ago

    Hmm, I'm only taking notice of this comment thread now.

    (sorry, but due to spam filtering issues I only receive bpo e-mail notifications intermittently... and that's despite having tried two separate e-mail providers which otherwise give me no problems :-/)

    In any case, I would have had a hard time giving a competent opinion on this issue. But I'm a bit saddened by how heated the discussion went. Hopefully this is all over.

    5531d0d8-2a9c-46ba-8b8b-ef76132a492c commented 4 years ago

    In any case, I would have had a hard time giving a competent opinion on this issue.

    Essentially it's a really simple Linux packaging issue for the external libmpdec. To have the exact same behavior for the external libmpdec as for the included libmpdec, packagers must use:

    3.8 \<--> 2.4.2 3.9 \<--> 2.5.0

    ArchLinux had no problems. Debian, and by extension Ubuntu, requires 3.8 and 3.9 to be on the same system during a transitional period, as pointed out in msg372928 (which is really the most important message of this whole thread).

    The commit that pinned _decimal to libmpdec version 2.5.0 broke this use case, but there are workarounds.

    My stance is that it is important that libmpdec is pinned so distros don't use a divergent version. Since there are multiple mitigations for Debian, I don't feel particularly guilty.

    Review of the commit that pinned 2.5.0 would have led to the exact same outcome: I would have pointed that out on GitHub.

    Note that with the Debian scheme there is never a good time to update libmpdec, regardless of the release cycle.

    pitrou commented 4 years ago

    Thanks for the clarification. I agree this does not seem to be a very big deal, if slightly annoying for the packager who will have to deal with it.

    5531d0d8-2a9c-46ba-8b8b-ef76132a492c commented 4 years ago

    Thanks for taking a look, Antoine.

    IIRC, the version pinning already accommodates Debian:

    #if !defined(MPD_VERSION_HEX) || MPD_VERSION_HEX < 0x02050000
      #error "libmpdec version >= 2.5.0 required"
    #endif

    In the first libmpdec versions, I had a stricter equality check, and I *think* (but I'm not 100% sure) that it was Matthias who requested the relaxation.

    Based on that, perhaps Debian should just use 2.5.0 for both 3.8 and 3.9 in the transition period.

    I'm more comfortable with 3.8 using 2.5.0 than 3.9 using 2.4.2.

    5531d0d8-2a9c-46ba-8b8b-ef76132a492c commented 4 years ago

    As noted in the first message of this thread, the sqrt-max-prec feature (requested by Mark and Tim) was already in 3.9 long before the beta freeze.

    I'm not sure why this is not clear from the original message.

    That fix is safe for Python, but not for the standalone libmpdec.

    The standalone libmpdec had to be updated, and was updated according to the Debian-friendly way requested by Matthias himself.

    Note that a pinning issue in another area of Python has surfaced in the last 24 hours. I wonder if the reaction will be a strong as here.

    5531d0d8-2a9c-46ba-8b8b-ef76132a492c commented 4 years ago

    The standalone libmpdec had to be updated, and was updated according to the Debian-friendly way requested by Matthias himself.

    Not updated, of course the sqrt-max-prec *had never been* in the standalone libmpdec, it is new in 2.5.0.

    5531d0d8-2a9c-46ba-8b8b-ef76132a492c commented 4 years ago

    More than that, I had *promised* Matthias privately to release a new libmpdec for the sqrt-max-prec feature a couple of months ago.

    I request that further packaging issues will be dealt with primarily by Matthias and myself.