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

Eventual Python3 migration #542

Closed
sej7278 opened this issue Jan 4, 2018 · 38 comments
Closed

Eventual Python3 migration #542

sej7278 opened this issue Jan 4, 2018 · 38 comments

Comments

@sej7278
Copy link
Collaborator

sej7278 commented Jan 4, 2018

/usr/bin/env python still points to 2.7 for me on Debian Sid, so its far from a problem for now, but python2.7 is EOL in 2 years.

i'm just thinking ard-reset-arduino, robotis-loader and ardmk-init should probably change their shebangs to use /usr/bin/python3 instead.

Also the arduino-mk.spec and debian/control files will need updating to use python3-serial instead of python-serial (debian) and python3-pyserial instead of pyserial (fedora).

Need to check MacOS/cygwin implications and run by @maqifrnswa and @tuna-f1sh too.

https://www.debian.org/doc/packaging-manuals/python-policy/ch-python3.html

@sej7278 sej7278 added the Debian label Jan 4, 2018
@sej7278
Copy link
Collaborator Author

sej7278 commented Jan 12, 2018

@sudar any idea what's involved with macos homebrew to switch to python3?

Arduino.mk already has python3 support, the fedora/debian packaging just needs updating, and the python "bin" files, but i assume the documentation needs updating for homebrew at least?

i figure maybe we should do this during @tuna-f1sh's samd21 PR and give it a new version at the same time?

@sudar
Copy link
Owner

sudar commented Jan 14, 2018

@sej7278

For homebrew, we don't explicitly include python as dependency.

I think with a little bit of documentation we can switch from Python 2 to Python 3.

@sej7278
Copy link
Collaborator Author

sej7278 commented Jan 14, 2018

Its more work that I thought, here is a largely untested branch diffed against master.

debian/control just needs:

-Depends: arduino-core (>= 1:1.0+dfsg-8), python, python-serial, make, ${misc:Depends}
+Depends: arduino-core (>= 1:1.0+dfsg-8), python3, python3-serial, make, ${misc:Depends}

the scripts themselves could probably just change their shebang and keep the version detection/future stuff for backwards compatibility, the instructions need a bit of testing, as would travis.

@tuna-f1sh
Copy link
Contributor

I think the biggest potential impact for this is with macOS users. It ships with an Apple compiled 2.7 but no 3. Most advanced users install homebrew Python 2 & 3 to avoid messing with the system Python as installing modules requires root - using /usr/bin/env currently ensures this version gets used rather than the system Python (some users might not have installed pyserial for the system Python). Additionally, /usr/bin/python3 will not resolve on a vanilla macOS install.

We would have to add a step of installing Python 3 to the homebrew instructions (until Apple adds it to their release). I believe homebrew creates a symbolic link to /usr/bin/python3.

@sej7278
Copy link
Collaborator Author

sej7278 commented Jan 15, 2018

well we've got over a year to look into it, but i thought we required homebrew anyway, not vanilla macos?

@sej7278 sej7278 added the brew label Jan 15, 2018
@ladislas
Copy link
Contributor

@sej7278 I'm not sure you need homebrew to use Arduino-Makefile as long as you have pyserial installed.

People can just clone the repo and use it from there after installing the Arduino app.

That being said, is requires a certain level of computer knowledge to start using the makefile and even know that you need it. I guess people at that point, using github and compiling from the command line, if they are on macOS, will all have homebrew installed. There not really any other way to dive into programming without it as most of the tools (if not all) are using it as some point.

@tuna-f1sh
Copy link
Contributor

tuna-f1sh commented Jan 16, 2018

@sej7278 homebrew doesn't install Python 3 by default so it would just need the lines adding:

brew install python3
pip3 install pyserial

edit: looking more into it, there are no symbolic links created in /usr/bin and they cannot be created anymore due to the System Integrity Protection (even using sudo). So the shebang /usr/bin/python3 does not work with macOS in it's current state.

@sej7278
Copy link
Collaborator Author

sej7278 commented Aug 25, 2019

i've rebased my python3 branch against master, is there anything more we need to add - this brew/pip3 stuff @tuna-f1sh ?

if the shebang is basically undoable on macos we'd have to replace all calls to bin files to python3 ard-reset-arduino which is awful, i guess we could check for the os first....

what about using virtualenv - is /usr/bin/env ok in macos?

@tuna-f1sh
Copy link
Contributor

Yes, I use /usr/bin/env python3 on macOS scripts now - that works.

@ladislas
Copy link
Contributor

@sej7278 @tuna-f1sh on macOS, brew install python now installs python 3 by default.

→ brew info python
python: stable 3.7.4 (bottled), HEAD
Interpreted, interactive, object-oriented programming language
https://www.python.org/
/usr/local/Cellar/python/3.7.4 (3,967 files, 61.4MB) *
  Poured from bottle on 2019-08-14 at 12:10:39
From: https://github.com/Homebrew/homebrew-core/blob/master/Formula/python.rb
==> Dependencies
Build: pkg-config ✘
Required: gdbm ✔, openssl ✔, readline ✔, sqlite ✔, xz ✔
==> Options
--HEAD
	Install HEAD version
==> Caveats
Python has been installed as
  /usr/local/bin/python3

Unversioned symlinks `python`, `python-config`, `pip` etc. pointing to
`python3`, `python3-config`, `pip3` etc., respectively, have been installed into
  /usr/local/opt/python/libexec/bin

If you need Homebrew's Python 2.7 run
  brew install python@2

You can install Python packages with
  pip3 install <package>
They will install into the site-package directory
  /usr/local/lib/python3.7/site-packages

See: https://docs.brew.sh/Homebrew-and-Python

@tuna-f1sh
Copy link
Contributor

That's good, but it doesn't change the install steps much:

brew install python
pip3 install pyserial

And I think one still has to use /usr/bin/env python3 to be cross compatible with all systems, otherwise this would need to be /usr/local/bin/python3 on macOS unless I'm mistaken.

@ladislas
Copy link
Contributor

No you're right, it just makes it easier on macOS as people using brew don't have to think about it at all.

@sej7278
Copy link
Collaborator Author

sej7278 commented Aug 26, 2019

ok, so i updated my fork to use virtualenv (seems to work ok on debian) and added the homebrew instructions to README.md, although i'm not very happy with how clear they are - i've sort of repeated the pyserial instructions and added the brew installation of python3 to the macports bit!

not sure how we get this pushed to debian as @maqifrnswa seems to have dropped off the internet at the end of 2017, hope nothing awful has happened there :-(

@sej7278
Copy link
Collaborator Author

sej7278 commented Sep 26, 2019

did some experimentation and came to the same conclusion as #616 that fedora (and debian) hates virtualenv.

any shebangs in packages (rpm/deb) have to be absolute, so /usr/bin/python3 not /usr/bin/env python3

so not sure where that leaves us with macos again (symlink /usr/local/bin/python3 to /usr/bin/python3 ?)

i made a fedora 30 rpm using hardcoded python3 shebangs here

@sej7278
Copy link
Collaborator Author

sej7278 commented Nov 12, 2019

Probably need to get this moving: https://lists.debian.org/debian-devel-announce/2019/11/msg00000.html

@maqifrnswa ?

@tuna-f1sh
Copy link
Contributor

After looking into this a few times to support both macOS and Debian, the only solution appears to be to call sh as the shebang to detect the user python3 installation, then pass this to it. 1 2

Please see: tuna-f1sh@284542c

Works in macOS and my Arch installation. Can you test with Debian? It feels dirty but if it works...

@sej7278
Copy link
Collaborator Author

sej7278 commented Nov 13, 2019

fedora and debian won't like that - they have specific policies about versioned shebangs and shelling out from one language to another is dirty, bad tuna bad! ;-)

@tuna-f1sh
Copy link
Contributor

Ha, you're right, not sure what came over me.

Looking at my macOS machine, it appears Python 3 is now distrubuted with the base install (Catalina), so there is a python3 in /usr/bin independant of the Homebrew install to /usr/local/bin.

This means we can use /usr/bin/python3 in the scripts and change the macOS install instructions to (no Homebrew steps):

sudo /usr/bin/python3 -m pip pyserial

With the caveat that users must be using macOS 10.15 or have python3 linked to /usr/bin. Seems like we have a solution.

@sej7278
Copy link
Collaborator Author

sej7278 commented Nov 14, 2019

i made a PR with @tuna-f1sh's new mac instructions if people could review/test please

#620

Looks like we need to update travis-ci too

@ladislas
Copy link
Contributor

My $0.02: I would not recommend macOS users to use the base python install to install pyserial or anything else as it forces the user to use sudo.

macOS developers tend to always have (or at least should have) homebrew installed as it keeps the system clean in case something stops working. It's the same with gems and ruby.

So for macOS instructions I would say:

  • Install Homebrew (http://brew.sh/)
  • brew install python
  • pip3 install -U pyserial

And voilà.

@tuna-f1sh
Copy link
Contributor

@ladislas I'm in complete agreement there regarding macOS and base Python but the problem we are facing is that Homebrew installs to /usr/local/bin and not /usr/bin (see your previous comment), which if we are to remove the /usr/bin/env python means the scripts will no long work using Homebrew Python.

@tuna-f1sh
Copy link
Contributor

tuna-f1sh commented Nov 14, 2019

Maybe the better solution in all this is to actually call the scripts with python3; they can still include the shebangs to /usr/bin/ for standalone use but most users will not have to worry about this.

For example, pre-append python3 here: https://github.com/sudar/Arduino-Makefile/blob/master/Arduino.mk#L860

@ladislas
Copy link
Contributor

/usr/local/bin is in the default macOS path, so #!/usr/bin/env python3 will work.

@sej7278
Copy link
Collaborator Author

sej7278 commented Nov 14, 2019

we can't use virtualenv @ladislas due to linux packaging requirements

@ladislas
Copy link
Contributor

we can't use virtualenv ladislas due to linux packaging requirements

I'm not a python expert, but I'm not using virtualenv on my machine. Is calling #!/usr/bin/env python3 using virtualenv?

@sej7278
Copy link
Collaborator Author

sej7278 commented Nov 14, 2019

@ladislas yes

@ladislas
Copy link
Contributor

Alright, my bad, I did not know. This post made it clearer to me :)

Then I think @tuna-f1sh's idea is the best: #542 (comment)

@tuna-f1sh
Copy link
Contributor

Finally got around to implementing #542 (comment)

tuna-f1sh@87d5241

@sej7278
Copy link
Collaborator Author

sej7278 commented Aug 4, 2020

Looks like we need to get a move on here, Debian Sid just tried to remove arduino-mk as its now removing python2.

created this branch which is most of #620 and tuna-f1sh/Arduino-Makefile@87d5241 plus a fix for an "is vs ==" syntax warning.

https://github.com/sej7278/Arduino-Makefile/tree/python3 and PR #639

could you look it over please @tuna-f1sh and others?

debian and fedora test packages here

works for my mega2560 on debian sid - uses python3 and doesn't get removed with python2 in debian.

@tuna-f1sh
Copy link
Contributor

@sej7278 merge into my fork and all works fine for me on macOS and Arch. Thanks for doing the grunt work.

There are some other changes relating to the SAMD support in my fork now but I should probably make a separate pull request.

Will be good when all this is rolled into a release so that other package managers can be updated too. Most of them are tracking something from 3 years ago!

@sej7278
Copy link
Collaborator Author

sej7278 commented Aug 5, 2020

@tuna-f1sh yeah, very keen on getting some of your SAM stuff merged and a couple of other PR's (examples and such) once we've done this python3 thing, then probably call it a 1.7 release or something.

be nice to fix this fecking TravisCI too!

@tuna-f1sh
Copy link
Contributor

be nice to fix this fecking TravisCI too!

#640 🤙

@sej7278
Copy link
Collaborator Author

sej7278 commented Aug 6, 2020

@tuna-f1sh any chance you could pull in #630 too - its basically a clean up of the examples, then i think maybe we'll just merge your giant PR

@tuna-f1sh
Copy link
Contributor

@sej7278 it's in!

@sej7278
Copy link
Collaborator Author

sej7278 commented Aug 6, 2020

@sudar do you fancy merging the world's largest PR here? ;-)

@sej7278
Copy link
Collaborator Author

sej7278 commented Aug 6, 2020

@tuna-f1sh could you note that it closes #616, #635 (python3) and #575 (examples) in HISTORY.md ?

Probably fixes #611 and #581 (samd) too doesn't it?

@sudar
Copy link
Owner

sudar commented Aug 6, 2020

@sej7278 Definitely!

Just give me a day or two, I will test it a bit and will merge it.

@sej7278 Once this is merged, I am thinking of bumping up the version to v2.0.0, since this is a backward incompatible change. What are your thoughts?

Also thank you @sej7278 @tuna-f1sh for all the hard work in adding support for python3.

@sej7278
Copy link
Collaborator Author

sej7278 commented Aug 7, 2020

@sudar yeah might be a better idea than 1.7.0

@sej7278 sej7278 closed this as completed Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants