Closed eberamp closed 1 year ago
This is awesome, thanks for making this. It almost all worked, I did have an issue though, that being that I couldn't get it to write the output files unless I just chmodded the directory to allow everyone to read and write, which isn't super ideal. \ Any idea how to fix that?
I also looked into the thing with profiles. Naturally docker can't see the ones you have natively, so you'd need to add one yourself with pcleaner profile new <some name>
. There are no tools to edit them inside the container, so I added vim and nano as options to be able to edit them at all. Let me know if there is a better way.
I did notice that pcleaner would assume that there will be xdg-open present to open the new profile after creating it. Currently it just fails (you can ignore it, the profile is created either way), but I included a patch to check if xdg-open exists first.
PS. docker complained that the image build system was deprecated and will be removed soon, but substituting it with the new buildx worked just fine. Does it make sense to include deprecated instructions in the readme?
I couldn't get it to write the output files unless I just chmodded the directory to allow everyone to read and write
What OS are you using? Are you using WSL(windows) by any chance?
Naturally docker can't see the ones you have natively
This should be possible attaching the volume though not sure exactly where they're stored haven't taken a good look at the code, the problem I encountered was when creating a new profile, I think it was a dependency issue will try to look exactly what was it this week.
There are no tools to edit them inside the container, so I added vim and nano as options to be able to edit them at all
Yeah the base image does not include those tools, I don't think they would be necessary, also you can edit them outside the container and changes should be reflected in the file since you've got the directory attached as volume, but don't use vim to edit outside because it actually doesn't write out to the file directly
docker complained that the image build system was deprecated and will be removed soon
You mean the command build image
? What version of Docker are you using, I'm currently using 20.10.23
What OS are you using? Are you using WSL(windows) by any chance?
I'm on Linux exclusively. I can get it to work by allowing write access for all users on my folder, but it would be nicer if there were a way to let the container act as my user. I tried doing that, but just ended up with a nameless container for some reason (I don't know what I'm doing when it comes to docker). But if it can be fixed by adding the docker user to some group I guess that works too. Might just be a me-problem that an experienced docker user would know how to solve.
the problem I encountered was when creating a new profile
That was an error caused by me assuming there would always be xdg-open installed on a Linux system, not thinking about minimalist installs like docker images that don't have that. It's a utility for opening a file with the system default application. It failing wasn't an issue though, and I suppressed the error message with commit 3664bde.
Oh yeah, on Linux profiles go to XDG_CONFIG_HOME/pcleaner
by default, which is typically ~/.config/pcleaner
. (Also works when xdg isn't configured on the host, like in this container). You can of course give a different path for a profile.
I gave it a test and loading the profile worked just fine.
you can edit them outside the container
That is, if you can even find where the hecc docker-internal files are stored. If it were saved inside the mounted volume, that'd be easy to find, I guess, but that isn't the default location pcleaner uses. Rather, it has some pcleaner user account inside the container somewhere. Accessing those with the text editors inside the container worked like a charm though, so I guess it's useful as a backup. Good to know about that re-link on write quirk, that could cause some nasty headaches!
docker version
I have version 23.0.1 (~1 month old), Buildx was introduced in version 19.03. (~2 years old). Your version came out approx. 2 months ago, so relatively new as well. \
It just changes docker image build -t pcleaner:v1 .
to docker buildx build -t pcleaner:v1 .
so no big deal.
Thanks for looking into it, hope this clears a few things up.
Well I'm not really that experienced managing containers lol but-
I can get it to work by allowing write access for all users on my folder
I don't really understand the problem, I mean I understand but I guess what I'm asking is why it's happening to you, I didn't have problems, which doesn't mean nobody will have them, but exactly what output files you mean, the cleaned images?
The user that is created within the container should have access to the bind mount and be able to write to it, but then again I'm using macOS, not sure if it's specific to linux
but it would be nicer if there were a way to let the container act as my user.
There is different ways to do it but should not be necessary
on Linux profiles go to XDG_CONFIG_HOME/pcleaner by default, which is typically ~/.config/pcleaner. ... That is, if you can even find where the hecc docker-internal files are stored.
Right, now I see the predicament, no yeah it'd be better if we install nano inside the container, I thought you meant like existing files you already have in your host or new files that were written to the current directory (mounted directory)
Let's explore the issue with Linux if you can give me more context, I'll try to run a Linux VM and try to replicate
Yeah, it fails to write the cleaned images to the mounted volume (permission denied), since it only has read access by default, and the container acts as a different user, not my current user that owns the files. Changing that 1001 to 1000 actually completely fixed the issue, it now works as before, just minus that problem. Was there a reason to have it not be 1000?
This article seems to explain it better than I could, the article you linked also helped me here.
Not sure why the issue wasn't on MacOS, maybe something to do with how it's virtualized(?), but do let me know if the following patch breaks that platform. If it doesn't, I think the dockerfile is good to go. :)
PS. I also added a few changes to the readme, let me know if you can improve it further or I made a mistake.
maybe something to do with how it's virtualized
You may be right!, actually the way docker works in macOS is via a lightweight VM contrary to Linux where containers run in a native way 🤔
Digging a little deeper I found that
In macOS, the osxfs driver pretends that the files are owned by the USER that the container runs as. If you are seeing mounted files as being owned by root, your container probably is set to run as root.
There's no harm done in changing the user id to match the same user id from the host, we can either pass it through the $UID variable but probably there's a better way to do it, let me investigate a little and I'll update the Dockerfile
Ah yeah, that does explain why there was no problem on macos, mystery solved.
All right, once you're happy with the results and finished tweaking it, I'll merge. \ Thanks a lot, I learned a bit about docker :)
Hey, have you come up with any changes? If not, I suppose it's fit for merging next week. Thanks for all you've done!
Changes
Notes: The usage should still be the same just inside a container, I have tested in macOS no CUDA support and works well, I just had some issues using custom profiles not sure if it's a dependency issue but will look into it later