Skip to content

Derby #10: Convert 50 sites to Argument Clinic across 4 files #64378

Closed
Closed
Derby #10: Convert 50 sites to Argument Clinic across 4 files#64378
Labels
extension-modulesC modules in the Modules dirtopic-argument-clinictype-featureA feature request or enhancement

Description

@larryhastings
larryhastings
opened on Jan 8, 2014
BPO 20179
Nosy @loewis, @pitrou, @taleinat, @larryhastings, @giampaolo, @tiran, @alex, @serhiy-storchaka, @dstufft, @ZackerySpytz
PRs
  • bpo-20179: Convert the _overlapped module to the Argument Clinic #14275
    		•
    Files
  • bytes_and_bytearray.patch
  • bytes_and_bytearray_2.patch
  • ssl_clinic.patch
  • ssl_clinic_2.patch
  • irc.transcript.of._ssl.clinic.discussion.txt
    		•

    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:

    assignee = None
closed_at = <Date 2020-07-10.18:49:47.074>
created_at = <Date 2014-01-08.00:15:20.609>
labels = ['extension-modules', 'type-feature', 'expert-argument-clinic']
title = 'Derby #10: Convert 50 sites to Argument Clinic across 4 files'
updated_at = <Date 2020-07-10.18:49:47.074>
user = 'https://github.com/larryhastings'

    bugs.python.org fields:

    activity = <Date 2020-07-10.18:49:47.074>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2020-07-10.18:49:47.074>
closer = 'serhiy.storchaka'
components = ['Extension Modules', 'Argument Clinic']
creation = <Date 2014-01-08.00:15:20.609>
creator = 'larry'
dependencies = []
files = ['33599', '33601', '38842', '39072', '39283']
hgrepos = []
issue_num = 20179
keywords = ['patch', 'needs review']
message_count = 27.0
messages = ['207635', '208590', '208698', '208713', '224134', '224135', '224760', '240149', '240150', '240152', '241234', '242473', '242475', '242477', '242479', '242481', '242493', '242494', '242523', '242524', '242525', '242526', '242550', '242551', '327366', '346162', '373474']
nosy_count = 12.0
nosy_names = ['loewis', 'janssen', 'pitrou', 'taleinat', 'larry', 'giampaolo.rodola', 'christian.heimes', 'alex', 'python-dev', 'serhiy.storchaka', 'dstufft', 'ZackerySpytz']
pr_nums = ['14275']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20179'
versions = ['Python 3.5']

    Activity

    1. larryhastings commented on Jan 8, 2014

      @larryhastings
      larryhastings
      on Jan 8, 2014
      ContributorAuthor

      This issue is part of the Great Argument Clinic Conversion Derby,
      where we're trying to convert as much of Python 3.4 to use
      Argument Clinic as we can before Release Candidate 1 on January 19.

      This issue asks you to change the following bundle of files:
      Modules/overlapped.c: 21 sites
      Modules/_ssl.c: 20 sites
      Objects/bytes_methods.c: 1 sites
      Objects/bytesobject.c: 8 sites

      Talk to me (larry) if you only want to attack part of a bundle.

      For instructions on how to convert a function to work with Argument
      Clinic, read the "howto":
      http://docs.python.org/dev/howto/clinic.html

    2. larryhastings
      added
      extension-modulesC modules in the Modules dir
      type-featureA feature request or enhancement
      on Jan 8, 2014

    3. taleinat commented on Jan 20, 2014

      @taleinat
      taleinat
      on Jan 20, 2014
      Contributor

      While converting Objects/stringlib/transmogrify.h as part of bpo-20180 (Derby #11), some changes to Objects/bytesobject.c and Objects/bytearrayobject.c were required. Those changes are included in the relevant patch attached to that issue.

    4. taleinat commented on Jan 21, 2014

      @taleinat
      taleinat
      on Jan 21, 2014
      Contributor

      Attached patch for AC conversion of Objects/bytesobject.c and Objects/bytearrayobject.c.

      This is one patch because there are changes that must be done in bytes_methods.h and bytes_methods.c that affect both of bytesobject.c and bytearrayobject.c. Those changes are in this patch as well.

      All methods were converted except for two groups:

      1. the various find methods which use the common argument parsing function 'stringlib_parse_args_finds_byte'
      2. 'new' and 'init' methods

      Also note that both of these classes use some common function implementations found under 'stringlib'. However, since that code is in separate files, I'll upload the conversion patches to the relevant issue (and write an appropriate comment here).

    5. taleinat commented on Jan 21, 2014

      @taleinat
      taleinat
      on Jan 21, 2014
      Contributor

      Attached updated patch for bytes and bytearray.

      In the previous patch I missed the 'clear' and 'copy' bytearray methods. This patch is a replacement for the previous one, with the additional conversion of these two methods.

    6. python-dev commented on Jul 27, 2014

      @python-dev
      python-devmannequin
      on Jul 27, 2014
      Mannequin

      New changeset 4d3d0659b55e by Martin v. Löwis in branch 'default':
      Issue bpo-20179: Apply Argument Clinic to bytes and bytearray.
      http://hg.python.org/cpython/rev/4d3d0659b55e

    7. loewis commented on Jul 27, 2014

      @loewis
      loewismannequin
      on Jul 27, 2014
      Mannequin

      Tal: Thanks for the patch. I applied it with updates to the current Argument Clinic.

      _ssl and _overlapped are still to be done.

    8. larryhastings commented on Aug 4, 2014

      @larryhastings
      larryhastings
      on Aug 4, 2014
      ContributorAuthor

      All the Derby patches should only go into trunk at this point.

    9. serhiy-storchaka
      added
      topic-argument-clinic
      on Feb 25, 2015

    10. serhiy-storchaka commented on Apr 6, 2015

      @serhiy-storchaka
      serhiy-storchaka
      on Apr 6, 2015
      Member

      Proposed patch converts the _ssl module to Argument Clinic. Total 39 methods converted.

    11. alex commented on Apr 6, 2015

      @alex
      alex
      on Apr 6, 2015
      Member

      I'm concerned the _ssl changes will make security backports significantly more difficult.

    12. serhiy-storchaka commented on Apr 6, 2015

      @serhiy-storchaka
      serhiy-storchaka
      on Apr 6, 2015
      Member

      _ssl.enum_certificates() and _ssl.enum_crls() is not converted because their parsing code look incorrect (bpo-23875).

    13. serhiy-storchaka commented on Apr 16, 2015

      @serhiy-storchaka
      serhiy-storchaka
      on Apr 16, 2015
      Member

      The patch updated to the tip.

    14. python-dev commented on May 3, 2015

      @python-dev
      python-devmannequin
      on May 3, 2015
      Mannequin

      New changeset 1eaaf27b3956 by Serhiy Storchaka in branch 'default':
      Issue bpo-20179: Converted the _ssl module to Argument Clinic.
      https://hg.python.org/cpython/rev/1eaaf27b3956

    15. 2 remaining items

    16. pitrou commented on May 3, 2015

      @pitrou
      pitrou
      on May 3, 2015
      Member

      I have no opinion on whether this is a good thing or not. Security backports should be few and far between, so I don't think it's a big problem if they are a bit more difficult.

    17. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      ContributorAuthor

      How often are patches backported to 2.7? While I understand the sentiment, I'd like to understand the scale of the objection being raised. I suspect it's infrequent, making the objection a minor one.

    18. tiran commented on May 3, 2015

      @tiran
      tiran
      on May 3, 2015
      Member

      You are right, patches are seldomly backported to Python 2.7. Features are never backported. Well, except for one exception: the ssl module. :)

    19. dstufft commented on May 3, 2015

      @dstufft
      dstufft
      on May 3, 2015
      Member

      I think it's worthwhile to maintain the ability to easily backport patches from 3.x to 2.7, especially given the security sensitive nature of the ssl module.

    20. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      ContributorAuthor

      We discussed it in IRC a bit (and I got a little education). I can propose three remedies:

      A) back out the Clinic conversion in _ssl.c
      B) support Clinic in 2.7 just for _ssl.c
      C) do a one-time backport of the Clinic generated code for _ssl.c

      IMO these are in my reverse order of preference; I'd prefer C, then B, then A. But I'm not supporting _ssl.c, you guys are. And I want you maintainers to be happy. So I'll abide by / help you in implementing whatever you prefer.

      FWIW, option C would mean doing an otherwise-inert backport of the current _ssl.c from trunk to 2.7, so that we could also backport (by hand) Modules/clinic/_ssl.c. Then, if in the future, when you change _ssl.c you still copy that file over and tweak it. But: if you change the arguments to a function, you'd have to *hand-edit* Modules/clinic/_ssl.c to match.

    21. pitrou commented on May 3, 2015

      @pitrou
      pitrou
      on May 3, 2015
      Member

      Le 03/05/2015 23:06, Larry Hastings a écrit :

      Larry Hastings added the comment:

      We discussed it in IRC a bit (and I got a little education).

      Can we have a transcript somewhere?

    22. larryhastings commented on May 3, 2015

      @larryhastings
      larryhastings
      on May 3, 2015
      ContributorAuthor

      Attached. Glad you asked right away, it would have been a lot harder to get later!

    23. pitrou commented on May 3, 2015

      @pitrou
      pitrou
      on May 3, 2015
      Member

      Thank you!

    24. pitrou commented on May 4, 2015

      @pitrou
      pitrou
      on May 4, 2015
      Member

      I can propose three remedies:

      A) back out the Clinic conversion in _ssl.c
      B) support Clinic in 2.7 just for _ssl.c
      C) do a one-time backport of the Clinic generated code for _ssl.c

      I'd rather have A or C than B.
      By the way, this discussion seems to focus on 2.7, but the same issue happens with 3.4 (although Clinic already exists here, so we can just backport Serhiy's work).

    25. larryhastings commented on May 4, 2015

      @larryhastings
      larryhastings
      on May 4, 2015
      ContributorAuthor

      Clinic's syntax is diverging from what shipped with 3.4. So if you copied _ssl.c over, it wouldn't work with the Clinic that shipped with 3.4.

      Maybe the best thing is if Clinic in trunk supports "legacy mode", where the code it generates is compatible with previous Python versions. That's basically B but without doing something crazy like shipping Clinic with 2.7.

    26. taleinat commented on Oct 8, 2018

      @taleinat
      taleinat
      on Oct 8, 2018
      Contributor

      Can someone clarify whether Modules/overlapped.c should be converted to use AC?

    27. ZackerySpytz commented on Jun 20, 2019

      @ZackerySpytz
      ZackerySpytzmannequin
      on Jun 20, 2019
      Mannequin

      PR 14275 converts Modules/overlapped.c. Many methods now use METH_FASTCALL.

    28. serhiy-storchaka commented on Jul 10, 2020

      @serhiy-storchaka
      serhiy-storchaka
      on Jul 10, 2020
      Member

      New changeset 9650fe0 by Zackery Spytz in branch 'master':
      bpo-20179: Convert the _overlapped module to the Argument Clinic (GH-14275)
      9650fe0

    29. serhiy-storchaka
      closed this as completedon Jul 10, 2020
    30. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      extension-modulesC modules in the Modules dirtopic-argument-clinictype-featureA feature request or enhancement

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions