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

curaLulzbot: Move from Python 3.8 back to 3.6 to avoid showstopper bugs #104205

Conversation

charles-dyfis-net
Copy link
Contributor

See https://gitlab.com/lulzbot3d/cura-le/cura-lulzbot/-/issues/72

Motivation for this change

cura-lulzbot does not work at all in NixSO 20.09, as cura_app.py refers to platform.linux_distribution(), which was deprecated in Python 3.6 and removed as of (or possibly before?) 3.8.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@prusnak
Copy link
Member

prusnak commented Nov 18, 2020

It should be enough to just drop the line linux_distro_name = platform.linux_distribution()[0].lower() in cura_app.py. It does not seem that the assigned variable linux_distro_name is used anywhere.

@charles-dyfis-net
Copy link
Contributor Author

It should be enough to just drop the line linux_distro_name = platform.linux_distribution()[0].lower() in cura_app.py. It does not seem that the assigned variable linux_distro_name is used anywhere.

True. However, I'm very hesitant to try to ship against a Python version upstream has (clearly) never tested.

@charles-dyfis-net
Copy link
Contributor Author

It should be enough to just drop the line linux_distro_name = platform.linux_distribution()[0].lower() in cura_app.py. It does not seem that the assigned variable linux_distro_name is used anywhere.

@prusnak -- FWIW, I've gotten a report (from @leenaars) that merely patching out that variable results in an executable that starts up successfully, but doesn't correctly operate past that point.

@charles-dyfis-net
Copy link
Contributor Author

One place where I'd appreciate feedback is on whether it's acceptable to have the change making numpy for Python 3.6 available (vs being in a gap between Python 3.4-and-older using numpy 1.16, and Python 3.8-and-newer using Numpy 1.20) inside the same commitish as the rest of the work, or whether it should be split either into its own commit, or into a completely separate PR.

@charles-dyfis-net
Copy link
Contributor Author

One place where I'd appreciate feedback is on whether it's acceptable to have the change making numpy for Python 3.6 available (vs being in a gap between Python 3.4-and-older using numpy 1.16, and Python 3.8-and-newer using Numpy 1.20) inside the same commitish as the rest of the work, or whether it should be split either into its own commit, or into a completely separate PR.

@FRidh, it looks like you're involved in numpy derivation maintenance -- are you the right person to ask on the above?

@FRidh
Copy link
Member

FRidh commented Apr 3, 2021

The 3.6 package set is not tested at all and can break at anytime. As more packages remove support for 3.6 it gets even harder to keep it working, an effort you would have to put in yourself. 3.6 is considered EOL end of this year.

@charles-dyfis-net
Copy link
Contributor Author

The 3.6 package set is not tested at all and can break at anytime. As more packages remove support for 3.6 it gets even harder to keep it working, an effort you would have to put in yourself. 3.6 is considered EOL end of this year.

Through EOY seems like a timeline on which I'm reasonably able to make that commitment. If Lulzbot hasn't managed to put out a new release of their Cura fork by the time the newest Python interpreter they support goes EOL, that would seem like an excellent time to call Cura-LE dead.

@FRidh
Copy link
Member

FRidh commented Apr 3, 2021

Sure, its up to you, just be aware we don't have any CI anymore for that package set. I'd strongly recommend working with upstream at migrating to a newer Python version.

Note also Python 3.6 will be removed for NixOS 21.11.

@charles-dyfis-net
Copy link
Contributor Author

Sure, its up to you, just be aware we don't have any CI anymore for that package set. I'd strongly recommend working with upstream at migrating to a newer Python version.

Note also Python 3.6 will be removed for NixOS 21.11.

👍; I've pinged their customer support to get an official statement on upstream maintenance plans.

(It turns out they have pushed out a minor revision, but it's only updating the frontend and not the Python libraries behind it, which are where much is the breakage is).

@charles-dyfis-net
Copy link
Contributor Author

charles-dyfis-net commented Apr 10, 2021

Upstream indicated that the failure to tag a new curaBinaryData release with updated firmware was in error, and they've retroactively created a tag at the correct position; revising the version-bump part of this PR to include the previously-missing components (which will enable support for the new 1.75mm toolhead).

@stale
Copy link

stale bot commented Oct 12, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 12, 2021
@charles-dyfis-net charles-dyfis-net mentioned this pull request Oct 23, 2021
12 tasks
@bobby285271
Copy link
Member

The package has been removed in #142660

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 23, 2021
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.

None yet

5 participants