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

Tools: stop requiring python-is-python3 package #28055

Merged

Conversation

peterbarker
Copy link
Contributor

No description provided.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Sep 10, 2024

If you uninstall this, you find that waf can't run.

$ ./Tools/autotest/sim_vehicle.py -v ArduPlane -DG --console --enable-dds
SIM_VEHICLE: Start
SIM_VEHICLE: Killing tasks
SIM_VEHICLE: Starting up at SITL location
SIM_VEHICLE: WAF build
SIM_VEHICLE: Configure waf
SIM_VEHICLE: "/home/ryan.friedman/Dev/ardu_ws/src/ardupilot/modules/waf/waf-light" "configure" "--board" "sitl" "--debug" "--enable-dds"
/usr/bin/env: ‘python’: No such file or directory
SIM_VEHICLE: (Configure waf) exited with code 32512
SIM_VEHICLE: Killing tasks

It was the python team's recommendation to update shebangs if a script could run on python3, to say python3.
This shouldn't be a huge change, and you can run python3 on Ubuntu 18 (bionic) which is EOL. On Ubuntu 20 (focal), python3 is the default.

docker run --net host -it ubuntu:18.04 bash
#apt update
#apt install python-dev
# which python
/usr/bin/python
# python --version
Python 2.7.17
# apt install python3-dev
# python3 --version
Python 3.6.9

To me, the risk of impacting people is low because we already install python3-dev in ubuntu 18.

if [ ${RELEASE_CODENAME} == 'bionic' ] ; then
SITLFML_VERSION="2.4"
SITLCFML_VERSION="2.4"
PYTHON_V="python3"
PIP=pip3

@peterbarker peterbarker changed the title Tools: stop requiring python-is-python3 packets Tools: stop requiring python-is-python3 package Sep 10, 2024
Copy link
Contributor

@khancyr khancyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't support py2 anymore (or not actively), so yes let's go into this direction !

Copy link
Collaborator

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we do this, we can also remove the # encoding: utf-8 line, that's only needed in python2 scripts.

@peterbarker
Copy link
Contributor Author

OK, all good, full-steam ahead.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Sep 11, 2024

First action seems to go request to the waf maintainer if they would be interested in following these recommendations: https://peps.python.org/pep-0394/#for-python-script-publishers

@peterbarker
Copy link
Contributor Author

First action seems to go request to the waf maintainer if they would be interested in following these recommendations: https://peps.python.org/pep-0394/#for-python-script-publishers

What entry-point are you worried about? waf-light? I usually use ./waf so I wouldn't get the problem :-)

Have you checked to see if it has been updated upstream?

We could probably also carry a trivial patch for it if required.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Sep 12, 2024

First action seems to go request to the waf maintainer if they would be interested in following these recommendations: https://peps.python.org/pep-0394/#for-python-script-publishers

What entry-point are you worried about? waf-light? I usually use ./waf so I wouldn't get the problem :-)

I removed python-is-python3 and it errored out in the waf submodule when building, so that's why I thought to start there.

Have you checked to see if it has been updated upstream?

Yea, I checked upstream, it's still using /usr/bin/env python and there's nothing on the issue tracker about it.

We could probably also carry a trivial patch for it if required.

For sure! It should be easy to whip up. I'll give it a go unless you're bored.

@peterbarker
Copy link
Contributor Author

We could probably also carry a trivial patch for it if required.

For sure! It should be easy to whip up. I'll give it a go unless you're bored.

Please go ahead. See what happens if you PR upstream too?

@peterbarker peterbarker force-pushed the pr/stop-using-python-is-python3 branch 3 times, most recently from da575aa to 8283479 Compare October 3, 2024 00:18
@peterbarker
Copy link
Contributor Author

@Ryanf55 I've created a trivial PR against our waf fork to use python3 and referenced it in here.

I think past this we don't require python to be in your path.

I've left the environment install stuff alone, on the basis that without it you can't check out 4.5 and expect things to work without more hackery. Maybe in 4.7 we can get rid of those.

@peterbarker
Copy link
Contributor Author

Needs ArduPilot/waf#19

@tridge
Copy link
Contributor

tridge commented Oct 7, 2024

can be merged after module update and pip3 -> python3 -m pip

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Oct 8, 2024

can be merged after module update and pip3 -> python3 -m pip

I applied those changes. Just needs that fixup squashed into Tools.

@peterbarker
Copy link
Contributor Author

can be merged after module update and pip3 -> python3 -m pip

I applied those changes. Just needs that fixup squashed into Tools.

Ah, whoops. Sorry, I didn't see this 'til now, and I've force-pushed.

Please ping me if you want to force-push one of my branches - I force-push and ask questions later on my own branches!

I think this one is good to merge now and have marked it as such.

@peterbarker peterbarker merged commit b68427a into ArduPilot:master Oct 15, 2024
107 checks passed
@peterbarker peterbarker deleted the pr/stop-using-python-is-python3 branch October 15, 2024 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants