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

Launcher for Windows (py.exe) may rank Python 3.xx (in the future) after 3.x #82687

Closed
ynyyn mannequin opened this issue Oct 17, 2019 · 9 comments
Closed

Launcher for Windows (py.exe) may rank Python 3.xx (in the future) after 3.x #82687

ynyyn mannequin opened this issue Oct 17, 2019 · 9 comments
Assignees
Labels
3.10 only security fixes OS-windows type-bug An unexpected behavior, bug, or error

Comments

@ynyyn
Copy link
Mannequin

ynyyn mannequin commented Oct 17, 2019

BPO 38506
Nosy @gvanrossum, @pfmoore, @tjguk, @zware, @zooba, @ZackerySpytz, @ynyyn
PRs
  • bpo-38506: Fix the Windows launcher's mishandling of 3.10 #18307
  • 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/zooba'
    closed_at = <Date 2020-11-16.22:03:57.875>
    created_at = <Date 2019-10-17.14:31:04.585>
    labels = ['type-bug', '3.10', 'OS-windows']
    title = 'Launcher for Windows (py.exe) may rank Python 3.xx (in the future) after 3.x'
    updated_at = <Date 2020-11-16.22:03:57.874>
    user = 'https://github.com/ynyyn'

    bugs.python.org fields:

    activity = <Date 2020-11-16.22:03:57.874>
    actor = 'steve.dower'
    assignee = 'steve.dower'
    closed = True
    closed_date = <Date 2020-11-16.22:03:57.875>
    closer = 'steve.dower'
    components = ['Windows']
    creation = <Date 2019-10-17.14:31:04.585>
    creator = 'ynyyn'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38506
    keywords = ['patch']
    message_count = 9.0
    messages = ['354843', '354846', '381061', '381063', '381117', '381118', '381131', '381165', '381169']
    nosy_count = 7.0
    nosy_names = ['gvanrossum', 'paul.moore', 'tim.golden', 'zach.ware', 'steve.dower', 'ZackerySpytz', 'ynyyn']
    pr_nums = ['18307']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue38506'
    versions = ['Python 3.10']

    @ynyyn
    Copy link
    Mannequin Author

    ynyyn mannequin commented Oct 17, 2019

    I, for interest, read some of source code of Python launcher, and found it used string comparison function (wcscmp(), in function compare_pythons()) to sort Python version in descending order.

    It works well currently. But if Python 3.10 or Python 3.xx comes up in the future, Python 3.xx will be ranked after 3.x.

    I modified the Registry and made a fake version Python 3.10, to check the launcher's behaviour.

    PS > py -0p
    Installed Pythons found by C:\Windows\py.exe Launcher for Windows
     -3.8-64        D:\Program Files\Python38\python.exe *
     -3.7-64        D:\Program Files\Python37\python.exe
     -3.7-32        D:\Program Files (x86)\Python37-32\python.exe
     -3.6-32        C:\Program Files (x86)\Python36-32\python.exe
     -3.10-64       D:\Program Files\Python37\python.exe
     -2.7-64        C:\python27-x64\python.exe
    

    The result turned out that Python 3.10 was really ranked after 3.x.

    And it seems that Python 3.xx should be a valid (or supported) version according to the comment from function validate_version().

    static BOOL
    validate_version(wchar_t * p)
    {
        /*
        Version information should start with the major version,
        Optionally followed by a period and a minor version,
        Optionally followed by a minus and one of 32 or 64.
        Valid examples:
          ...
          2.7-32
          The intent is to add to the valid patterns:
          3.10
          3-32
          ...
        */
    
    I am not sure whether this is a potential defect that had been confirmed before... But I do not see some relevant comments in the source code.
    

    @ynyyn ynyyn mannequin added 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes OS-windows type-bug An unexpected behavior, bug, or error labels Oct 17, 2019
    @zooba
    Copy link
    Member

    zooba commented Oct 17, 2019

    You're right that this will need fixing. But we don't really have to fix it any earlier than 3.10 - the launcher is backwards-compatible but not necessarily forwards-compatible. Still, we can take a fix whenever.

    Probably it should just change to using CompareStringEx with (at least) the SORT_DIGITSASNUMBERS flag.

    https://docs.microsoft.com/windows/win32/api/stringapiset/nf-stringapiset-comparestringex

    @zooba zooba removed 3.7 (EOL) end of life 3.8 only security fixes labels Oct 17, 2019
    @gvanrossum
    Copy link
    Member

    I think now's the time to fix it, given that we're two alphas into 3.10 already. (I independently discovered this and filed it as bpo-42365.)

    @gvanrossum gvanrossum added 3.10 only security fixes and removed 3.9 only security fixes labels Nov 16, 2020
    @gvanrossum
    Copy link
    Member

    Hm, actually I think this needs to be backported to 3.8 and 3.9 (at least) since IIUC whichever release is installed last (or first?) overwrites "py.exe", so if "py.exe" came from e.g. 3.9, and 3.10 is present, we still want it to sort that correctly.

    @gvanrossum gvanrossum added 3.8 only security fixes 3.9 only security fixes labels Nov 16, 2020
    @zooba
    Copy link
    Member

    zooba commented Nov 16, 2020

    We can backport it, but the latest version always wins. I deliberately designed the installer (which has multiple embedded MSIs in it) to make sure this worked, so that earlier versions won't overwrite the launcher (anymore).

    Besides, this is a purely cosmetic change. PEP-514 describes how to programmatically get the list of versions (i.e. without using py.exe at all).

    @gvanrossum
    Copy link
    Member

    You may call it cosmetic, but for me it's a matter of usability.

    Nevertheless, given how you designed the installer, we can drop the backport.

    @gvanrossum gvanrossum removed 3.8 only security fixes 3.9 only security fixes labels Nov 16, 2020
    @zooba
    Copy link
    Member

    zooba commented Nov 16, 2020

    It's cosmetic in the sense that it *only* affect usability, and not correctness.

    If it had no impact at all, I'd be calling it "pointless" ;)

    @zooba
    Copy link
    Member

    zooba commented Nov 16, 2020

    New changeset f62dad1 by Zackery Spytz in branch 'master':
    bpo-38506: Fix the Windows py.exe launcher's misordering of 3.10 (GH-18307)
    f62dad1

    @zooba
    Copy link
    Member

    zooba commented Nov 16, 2020

    Thanks for the patch, Zackery!

    @zooba zooba closed this as completed Nov 16, 2020
    @zooba zooba self-assigned this Nov 16, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants