Skip to content
Snippets Groups Projects

Stop assuming that /usr/bin/python exists

    • Closed (moved) Issue created by Trac @tracbot

    This issue is relevant to the following ticket: #29913 (moved)

    In short, the problem in #29913 (moved) was that Python 3 scripts cannot depend on a hardcoded path in order to find the Python 3 executable, because the path itself is not immutable. (For instance, as seen in that ticket itself.

    Unfortunately, the same exact issue also applies to Python 2 scripts and their relevant executables.

    Trac:
    Username: alwayslivid

    To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information

    Child items 0

    • No child items are currently assigned. Use child items to break down this issue into smaller parts.

    Activity

    • Trac changed milestone to %Tor: 0.4.4.x-final

      changed milestone to %Tor: 0.4.4.x-final

    • Trac
      Trac @tracbot ·
      Author

      Trac:
      Parent Ticket: #33193 (moved)

    • Trac added 044-should BugSmashFund actualpoints::0.1 component::core tor/tor milestone::Tor: 0.4.4.x-final parent::33193 priority::medium reporter::alwayslivid resolution::fixed reviewer::teor severity::normal status::closed type::defect labels

      added 044-should BugSmashFund actualpoints::0.1 component::core tor/tor milestone::Tor: 0.4.4.x-final parent::33193 priority::medium reporter::alwayslivid resolution::fixed reviewer::teor severity::normal status::closed type::defect labels

    • Trac
      Trac @tracbot ·
      Author

      Please check my proposed change here: https://github.com/torproject/tor/pull/1712

      This change will be particularly useful for people that are using FreeBSD, have compiled Python on a Linux machine, or use non-traditional setups of Python.

      Trac:
      Username: alwayslivid

    • T
      teor @teor ·

      Looks good to me, but I think the bugfix version in the changes file should be much earlier.

      We use the earliest version that the bug was introduced in, so you want the first commit that used the path "/usr/bin/python". Here are some instructions for finding it: https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandards.md#n121

      I'd also like to check with Nick (another maintainer), to see if this change go in 0.4.3, or 0.4.4,

      Here are the tradeoffs:

      • It's a long-standing bug on BSD, so it's not urgent
      • We still haven't checked if all of our scripts work on python 3 (see #33196 (moved)), and this change could make more scripts use python 3 on more machines

      Trac:
      Version: Tor: 0.4.2.5 to N/A
      Milestone: N/A to Tor: 0.4.3.x-final
      Keywords: N/A deleted, extra-review added
      Cc: N/A to nickm
      Status: new to needs_review

    • T
      teor @teor ·

      Also, when we merge, we should cherry-pick this change, because merging from a user branch called "maint-0.4.2" will confuse developers.

      Usually, we name branches after the ticket number, or after the feature or bug they fix.

      Our maint-0.4.2 branch is for maintenance changes only, and I don't think this change qualifies. It's too risky.

    • Trac
      Trac @tracbot ·
      Author

      I probably misinterpreted something in the documentation, excuse me for that one.

      I could close the pull request and make the change again on top of the master or another desired branch.

      Trac:
      Username: alwayslivid

    • Trac
      Trac @tracbot ·
      Author

      I checked some of the scripts and changed "python" to "python2" wherever the syntax wouldn't allow using Python 3. I also imported print_function from future in some cases.

      Trac:
      Username: alwayslivid

    • T
      teor @teor ·

      Replying to alwayslivid:

      I probably misinterpreted something in the documentation, excuse me for that one.

      Maybe our documentation isn't clear?

      I could close the pull request and make the change again on top of the master or another desired branch.

      That would be great!

      Replying to alwayslivid:

      I checked some of the scripts and changed "python" to "python2" wherever the syntax wouldn't allow using Python 3.

      We'd like to transition to Python 3, since python 2 is not supported. So let's leave those scripts with "python" and fix the Python 3 errors?

      There's similar work going on in #33193 (moved), you might want to check out the latest PR there?

      I also imported print_function from __future__ in some cases.

      I think we already added all the Python 2.7 to Python 3 future statements in master in #32732 (moved).

      Trac:
      Status: needs_review to needs_revision

    • T
      teor @teor ·

      We'd like to make these changes in two different releases:

      • /usr/bin/env python change in 0.4.3, and
      • leave the other syntax fixes for 0.4.4 (in #33193 (moved)).

      The master branch is currently 0.4.3, but we will split off maint-0.4.3 soon, and then the master branch will become maint-0.4.4.

      Trac:
      Parent: N/A to #33193 (moved)

    • Nick Mathewson
      Nick Mathewson @nickm ·

      Trac:
      Keywords: extra-review deleted, extra-review 043-can added

    • Trac
      Trac @tracbot ·
      Author

      Replying to teor:

      Replying to alwayslivid:

      I could close the pull request and make the change again on top of the master or another desired branch.

      That would be great!

      Just for the sake of clarification, I ought to use the master branch for my own changes, right?

      Replying to alwayslivid:

      I checked some of the scripts and changed "python" to "python2" wherever the syntax wouldn't allow using Python 3.

      We'd like to transition to Python 3, since python 2 is not supported. So let's leave those scripts with "python" and fix the Python 3

      Transitioning from Python 2 to 3 would be pretty great, especially when you take how critical Tor and its surrounding software is into account. For the time being, however, this change would potentially break some of the scripts, considering that python can also refer to python3 and some scripts simply aren't suited for python3 right now. Maybe this explicit mention of python2 would also mobilize more people to work on that. (I'd like to do some work on that myself later on.)

      I'm not sure whether I misinterpreted something, but the changes of the aforementioned tree (#33193 (moved)) don't seem to be conflicting with my own changes in any capacity.

      Trac:
      Username: alwayslivid

    • T
      teor @teor ·

      Replying to alwayslivid:

      Replying to teor:

      Replying to alwayslivid:

      I could close the pull request and make the change again on top of the master or another desired branch.

      That would be great!

      Just for the sake of clarification, I ought to use the master branch for my own changes, right?

      Yes please!

      Replying to alwayslivid:

      I checked some of the scripts and changed "python" to "python2" wherever the syntax wouldn't allow using Python 3.

      We'd like to transition to Python 3, since python 2 is not supported. So let's leave those scripts with "python" and fix the Python 3

      Transitioning from Python 2 to 3 would be pretty great, especially when you take how critical Tor and its surrounding software is into account. For the time being, however, this change would potentially break some of the scripts, considering that python can also refer to python3 and some scripts simply aren't suited for python3 right now. Maybe this explicit mention of python2 would also mobilize more people to work on that. (I'd like to do some work on that myself later on.)

      Some OSes don't install Python 2 as python2, so we can't change to /usr/bin/env python2 in the current tor 0.4.3 alpha series.

      Maybe it would be best to leave all these changes until 0.4.4, when we have time to fix any Python 3 issues?

      I'm not sure whether I misinterpreted something, but the changes of the aforementioned tree (#33193 (moved)) don't seem to be conflicting with my own changes in any capacity.

      I wasn't sure if there were conflicts.

      But I thought if you were trying to fix issues with Python 3, you might want to re-use some of those fixes.

    • T
      teor @teor ·

      We've opened merges to 0.4.4 in our master branch.

      Let's make all these changes in master, except for scripts that are:

      • supposed to run on Python 2,
      • but are broken in maint-0.4.3.

      Trac:
      Keywords: extra-review 043-can deleted, extra-review 044-should added
      Milestone: Tor: 0.4.3.x-final to Tor: 0.4.4.x-final

    • Trac
      Trac @tracbot ·
      Author

      Will work on it.

      Trac:
      Username: alwayslivid

    • Trac
      Trac @tracbot ·
      Author

      New pull request: https://github.com/torproject/tor/pull/1743

      Trac:
      Username: alwayslivid

    • T
      teor @teor ·

      Thanks! These changes look good.

      Feel free to open other trac tickets and pull requests for Python 3 compatibility changes. Now we're in 0.4.4, we can merge Python 3 fixes.

      Trac:
      Actualpoints: N/A to 0.1
      Keywords: extra-review 044-should deleted, 044-should, BugSmashFund added
      Status: needs_revision to merge_ready
      Reviewer: N/A to teor

    • David Goulet
      David Goulet @dgoulet ·

      Merged

      Trac:
      Resolution: N/A to fixed
      Status: merge_ready to closed

    • Trac closed

      closed

    • Trac added 48m of time spent

      added 48m of time spent

    • Trac mentioned in issue #33193 (moved)

      mentioned in issue #33193 (moved)

    • Trac moved to tpo/core/tor#33192 (closed)

      moved to tpo/core/tor#33192 (closed)

    Please register or sign in to reply