Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move getopt() to Py_GetOpt() and use it unconditionally #33417

Closed
Yhg1s opened this issue Oct 30, 2000 · 7 comments
Closed

move getopt() to Py_GetOpt() and use it unconditionally #33417

Yhg1s opened this issue Oct 30, 2000 · 7 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@Yhg1s
Copy link
Member

Yhg1s commented Oct 30, 2000

BPO 402170
Nosy @tim-one, @freddrake, @Yhg1s
Files
  • None: None
  • 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 = 'https://github.com/Yhg1s'
    closed_at = <Date 2000-11-08.22:53:43.000>
    created_at = <Date 2000-10-30.17:43:03.000>
    labels = ['interpreter-core']
    title = 'move getopt() to Py_GetOpt() and use it unconditionally'
    updated_at = <Date 2000-11-08.22:53:43.000>
    user = 'https://github.com/Yhg1s'

    bugs.python.org fields:

    activity = <Date 2000-11-08.22:53:43.000>
    actor = 'fdrake'
    assignee = 'twouters'
    closed = True
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2000-10-30.17:43:03.000>
    creator = 'twouters'
    dependencies = []
    files = ['2904']
    hgrepos = []
    issue_num = 402170
    keywords = ['patch']
    message_count = 6.0
    messages = ['34725', '34726', '34727', '34728', '34729', '34730']
    nosy_count = 4.0
    nosy_names = ['tim.peters', 'fdrake', 'twouters', 'moshez']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue402170'
    versions = []

    @Yhg1s
    Copy link
    Member Author

    Yhg1s commented Oct 30, 2000

    No description provided.

    @Yhg1s Yhg1s closed this as completed Oct 30, 2000
    @Yhg1s Yhg1s self-assigned this Oct 30, 2000
    @Yhg1s Yhg1s added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Oct 30, 2000
    @Yhg1s Yhg1s closed this as completed Oct 30, 2000
    @Yhg1s Yhg1s self-assigned this Oct 30, 2000
    @Yhg1s Yhg1s added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Oct 30, 2000
    @Yhg1s
    Copy link
    Member Author

    Yhg1s commented Oct 30, 2000

    This patch attempts to do what Tim suggested in the python-dev thread about getopt()'s prototype and the difficulties of it. the 'getopt' implementation as provided in Python/getopt.c is renamed to Py_GetOpt(), the exported variables 'opterr', 'optind' and 'optarg' are prefixed with Py_, and all use in the Python sourcetree is adjusted.

    The patch is missing the 'pygetopt.h' include file, though :P I'll resubmit a proper patch later.

    There are a couple of issues still open: the name of the getopt.c file, its use of 'fprintf(stderr, ... )', its license, documentation (which this patch lacks) and whether this Py_GetOpt should be an officially exported API at all.

    @Yhg1s
    Copy link
    Member Author

    Yhg1s commented Oct 30, 2000

    New patch, includes pygetopt.h by hack. (not sure if it patches cleanly, but it's not that exciting a file anyway :) Assigned to.... (spin wheel... Guido. no. spin wheel... Barry. no. spin wheel... Moshe. dang. spin wheel... *nudge*. Ah, finally,) Tim.

    @moshez
    Copy link
    Mannequin

    moshez mannequin commented Nov 1, 2000

    Well, as the one who almost got the assignment <wink>, I'm +1 on it. Using native getopt is more trouble then it's worth, especially considering the fact that we've had an implementation of our own for so long. So here's for reinventing the wheel! <wink>

    @tim-one
    Copy link
    Member

    tim-one commented Nov 2, 2000

    Accepted and assigned back to Thomas.

    Guido approved of this "in theory" before, so go for it! I would like to see the function renamed to _PyOS_GetOpt(), because we always stick "OS" in the name of an OS substitute function, and the leading underscore keeps it out of the public API (thus answering one of your open issues: if people clamor for a public getopt replacement, we can add that later; but if we make it public from the start, it can never go away).

    About the license, we can't change it, but it certainly allows us to modify the code and distribute your changes. Under copyright law, I don't believe the changes are substantial enough that we could legitimately claim a new copyright for the new version. So the whole license thing seems a non-issue to me.

    Documentation? If it's in the private API, it doesn't need any <wink>.

    fprintf(stderr, ...)? Sure. Python barely exists by the time this code is called, and there's really nothing better to do (note that Py_Main calls fprintf(stderr, ...) itself later in a couple of other startup error cases).

    @freddrake
    Copy link
    Member

    Already checked in, so I'll close this for Thomas.

    Thanks, Thomas!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    @vstinner
    Copy link
    Member

    Code changed by commit 2cffc7d:

    commit 2cffc7d4202fc1197280a05d998075551b459283
    Author: Thomas Wouters <thomas@python.org>
    Date:   Fri Nov 3 08:18:37 2000 +0000
    
        Move our own getopt() implementation to _PyOS_GetOpt(), and use it
        regardless of whether the system getopt() does what we want. This avoids the
        hassle with prototypes and externs, and the check to see if the system
        getopt() does what we want. Prefix optind, optarg and opterr with _PyOS_ to
        avoid name clashes. Add new include file to define the right symbols. Fix
        Demo/pyserv/pyserv.c to include getopt.h itself, instead of relying on
        Python to provide it.
    

    Include/pygetopt.h was moved to Include/internal/pygetopt.h by commit e425bd7 (then renamed to Include/internal/pycore_getopt.h).

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants