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
Add Dockerfile support #3
Conversation
xdg-open isn't installed in this container, and isn't necessary, so just cleanly fail when that's the case.
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. 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 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? |
What OS are you using? Are you using WSL(windows) by any chance?
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.
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
You mean the command |
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.
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 I gave it a test and loading the profile worked just fine.
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!
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. Thanks for looking into it, hope this clears a few things up. |
Well I'm not really that experienced managing containers lol but-
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
There is different ways to do it but should not be necessary
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. |
When using UID 1000, the container will act as the current user (provided it is also UID 1000) thereby not requiring any special access modifiers to write files to the mounted volume.
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
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. |
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