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

Drop w9xpopen and all dependencies #46657

Closed
tpn opened this issue Mar 18, 2008 · 8 comments
Closed

Drop w9xpopen and all dependencies #46657

tpn opened this issue Mar 18, 2008 · 8 comments
Assignees
Labels
build The build process and cross-build OS-windows type-feature A feature request or enhancement

Comments

@tpn
Copy link
Member

tpn commented Mar 18, 2008

BPO 2405
Nosy @loewis, @amauryfa, @tiran, @tpn, @briancurtin
Superseder
  • bpo-14470: Remove using of w9xopen in subprocess module
  • Files
  • deprecate-w9xpopen.patch
  • 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/tpn'
    closed_at = <Date 2012-12-31.06:45:57.154>
    created_at = <Date 2008-03-18.20:09:28.845>
    labels = ['type-feature', 'OS-windows', 'build']
    title = 'Drop w9xpopen and all dependencies'
    updated_at = <Date 2012-12-31.06:45:57.142>
    user = 'https://github.com/tpn'

    bugs.python.org fields:

    activity = <Date 2012-12-31.06:45:57.142>
    actor = 'brian.curtin'
    assignee = 'trent'
    closed = True
    closed_date = <Date 2012-12-31.06:45:57.154>
    closer = 'brian.curtin'
    components = ['Build', 'Windows']
    creation = <Date 2008-03-18.20:09:28.845>
    creator = 'trent'
    dependencies = []
    files = ['19608']
    hgrepos = []
    issue_num = 2405
    keywords = ['patch']
    message_count = 8.0
    messages = ['63978', '64009', '64249', '121208', '128894', '130298', '130305', '178644']
    nosy_count = 7.0
    nosy_names = ['loewis', 'amaury.forgeotdarc', 'techtonik', 'christian.heimes', 'trent', 'brian.curtin', 'benrg']
    pr_nums = []
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '14470'
    type = 'enhancement'
    url = 'https://bugs.python.org/issue2405'
    versions = ['Python 3.2', 'Python 3.3']

    @tpn
    Copy link
    Member Author

    tpn commented Mar 18, 2008

    Python 2.6+ drops support for Windows 95/98, which removes the need for
    w9xpopen. Get rid of the module and all dependencies (such as in the .msi).

    @tpn tpn self-assigned this Mar 18, 2008
    @tpn tpn added build The build process and cross-build type-feature A feature request or enhancement labels Mar 18, 2008
    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Mar 18, 2008

    Tim Peters once commented that w9xpopen cannot go away as long as people
    still use alternative shells (through COMSPEC) that still have the
    original issue that command.com had.

    I don't know how relevant that still is, and whether perhaps breakage is
    acceptable if people install odd shells that are not fully compatible with
    cmd.exe.

    @tiran
    Copy link
    Member

    tiran commented Mar 21, 2008

    I'm not sure either but I like to consider the removal of w9xpopen
    wrapper for the 3.x series. The py3k project was started to remove old
    cruft and I view w9xpopen as such a cruft.

    @amauryfa
    Copy link
    Member

    It is still time to add to 3.2 a DeprecationWarning when w9xpopen is used, and remove the feature in 3.3.
    See attached patch

    @techtonik
    Copy link
    Mannequin

    techtonik mannequin commented Feb 20, 2011

    I still see w9xpopen.exe in my Python 3.2 installation, which is kind of strange to me.

    @benrg
    Copy link
    Mannequin

    benrg mannequin commented Mar 7, 2011

    w9xpopen is currently used on NT. The patch to use it on NT was checked in by bquinlan in August of 2001 (http://mail.python.org/pipermail/patches/2001-August/005719.html). He claims that it is necessary in NT, even though (a) the cited knowledge base article explicitly states that it is not necessary on NT, and (b) the knowledge base article has now been deleted from Microsoft's web site, indicating that they consider it no longer relevant (they have deleted all Win9x-specific documentation, but Win2K-specific documentation is still there).

    I just don't believe that the problem solved by w9xpopen has ever existed in any version of NT. There is no credible evidence for it. There are any number of other reasons why introducing an intermediate process might have hidden some unrelated bug or otherwise resolved the problem the Win9x->Win2K upgraders were having a decade ago. I think that the use of w9xpopen in NT is a bug, not an obsolete feature, and there's no reason it couldn't be gone in 3.2.1.

    Also, I suppose it doesn't matter any more, but the logic for deciding when to run w9xpopen should be (target executable is 16-bit), which can be determined by reading the file header. Right now the test is (shell is True and (running on win9x or the command processor is named "command.com")). Every part of this test is deficient. Python programs can spawn 16-bit processes (including the shell itself) without using shell=True. Not every win9x shell is 16-bit; 32-bit shells like cmd.exe work fine. And there are 16-bit shells not named "command.com", such as 4DOS.

    @benrg
    Copy link
    Mannequin

    benrg mannequin commented Mar 8, 2011

    It turns out that, on Windows 7 32-bit with COMSPEC pointing to command.com, platform.popen('dir').read() works with w9xpopen and fails (no output) without it.

    But the reason has nothing to do with the old Win9x problem. It's because subprocess always quotes the command line after /c, which command.com doesn't understand. But w9xpopen decodes the command line (in the runtime, before main() is called) and then reencodes it, this time quoting only arguments with spaces in them. Command.com then gets /c dir, and is happy. It would be interesting if this was the bug that led to w9xpopen being used in NT for the last ten years.

    There are layers upon layers of brokenness here. w9xpopen should not be messing with the command line in the first place; it should call GetCommandLine() and pass the result untouched to CreateProcess (after skipping its own name). It certainly should not be using the argv[] contents, which are parsed with an algorithm that doesn't match the one used by cmd.exe. The decoding-encoding process munges the command line in hard-to-understand ways. Additionally, subprocess.py doesn't quote the shell name (my usual shell is C:\Program Files\TCCLE12\TCC.EXE), and it converts an argument list to a string using list2cmdline even when shell=True, which makes little sense to me.

    I think w9xpopen should be deleted and forgotten. It was written badly and has apparently been largely ignored for 10+ years. There is probably a better solution to the problem even on Win9x, such as a worker thread in the Python process that waits on both the process and pipe handles. But also, all of the shell=True code in subprocess.py needs to be rethought from the ground up. I don't think it should exist at all; far better to provide convenient support in subprocess for setting up pipelines, and require people to explicitly invoke the shell for the few remaining legitimate use cases. That should probably be discussed elsewhere, though.

    @briancurtin
    Copy link
    Member

    This was fixed in bpo-14470.

    @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
    build The build process and cross-build OS-windows type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants