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

Update old Python 2 code in Docs/tools/extensions/patchlevel.py #73535

Closed
DimitrisJim mannequin opened this issue Jan 23, 2017 · 14 comments
Closed

Update old Python 2 code in Docs/tools/extensions/patchlevel.py #73535

DimitrisJim mannequin opened this issue Jan 23, 2017 · 14 comments
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@DimitrisJim
Copy link
Mannequin

DimitrisJim mannequin commented Jan 23, 2017

BPO 29349
Nosy @db3l, @ned-deily, @vadmium, @DimitrisJim
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • patchlevel.patch
  • patchlevel_print.patch
  • patchlevel_with.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 = None
    closed_at = <Date 2017-01-30.18:08:11.154>
    created_at = <Date 2017-01-23.09:41:21.129>
    labels = ['type-bug', '3.7', 'docs']
    title = 'Update old Python 2 code in Docs/tools/extensions/patchlevel.py'
    updated_at = <Date 2017-03-31.16:36:27.119>
    user = 'https://github.com/DimitrisJim'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:27.119>
    actor = 'dstufft'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2017-01-30.18:08:11.154>
    closer = 'ned.deily'
    components = ['Documentation']
    creation = <Date 2017-01-23.09:41:21.129>
    creator = 'Jim Fasarakis-Hilliard'
    dependencies = []
    files = ['46387', '46405', '46406']
    hgrepos = []
    issue_num = 29349
    keywords = ['patch']
    message_count = 14.0
    messages = ['286065', '286136', '286178', '286438', '286440', '286445', '286450', '286451', '286452', '286454', '286455', '286458', '286461', '286496']
    nosy_count = 6.0
    nosy_names = ['db3l', 'ned.deily', 'docs@python', 'python-dev', 'martin.panter', 'Jim Fasarakis-Hilliard']
    pr_nums = ['552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29349'
    versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

    @DimitrisJim
    Copy link
    Mannequin Author

    DimitrisJim mannequin commented Jan 23, 2017

    There's a hidden Python 2 print call in the script that only gets reached after you move the Doc/ folder outside the main CPython directory and run make html.

    Additionally, an obsolete way of assuring a file gets closed is used (changed to use with statement).

    P.S: Tagged this with the Documentation component, maybe build is more applicable but I wasn't sure.

    @DimitrisJim DimitrisJim mannequin added the 3.7 (EOL) end of life label Jan 23, 2017
    @DimitrisJim DimitrisJim mannequin assigned docspython Jan 23, 2017
    @DimitrisJim DimitrisJim mannequin added docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error labels Jan 23, 2017
    @vadmium
    Copy link
    Member

    vadmium commented Jan 24, 2017

    Is it okay to only fix this in 3.5+? 3.4 only gets security fixes now.

    Either way, the “with” statement changes is not a bug fix and should only go into 3.7.

    @DimitrisJim
    Copy link
    Mannequin Author

    DimitrisJim mannequin commented Jan 24, 2017

    I'm breaking these to separate files to make it easier to apply. I also noticed that other files in [Doc/tools/extensions/](https://github.com/python/cpython/blob/main/Doc/tools/extensions) use old constructs so I'm not sure about the with.

    I'm guessing that either it should be changed in other files too or, since it's working fine, leave it as is.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 29, 2017

    New changeset ecdc864884a9 by Martin Panter in branch '3.5':
    Issue bpo-29349: Fix Python 2 syntax in documentation build code
    https://hg.python.org/cpython/rev/ecdc864884a9

    New changeset cdcb33f37bf3 by Martin Panter in branch '3.6':
    Issues bpo-29349: Merge Py 2 fix 3.5
    https://hg.python.org/cpython/rev/cdcb33f37bf3

    New changeset db8b917bbfdd by Martin Panter in branch 'default':
    Issues bpo-29349: Merge Py 2 fix 3.6
    https://hg.python.org/cpython/rev/db8b917bbfdd

    New changeset ca7d2af9920e by Martin Panter in branch 'default':
    Issues bpo-29349: Add NEWS for 3.7; use “with” statement
    https://hg.python.org/cpython/rev/ca7d2af9920e

    @vadmium
    Copy link
    Member

    vadmium commented Jan 29, 2017

    I think the general rule is to clean up code if you are doing something else in nearby code, but don’t go out of your way with unnecessary cleanups to arbitrary code. Otherwise it adds too much noise to the repository history, review process, risks adding bugs, etc, for little gain.

    Anyway, thanks for the patch!

    @vadmium vadmium closed this as completed Jan 29, 2017
    @ned-deily
    Copy link
    Member

    These changes have broken some buildbots, for example, the OS X dmg buildbots which are still using Python 2 for sphinx builds of the docs. They should be version agnostic as much as possible.

    http://buildbot.python.org/all/builders/bolen-dmg-3.x/builds/59

    @ned-deily ned-deily reopened this Jan 29, 2017
    @vadmium
    Copy link
    Member

    vadmium commented Jan 29, 2017

    Thanks Ned. Do you know what version of Python Sphinx uses (which runs patchlevel.py)?

    According to bpo-28039, David set up Python 2.7 so that “make touch” would work. But the log also uses a python2.5 command, and apparently Python 2.3 also installed on that buildbot.

    If we can rely on 2.6+, we just need to add “from __future__ import print_function”. Otherwise, we may need something like

    if sys.stderr is not None:
        sys.stderr.write(...)

    and also do something about that “with” statement.

    @ned-deily
    Copy link
    Member

    Dunno for sure, perhaps David can answer that. But the Sphinx docs imply that Sphinx 1.4.6 requires at least 2.6 and I don't test the installer build with anything less than 2.7.

    @vadmium
    Copy link
    Member

    vadmium commented Jan 29, 2017

    According to <https://bugs.python.org/issue17861#msg217417\>, Ned says 2.6+ is already needed to build the Python 3.5 documentation, so maybe Sphinx uses that.

    @db3l
    Copy link
    Contributor

    db3l commented Jan 29, 2017

    2.7.12 (/usr/local/bin) is used for the build slave and main external commands (such as hg and sphinx) and is in the buildslave path, but 2.5.1 is still the default in /usr/bin so can get used for processes with a restricted environment. Tiger's original 2.3 is present but unused.

    The DMG build-installer script itself runs under 2.5.1, because it specifies "python2.5" in its command to the slave. I believe that was originally added once the system 2.3 couldn't be used.

    During the DMG construction process, a particular step might, I suppose use 2.5.1 or 2.7.12 depending on whether it's internal to the build script, something using the same executable as the script, a subprocess with limited path, or an independent utility (like sphinx) referencing 2.7.

    I believe Ned's comment in bpo-17861 about 2.6+ is a reference to the sphinx step, which is covered by the fact that sphinx is using the newer 2.7.12.

    What appears to break in this specific case is some of the preparation - specifically the Makefile referencing patchlevel is running "python". That should work (2.7.12) if it inherits the overall build slave environment (which has /usr/local/bin first) but I think the install script filters that, so it's probably getting the /usr/bin/python 2.5.1 version.

    I've just changed the default /usr/bin/python to be 2.7 so external processes run by build-installer should get 2.7. I don't think that will break anything else currently done on the machine, but I'll deal with any fallout if needed. I'm rerunning build 59 now.

    As in bpo-28039 there's no immediate need to change the installer script itself, but since it's clearly the last hold out, I'll find some time to test that I've got any pre-requisites for it set up at which point the build master could be changed to remove the python2.5 reference.

    @db3l
    Copy link
    Contributor

    db3l commented Jan 29, 2017

    Whoops, I just realized that the patch still needs adjusting to be 2.x compatible, so obviously the extra build still won't work.

    But at this point it should be safe to assume 2.6+ such as for the rest of the sphinx processing.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 29, 2017

    New changeset 9880928ec962 by Martin Panter in branch '3.5':
    Issue bpo-29349: Use __future__ print_function; Sphinx may use Python 2.6+
    https://hg.python.org/cpython/rev/9880928ec962

    New changeset 1708afd284ff by Martin Panter in branch '3.6':
    Issues bpo-29349: Merge Py 2.6+ compatibility from 3.5
    https://hg.python.org/cpython/rev/1708afd284ff

    New changeset e50058ecd808 by Martin Panter in branch 'default':
    Issues bpo-29349: Merge Py 2.6+ compatibility from 3.6
    https://hg.python.org/cpython/rev/e50058ecd808

    @vadmium
    Copy link
    Member

    vadmium commented Jan 29, 2017

    I pushed the simpler 2.6-compatible option. Keeping this open to check the buildbot is happy overnight.

    @ned-deily
    Copy link
    Member

    The fix looks good and the dmg buildbots are happy again. Thanks, Martin and David.

    @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.7 (EOL) end of life docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants