python / cpython

The Python programming language
https://www.python.org
Other
63.17k stars 30.25k forks source link

"make altinstall" installs many files with incorrect shebangs #54527

Open 161dbd22-c858-4db5-b183-fdd94c13e4d1 opened 13 years ago

161dbd22-c858-4db5-b183-fdd94c13e4d1 commented 13 years ago
BPO 10318
Nosy @birkenfeld, @jcea, @ncoghlan, @abalkin, @ericvsmith, @benjaminp, @ned-deily, @ezio-melotti, @merwok, @bitdancer
Files
  • env-python3.diff
  • tools-scripts.diff
  • pep8-shebangs.diff
  • no-shebangs-for-stdlib.diff
  • 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: ```python assignee = None closed_at = None created_at = labels = ['build', 'docs'] title = '"make altinstall" installs many files with incorrect shebangs' updated_at = user = 'https://bugs.python.org/allan' ``` bugs.python.org fields: ```python activity = actor = 'orsenthil' assignee = 'none' closed = False closed_date = None closer = None components = ['Build', 'Documentation'] creation = creator = 'allan' dependencies = [] files = ['19850', '22584', '23755', '23763'] hgrepos = [] issue_num = 10318 keywords = ['patch'] message_count = 26.0 messages = ['120468', '120485', '120489', '120493', '120494', '120495', '120509', '120546', '120881', '122617', '122618', '122619', '134543', '139892', '141348', '141358', '147945', '147989', '148003', '148018', '148117', '148159', '148192', '148210', '148296', '148485'] nosy_count = 14.0 nosy_names = ['georg.brandl', 'jcea', 'ncoghlan', 'belopolsky', 'eric.smith', 'benjamin.peterson', 'ned.deily', 'ezio.melotti', 'eric.araujo', 'Arfrever', 'r.david.murray', 'docs@python', 'allan', 'python-dev'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = None url = 'https://bugs.python.org/issue10318' versions = ['Python 3.3'] ```

    161dbd22-c858-4db5-b183-fdd94c13e4d1 commented 13 years ago

    The following files are incorrectly installed with a "#!/usr/bin/env python" shebang when using "make altinstall":

    usr/lib/python2.7/base64.py usr/lib/python2.7/bsddb/dbshelve.py usr/lib/python2.7/bsddb/test/test_dbtables.py usr/lib/python2.7/cgi.py usr/lib/python2.7/cgi.py usr/lib/python2.7/Cookie.py usr/lib/python2.7/cProfile.py usr/lib/python2.7/difflib.py usr/lib/python2.7/distutils/tests/test_build_scripts.py usr/lib/python2.7/distutils/tests/test_install_scripts.py usr/lib/python2.7/distutils/unixccompiler.py usr/lib/python2.7/encodings/rot_13.py usr/lib/python2.7/idlelib/PyShell.py usr/lib/python2.7/keyword.py usr/lib/python2.7/lib2to3/pgen2/token.py usr/lib/python2.7/lib2to3/tests/data/different_encoding.py usr/lib/python2.7/lib2to3/tests/pytree_idempotency.py usr/lib/python2.7/mailbox.py usr/lib/python2.7/mimify.py usr/lib/python2.7/pdb.py usr/lib/python2.7/platform.py usr/lib/python2.7/profile.py usr/lib/python2.7/pydoc.py usr/lib/python2.7/quopri.py usr/lib/python2.7/smtpd.py usr/lib/python2.7/smtplib.py usr/lib/python2.7/symbol.py usr/lib/python2.7/tabnanny.py usr/lib/python2.7/tarfile.py usr/lib/python2.7/test/curses_tests.py usr/lib/python2.7/test/pystone.py usr/lib/python2.7/test/regrtest.py usr/lib/python2.7/test/re_tests.py usr/lib/python2.7/test/test_al.py usr/lib/python2.7/test/test_anydbm.py usr/lib/python2.7/test/test_array.py usr/lib/python2.7/test/test_binhex.py usr/lib/python2.7/test/test_bsddb.py usr/lib/python2.7/test/test_bz2.py usr/lib/python2.7/test/test_cd.py usr/lib/python2.7/test/test_cl.py usr/lib/python2.7/test/test_cmd.py usr/lib/python2.7/test/test_codecencodings_cn.py usr/lib/python2.7/test/test_codecencodings_hk.py usr/lib/python2.7/test/test_codecencodings_jp.py usr/lib/python2.7/test/test_codecencodings_kr.py usr/lib/python2.7/test/test_codecencodings_tw.py usr/lib/python2.7/test/test_codecmaps_cn.py usr/lib/python2.7/test/test_codecmaps_hk.py usr/lib/python2.7/test/test_codecmaps_jp.py usr/lib/python2.7/test/test_codecmaps_kr.py usr/lib/python2.7/test/test_codecmaps_tw.py usr/lib/python2.7/test/test_dl.py usr/lib/python2.7/test/test_dumbdbm.py usr/lib/python2.7/test/test_eof.py usr/lib/python2.7/test/test_errno.py usr/lib/python2.7/test/test_future.py usr/lib/python2.7/test/test_gl.py usr/lib/python2.7/test/test_gzip.py usr/lib/python2.7/test/test_imageop.py usr/lib/python2.7/test/test_imgfile.py usr/lib/python2.7/test/test_logging.py usr/lib/python2.7/test/test_marshal.py usr/lib/python2.7/test/test_multibytecodec.py usr/lib/python2.7/test/test_multibytecodec_support.py usr/lib/python2.7/test/test_multiprocessing.py usr/lib/python2.7/test/test_popen2.py usr/lib/python2.7/test/test_popen.py usr/lib/python2.7/test/test_random.py usr/lib/python2.7/test/test_sets.py usr/lib/python2.7/test/test_smtpnet.py usr/lib/python2.7/test/test_socket.py usr/lib/python2.7/test/test_tcl.py usr/lib/python2.7/test/test_urllib2_localnet.py usr/lib/python2.7/test/test_urllib2net.py usr/lib/python2.7/test/test_urllibnet.py usr/lib/python2.7/test/test_urlparse.py usr/lib/python2.7/test/test_userstring.py usr/lib/python2.7/test/test_whichdb.py usr/lib/python2.7/test/test_with.py usr/lib/python2.7/timeit.py usr/lib/python2.7/token.py usr/lib/python2.7/trace.py usr/lib/python2.7/UserString.py usr/lib/python2.7/uu.py usr/lib/python2.7/webbrowser.py usr/lib/python2.7/whichdb.py

    These should point to /usr/bin/python2.7 instead.

    ericvsmith commented 13 years ago

    Is there any reason for the test files to have a shebang line at all? I think those should be removed, which would cut the problem in half.

    Also, given "-m", I'm not sure any of the files in the stdlib should have a shebang line. Is there really an expectation that these should be run directly? Is anyone really running UserString.py as a script in order to run its tests?

    bitdancer commented 13 years ago

    Benjamin did some cleanup in this area in at least py3k, so he might have some thoughts, making him nosy.

    ncoghlan commented 13 years ago

    Removing shebang lines from svn completely and only *adding* them during installation steps as appropriate may be an interesting approach. (I noted that my grep of my local build found only correct references to python3.2 in the built scripts directory)

    I'll add the list of Py3k files that are unexpectedly referencing something other than "/usr/bin/env python3" in SVN as well (note that this is a straight grep, without checking to see if any of them are *meant* to be referring to Python 2.x):

    Doc/distutils/setupscript.rst does not use python3 in the example Doc/faq/library.rst (multiple instances) Doc/howto/unicode.rst Doc/howto/webservers.rst Doc/library/cgi.rst Doc/library/logging.rst Doc/library/urllib.request.rst Doc/using/unix.rst

    Lib/test/test_logging.py: #! /usr/bin/env python

    Lib/cgi.py: #! /usr/bin/env python

    Mac/BuildScript/build-installer.py: #! /usr/bin/env python Mac/Tools/fixapplepython23.py: #! /usr/bin/env python Mac/Tools/bundlebuilder.py: #! /usr/bin/env python

    Tools/gdb/libpython.py: #! /usr/bin/env python Tools/pybench/clockres.py: #!/usr/bin/env python Tools/pybench/pybench.py: #!/usr/local/bin/python -O Tools/pybench/Setup.py: #!python Tools/pybench/systimes.py: #!/usr/bin/env python Tools/pynche/pynche: #! /usr/bin/env python Tools/pynche/pynche.pyw: #! /usr/bin/env python Tools/scripts/2to3.py: #! /usr/bin/env python Tools/scripts/gprof2html.py: #! /usr/bin/env python32.3 Tools/scripts/reindent-rst.py: #!/usr/bin/env python Tools/world/world: #! /usr/bin/env python

    The spec file in Misc/RPM also has multiple references to fixing shebang lines, but I don't know anything about spec files, so I didn't even try to check if it was doing the right thing.

    ncoghlan commented 13 years ago

    For the record, my list is from an svn checkout of r86191

    ncoghlan commented 13 years ago

    A few more deeper in the py3k source tree:

    Doc/tools/docutils/_string_template_compat.py Doc/tools/docutils/readers/python/pynodes.py Doc/tools/sphinx/pycode/pgen2/token.py Lib/lib2to3/tests/data/different_encoding.py

    Adding Georg, since this affects the docs as well as the source code.

    birkenfeld commented 13 years ago

    You can ignore those under Doc/tools; they are neither part of the distribution and nor installed.

    ned-deily commented 13 years ago

    I agree with Eric's comment about why have shebang lines at all for files in the standard library. There isn't any use case or recommendation for ever putting /path/to/lib/pythonx.x or its subdirectories directly on a shell search path is there?

    WRT the three files found in the Mac directory, I think all of these should be left alone for right now. Specifically:

    Mac/BuildScript/build-installer.py is the script used to build OS X installer images; at the moment, it depends on a system Python 2 as a build tool, primarily because of Sphinx, and there has been an effort to keep the Python 2 and Python 3 versions of the script in sync. Eventually that will need to be changed. The shebang line could simply be removed.

    Mac/Tools/fixapplepython23.py: this one needs to be looked at a bit more as it runs during the installation process but only on OS X 10.3, a minor and dwindling niche of the user base. I think that it actually depends on the Apple-installed system Python at run time. I'll follow up on it.

    Mac/Tools/bundlebuilder.py: #! /usr/bin/env python AFAIK, bundlebuilder is neither used during the build process of Python 3 nor is it installed. It is used in the Python 2 build process.

    abalkin commented 13 years ago

    I would like to add my +1 to Eric's msg120485 above. What I really find puzzling is why some scripts in Tools/ have hashbangs, but don't have execute permission.

    Tools/scripts/cleanfuture.py Tools/scripts/combinerefs.py Tools/scripts/db2pickle.py Tools/scripts/find_recursionlimit.py Tools/scripts/md5sum.py Tools/scripts/pickle2db.py Tools/scripts/pysource.py Tools/scripts/svneol.py

    I suggest a simple rule: no execute bit set in permissions - no hashbang line.

    And I don't think stdlib modules should have execute bit:

    -rwxr-xr-x Lib/base64.py -rwxr-xr-x Lib/cProfile.py -rwxr-xr-x Lib/cgi.py -rwxr-xr-x Lib/keyword.py -rwxr-xr-x Lib/pdb.py -rwxr-xr-x Lib/platform.py -rwxr-xr-x Lib/profile.py -rwxr-xr-x Lib/pydoc.py -rwxr-xr-x Lib/quopri.py -rwxr-xr-x Lib/smtpd.py -rwxr-xr-x Lib/smtplib.py -rwxr-xr-x Lib/symbol.py -rwxr-xr-x Lib/tabnanny.py -rwxr-xr-x Lib/token.py -rwxr-xr-x Lib/uu.py

    -rwxr-xr-x Lib/test/pystone.py -rwxr-xr-x Lib/test/re_tests.py -rwxr-xr-x Lib/test/regrtest.py -rwxr-xr-x Lib/test/test_array.py -rwxr-xr-x Lib/test/test_binhex.py -rwxr-xr-x Lib/test/test_dbm_gnu.py -rwxr-xr-x Lib/test/test_dbm_ndbm.py -rwxr-xr-x Lib/test/test_errno.py -rwxr-xr-x Lib/test/test_userstring.py

    merwok commented 13 years ago

    Attached patch fixes shebangs to use python3 in py3k.

    There are a lot of examples that use a bare “python”; changing all of those would cause merging pains.

    abalkin commented 13 years ago

    Not strictly related to this issue, but do we want to recommend redundant encoding cookie in the docs?

    merwok commented 13 years ago

    Good call. +1 on removing them.

    merwok commented 13 years ago

    There are a lot of examples that use a bare “python”; changing all of those would cause merging pains.

    I’ve changed my mind. Given the python/python2/python3 drama with distributions, I now think that we should use “python3” in the 3.x docs. Merging is not hard.

    merwok commented 13 years ago

    This 3.2 patch updates UNIX rights and shebangs in Tools/scripts.

    I also edited mailerdaemon, which used a string exception.

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 13 years ago

    New changeset 65d5ecefd50d by Éric Araujo in branch '3.2': Fix missing or wrong shebangs and missing executable bits for scripts (bpo-10318) http://hg.python.org/cpython/rev/65d5ecefd50d

    New changeset 5467abaaf5eb by Éric Araujo in branch 'default': Merge from 3.2 (bpo-10318, bpo-12255, bpo-12043, bpo-12417 and other fixes) http://hg.python.org/cpython/rev/5467abaaf5eb

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 13 years ago

    New changeset 7fb0abf928e0 by Éric Araujo in branch '2.7': Fix missing or shebangs and executable bits for scripts (bpo-10318) http://hg.python.org/cpython/rev/7fb0abf928e0

    ezio-melotti commented 12 years ago

    Éric, what's the status of this?

    merwok commented 12 years ago

    The patches I’ve discussed and committed were actually peripheral. The original bug reported here is that shebangs shouldn’t use “/usr/bin/env python” with an altinstall installation, as in that case you’re not creating a python (or python3) binary but a pythonx.y, so the shebangs should refer to that exact x.y versions.

    The few scripts from Tools/scripts (idle, pydoc, 2to3, python-config) are installed with an x.y suffix and hard-code the Python version and location at build time, by using distutils. (That’s why Nick reports the only matches for “3.2” found by grep are in the build/scripts directory.)

    The stdlib modules do use “/usr/bin/env python[3]”. I see various ways to handle that: a) Reject the bug as works for me: these are stdlib modules, not scripts, they can be imported or executed with -m, they’re not symlinked (by us) from anywhere so the bug, while technically valid, has no real effect.

    b) Further complicate the build/install machinery to update shebangs in altinstall mode.

    c) Remove useless shebangs and execute bit in the stdlib.

    My preference is to do c) for 3.3 and nothing for the stable versions.

    (About wrongly using python in Python 3 docs: I’ll open another bug for that).

    jcea commented 12 years ago

    +1 to "c".

    ncoghlan commented 12 years ago

    +1 to 'c', but it should come with an update to PEP-8 to say "don't do that".

    merwok commented 12 years ago

    Patch to PEP-8 attached.

    ncoghlan commented 12 years ago

    Hmm, my initial reaction is that that specific wording is stronger than I had in mind - there's nothing really wrong with having a shebang line and execute bit set on a top level module and symlinking it from /usr/bin. The problem is that we're doing those things for modules that we *don't* install as binaries, and that's silly (particularly when the shebang lines are wrong on altinstall). However, that concern can specifically by addressed by moving the new section down to be a subheading in the "Rules that apply only to the standard library" section.

    I'd also mention the justification that this is due to such shebang lines creating a maintenance problem for handling parallel installations of different Python versions.

    Also, with PEP-397 a viable candidate, we may as well make the wording OS neutral. Something like:

    \======================= Executable Files and Shebang Lines

    Standard library modules (including test modules) should not be flagged as executable files nor contain a shebang line.

    Including shebang lines in standard library modules is an issue, as they create a maintenance problem when multiple versions of Python are installed in parallel. The easiest way to avoid accidentally invoking the wrong version of Python is to simply not include such lines at all.

    If a module provides command-line functionality, it can be used with python -m module or via a small script (in a different file) that imports the module and calls one of its functions (e.g. the pydoc script imports the pydoc module and calls pydoc.cli()).

    Any installed scripts should use a shebang line of the form::

        #!/usr/bin/env pythonX.Y

    where X.Y is the specific Python version being installed. Updating these lines to the correct Python version should be automated, either as part of the installation process or as part of the version update process (see PEP-101). \=======================

    The PEP-8 update should be run by the wider audience of python-dev before it gets committed, though.

    merwok commented 12 years ago

    Hmm, my initial reaction is that that specific wording is stronger than I had in mind - there's nothing really wrong with having a shebang line and execute bit set on a top level module and symlinking it from /usr/bin. Okay. (On that topic, http://lists.debian.org/debian-python/2011/11/msg00058.html may interest you.)

    The problem is that we're doing those things for modules that we *don't* install as binaries, and that's silly Yep. Attached patch removes them for 3.3.

    I'd also mention the justification that this is due to such shebang lines creating a maintenance problem for handling parallel installations of different Python versions. I’d rather just say that it’s unneeded. With all due respect to the original poster, I don’t think this really caused problems.

    I will move my addition to the stdlib-only section. I’m not sure about OS-neutrality; the executable bit is Unix-specific and I’d rather use that exact term than a vague “flagged as executable”. I’ll make the part about shebangs neutral however, it won’t be hard.

    About this part of your proposal:

    Any installed scripts should use a shebang line of the form::

    !/usr/bin/env pythonX.Y

    Due to the use of distutils’ build_scripts that hard-codes one path, I’m not sure it’s time yet to make that recommendation. For packaging, I intend to launch a discussion about that behavior, which is often unhelpful.

    I really appreciate your taking time to review, and will submit the next revision of the patch here before going to python-dev.

    ncoghlan commented 12 years ago

    Hmm, interesting mailing list post - I hadn't thought about how the auto-initialisation of sys.path[0] aligns with the Windows vs Unix difference in PATH handling (i.e. whether or not the current directory is considered to be on PATH), with Python coming down in favour of the usually-more-convenient-but-less-secure Windows approach.

    We have -S to disable all site processing, -s to disable user site packages and -E to ignore PYTHONPATH and PYTHONHOME - perhaps there should be another flag to skip the auto-initialisation of sys.path[0]. I may include something along those lines in PEP-395.

    ncoghlan commented 12 years ago

    I created bpo-13475 to discuss the idea of a command line option to override sys.path[0] initialisation.

    merwok commented 12 years ago

    Can I removed the shebangs in the 3.3 stdlib or do I need to go through with the PEP-8 patch on python-dev first?