OctoPrint / OctoPrint-FirmwareUpdater

OctoPrint plugin for flashing pre-compiled firmware images to a 3D printer.
https://plugins.octoprint.org/plugins/firmwareupdater/
GNU Affero General Public License v3.0
351 stars 76 forks source link

Instruction Changes for Python 3.9 #349

Closed NeoMatrixJR closed 4 months ago

NeoMatrixJR commented 1 year ago

Changes to instructions for Python 3.9 as current Python 3 instructions are broken.

draeath commented 1 year ago

Pointing users to a third party site for code seems dangerous.

salfter commented 1 year ago

Pointing users to a third party site for code seems dangerous.

The rationale for that is given in issue #321: https://github.com/Thynix/pyheatshrink has no releases that include the changes we need, and it includes submodules that won't be pulled when a tarball is downloaded from here. If you're concerned, I included the command that generated the tarball:

(cd /tmp && docker run "-v${PWD}:/a" devsisters/tarballize https://github.com/Thynix/pyheatshrink master) && sudo chown `whoami` /tmp/master.tar.xz && mv /tmp/master.tar.xz pyheatshrink-master.tar.xz

It still needs to be hosted somewhere...from my point of view, better to use a WordPress instance I control than some janky file-hosting service that could go the way of Imgur tomorrow. If it could be put someplace more "official" (within this repo, perhaps?), that'd be better.

draeath commented 1 year ago

It's probably better to include it in the repo via git-lfs if the license allows for it.

Is it a binary artifact? (I'm assuming so)

draeath commented 1 year ago

Also, one shouldn't be afraid of git submodules, which can point to arbitrary commits - they need not be associated with a particular head.

Or perhaps host a fork?

jneilliii commented 9 months ago

Curious if this is even necessary now? I just installed marlin-binary-protocol and it installed heatshrink2 instead of heatshrink.

looks like after removing heatshrink, heatshrink2, and marlin-binary-protocol and installing just marlin-binary-protocol it fails on heatshrink, so I guess you do still need it.

salfter commented 8 months ago

I got it working by patching marlin-binary-protocol to only use heatshrink2, not heatshrink. Since heatshrink only (IIRC) works with Python 2 and nobody should be using Python 2 at this point, this shouldn't be a problem.

https://gitlab.alfter.us/salfter/marlin-binary-protocol/-/archive/v0.0.8/marlin-binary-protocol-v0.0.8.tar.gz

jneilliii commented 8 months ago

I agree, would be better to just remove the python 2 stuff and move on. It would only effect 4.66% of the userbase @benlye

image

draeath commented 8 months ago

Do the octoprint images ship python3? If not, I think the python2 stuff can just go.

Otherwise, pip requirements.txt can contain interpreter version logic. You should be able to specify that one package is only used with py3, the other py2. I would be surprised if setuptools can't do something similar.

https://stackoverflow.com/a/33451105 shows how to do it for pip, plus something I had somewhere with an exclusion for PyPy shows that there's yet more logic that can go in there.

SomeProject==5.4; python_version < '2.7'
yappi; platform_python_implementation != 'PyPy'

As for the imports, those can be done in a try/catch or in logic that checks the interpreter version, and a module can be imported with an arbitrary name (though one should take care that the stuff in the module you reference takes the same arguments etc)

jneilliii commented 8 months ago

Do the octoprint images ship python3? If not, I think the python2 stuff can just go.

Current stable versions of OctoPi are python 3 for sure.

benlye commented 8 months ago

I'm happy to drop support for Python 2.