linuxserver / docker-jellyfin

GNU General Public License v3.0
607 stars 91 forks source link

Use User Supplied Owner for Web Files #192

Closed Silvenga closed 1 year ago

Silvenga commented 1 year ago

linuxserver.io



Description:

This resolves a common pain point in using some common Jellyfin plugins, notably https://github.com/nicknsy/jellyscrub.

Benefits of this PR and context:

This resolves https://github.com/linuxserver/docker-jellyfin/issues/182.

This also prevents having to run Jellyfin as root (the most common work around).

How Has This Been Tested?

docker build . -t test
docker run -it --rm test
docker exec -it <container id> bash

/usr/share/jellyfin# ls -las
total 144
  8 drwxr-xr-x 1 root root   4096 Feb 27 23:28 .
  8 drwxr-xr-x 1 root root   4096 Feb 27 23:28 ..
128 drwxr-xr-x 1 abc  abc  122880 Feb 27 23:28 web
LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/jellyfin/10.8.9-1-pkg-a0882b73-pr-192/index.html https://ci-tests.linuxserver.io/lspipepr/jellyfin/10.8.9-1-pkg-a0882b73-pr-192/shellcheck-result.xml

Silvenga commented 1 year ago

Ooh, that's slick...

LinuxServer-CI commented 1 year ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/jellyfin/10.8.9-1-pkg-a0882b73-pr-192/index.html https://ci-tests.linuxserver.io/lspipepr/jellyfin/10.8.9-1-pkg-a0882b73-pr-192/shellcheck-result.xml

thespad commented 1 year ago

My only concern is the startup performance hit on chowning that many files because of the COW penalty on first modify.

@drizuid you've still got the overlayfs bug on one of your hosts haven't you? Could you compare the live with PR image here and see if it's a big difference?

Silvenga commented 1 year ago

Ah, interesting. Would changing this to be opt-in be safer @thespad? e.g. an environment variable CHANGE_OWER_OF_WEB_FILES=1 (I'm bad at naming things).

drizuid commented 1 year ago

Ah, interesting. Would changing this to be opt-in be safer @thespad? e.g. an environment variable CHANGE_OWER_OF_WEB_FILES=1 (I'm bad at naming things).

We are not big fans of adding envvars.

@drizuid you've still got the overlayfs bug on one of your hosts haven't you? Could you compare the live with PR image here and see if it's a big difference?

I'll do some timing on it, but it might be 4-18 hours from now, depending on what is going on here.

drizuid commented 1 year ago

sorry for the delay, here are the results.

===latest Build===
2023-03-03T14:47:36.031532864Z ───────────────────────────────────────
2023-03-03T14:47:36.031537456Z
2023-03-03T14:47:36.081444845Z [custom-init] No custom files found, skipping...
<truncated>
2023-03-03T14:47:40.117514048Z [ls.io-init] done.

As shown above, the latest build is about 4 seconds start to finish on startup

===PR Build===
2023-03-03T14:44:22.800273404Z -------------------------------------
2023-03-03T14:44:22.800277760Z
2023-03-03T14:45:16.726852499Z [custom-init] No custom files found, skipping...
<truncated>
2023-03-03T14:45:20.763270416Z [ls.io-init] done.

this PR is just about a full minute from start to finish.

My suggestion is that due to the issues this PR would cause many users, this should be made into a docker-mod. That way, users can load it if they want this capability and are willing to accept the slow down. https://docs.linuxserver.io/general/container-customization

thespad commented 1 year ago

Also worth nothing that for some people it won't have a significant impact, PR build on my host for example:

2023-03-03T15:54:18.543704622Z -------------------------------------
...
2023-03-03T15:54:19.422871284Z [custom-init] No custom files found, skipping...
...
2023-03-03T15:54:22.454253239Z [ls.io-init] done.
thespad commented 1 year ago

Closing PR as it's not something we'll merge into live, but could be a mod as driz mentioned.