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

[Partially fixed] Wrong permissions for Klipper virtual env #134

Open
Frix-x opened this issue Jun 18, 2024 · 9 comments
Open

[Partially fixed] Wrong permissions for Klipper virtual env #134

Frix-x opened this issue Jun 18, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@Frix-x
Copy link

Frix-x commented Jun 18, 2024

What happened

Hi, I'm Shake&Tune dev and since I switched the tool to a Klipper plugin (v4), several people have told me that there is a problem installing it when also using RatOS:

ERROR: Could not install packages due to an OSError: [Errno 13] Permission denied: '/home/pi/klippy-env/lib/python3.9/site-packages/mpl_toolkits/mplot3d/proj3d.py'
Check the permissions.

After some troubleshooting, it's due to the Klipper venv not having the correct permissions: some of the packages are installed as root, and thus it's not possible to install/update any of them from a user script (like the Shake&Tune install script). I'm pretty sure this is unwanted and probably a bug.

What did you expect to happen

Install script of Shake&Tune activate the Klipper venv and install the required packages correctly without permissions errors

How to reproduce

Just try to run wget -O - https://raw.githubusercontent.com/Frix-x/klippain-shaketune/main/install.sh | bash on a RatOS installed printer

Additional information

Here are the relevant issues on my side:
Frix-x/klippain-shaketune#116
Frix-x/klippain-shaketune#127

The workaround to make it work currently is to issue manually:

sudo chown -R pi:pi /home/pi/klippy-env/lib/python3.9/site-packages/matplotlib
sudo chown -R pi:pi /home/pi/klippy-env/lib/python3.9/site-packages/mpl_toolkits

And then Shake&Tune can be installed correctly

@Frix-x Frix-x added the bug Something isn't working label Jun 18, 2024
@miklschmidt
Copy link
Member

miklschmidt commented Jun 21, 2024

Hey Frix!

First of all, love Shake&Tune, you've done an awesome job with that, i will probably look into porting some of your work to the Realtime analysis tool once we get all the small stuff out of the way in 2.1. If you want to be involved in any way, don't hesitate to reach out!

I actually considered preinstalling shake&tune but as much of our attention are currently going towards hybrid, we'd need to make some changes/additions, so it got put on the backburner. I might refer people to your docs though, they're hilarious and great at the same time :D

Anyway! About the issue, not exactly sure when that happens as klipper is installed as pi, actually the CustomPiOS module comes straight from MainsailOS, i don't think i made any modifications. Might be a third party add-on. Currently looking into what's going on before just doing the "dumb thing" and chown'ing everything at the end.

Will get back to you shortly, thanks for reaching out!

@miklschmidt
Copy link
Member

miklschmidt commented Jun 21, 2024

@Frix-x Found the issue, will be fixed in 2.1-RC2.

I have a small request, when linking klippy extensions check if ratos exists and register the extension with the ratos configurator instead of linking it manually. This way the configurator will manage the extension, it'll link it, restore it if the user runs hard recovery on klipper or other moonraker related shenanigans. It'll also take care of adding it to ignored files etc:

if "which ratos" &> /dev/null; then
	ratos extensions register klipper "shaketune" "${PATH_TO_FILE}"
	ratos extensions symlink klipper
fi

@miklschmidt
Copy link
Member

I will issue an update to 2.1 and 2.0 that recursively chowns the klippy-env. Will close this when done.

@miklschmidt
Copy link
Member

I will issue an update to 2.1 and 2.0 that recursively chowns the klippy-env. Will close this when done.

I would like to ask why you wouldn't do this on 2.1 for ratos.cfg to make it read only after the configurator makes it changes?

@smwoodward i already explained to you why this doesn't do what you think it does. RatOS-configurator generates RatOS.cfg, and it runs as BASE_USER, same as moonraker (which is what you're interacting with when editing configs in mainsail). Making it not writable by BASE_USER, would break it.

Please keep your questions related to #135 in #135 .

@smwoodward

This comment was marked as abuse.

@Frix-x
Copy link
Author

Frix-x commented Jun 22, 2024

Hey!

First of all, thanks for the kind words about Shake&Tune! I'm glad to hear that you’re considering porting some of the work to your realtime analysis tool. I'd love to be involved, so feel free to reach out whenever you need.

As for future developments on my side, I am planning to add support for hybrid printers in Shake&Tune as well. While I'm still figuring out the specifics, some elements should be available soon. I'd be happy to collaborate on this if you have any input or requirements from your side as I don't have any hybrid printer on hand at the moment...

Regarding the initial issue: thank you for addressing the problem so promptly! And I can certainly update Shake&Tune to register with the Ratos configurator if it exists (or if you want to do it by yourself by opening a PR, I'll be happy to merge it). However, I have a question: does this process support registering an entire folder? Shake&Tune consists of multiple files, so I want to ensure that the registration and management through Ratos will handle all of them appropriately.

@miklschmidt
Copy link
Member

@Frix-x Awesome, will hit you up on discord when i get back to working on it :)

As for future developments on my side, I am planning to add support for hybrid printers in Shake&Tune as well. While I'm still figuring out the specifics, some elements should be available soon. I'd be happy to collaborate on this if you have any input or requirements from your side as I don't have any hybrid printer on hand at the moment...

For sure! We're experimenting with a few things for measuring the Y's individually (or at least try to isolate them as much as possible) for doing belt tension graphs. I'll be happy to give it a go, would like to replace all of our graph scripts with shake tune in the future. I'm sure @HelgeKeck would want to play with it too.

does this process support registering an entire folder?

Currently it only handles the extras (specifically to have recovery options in case the user ran "hard recovery" on klipper through moonraker). There are plans to work out a more full-fledged plugin system in the future, but that is quite a long way out still. As far as i know, most of the scripts run standalone, right? So as long as Shake&Tune is registered with the moonraker update manager, those should be safe.

I can submit a PR if you want, just have to get a bunch of stuff off my list first :)

@HelgeKeck
Copy link

I'm sure @HelgeKeck would want to play with it too.

And how he wants

@HelgeKeck
Copy link

There is a lot that could be done with Shake&Tune and single toolhead and IDEX hybrid printers, cant wait to try it out

@miklschmidt miklschmidt changed the title Wrong permissions for Klipper virtual env [Partially fixed] Wrong permissions for Klipper virtual env Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants