-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
addpkg(main/jellyfin): 10.10.7 #24184
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
base: master
Are you sure you want to change the base?
Conversation
I have been able to personally test the arm64 build but both x86 builds remain untested, would be nice if someone could test those |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The skiasharp
part of this build should probably be split out into a separate package that jellyfin-server
can then depend on.
Same for jellyfin-ffmpeg
probably, although that may be better served as a subpackage of jellyfin-server
.
See also:
TERMUX_SUBPKG_DESCRIPTION="Minimal FFmpeg libraries for Audacity" | |
TERMUX_SUBPKG_INCLUDE=" | |
opt/audacity/lib/ | |
share/doc/audacity-ffmpeg/ | |
" |
Thank you for taking over this PR, it's quite a lot of work.
packages/jellyfin-server/build.sh
Outdated
TERMUX_PKG_DESCRIPTION="A free media system for organizing and streaming media (server)" | ||
TERMUX_PKG_LICENSE="GPL-2.0" | ||
TERMUX_PKG_MAINTAINER="@termux" | ||
TERMUX_PKG_VERSION="10.10.7" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using an array for the version here, this is supported by the build system.
TERMUX_PKG_VERSION="10.10.7" | |
TERMUX_PKG_VERSION=( | |
"10.10.7" | |
"7.0.2.9" | |
) |
The -
part of the version number is reserved for the package revision and should be replaced with a .
here and substituted for the URL.
packages/jellyfin-server/build.sh
Outdated
_FFMPEG_VERSION="7.0.2-9" | ||
TERMUX_PKG_SRCURL=( | ||
"https://github.com/jellyfin/jellyfin/archive/refs/tags/v${TERMUX_PKG_VERSION}.tar.gz" | ||
"https://github.com/jellyfin/jellyfin-ffmpeg/archive/refs/tags/v${_FFMPEG_VERSION}.tar.gz" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use a bit of substitution here to fix the URL for jellyfin-ffmpeg
"https://github.com/jellyfin/jellyfin-ffmpeg/archive/refs/tags/v${_FFMPEG_VERSION}.tar.gz" | |
TERMUX_PKG_SRCURL=( | |
"https://github.com/jellyfin/jellyfin/archive/refs/tags/v${TERMUX_PKG_VERSION[0]}.tar.gz" | |
"https://github.com/jellyfin/jellyfin-ffmpeg/archive/refs/tags/v${TERMUX_PKG_VERSION[1]%.*}-${TERMUX_PKG_VERSION[1]##*.}.tar.gz" | |
) |
packages/jellyfin-server/build.sh
Outdated
local _idx; | ||
local _sha256sums=(); | ||
local _error=0; | ||
( [ "${#_GIT_URL[@]}" -eq "${#_GIT_COMMIT[@]}" ] && [ "${#_GIT_URL[@]}" -eq "${#_GIT_BRANCH[@]}" ] ) || termux_error_exit '_GIT_URL, _GIT_COMMIT and _GIT_BRANCH must be arrays of the same length' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our build scripts are written for Bash, and arithmetic evaluation lets this be expressed a bit more nicely.
( [ "${#_GIT_URL[@]}" -eq "${#_GIT_COMMIT[@]}" ] && [ "${#_GIT_URL[@]}" -eq "${#_GIT_BRANCH[@]}" ] ) || termux_error_exit '_GIT_URL, _GIT_COMMIT and _GIT_BRANCH must be arrays of the same length' | |
(( ${#_GIT_URL[@]} == ${#_GIT_COMMIT[@]} && ${#_GIT_URL[@]} == ${#_GIT_BRANCH[@]} )) || termux_error_exit '_GIT_URL, _GIT_COMMIT and _GIT_BRANCH must be arrays of the same length' |
packages/jellyfin-server/build.sh
Outdated
"git+https://github.com/ericsink/cb.git" | ||
"git+https://github.com/ericsink/SQLitePCL.raw.git" | ||
"git+https://github.com/mono/skia.git" | ||
"git+https://github.com/jellyfin/jellyfin-web.git" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 1.) There shouldn't be
.git
suffixes on those URLs. - 2.) Consider using associcative arrays (hashmaps) if you need a secondary index, e.g. the project name.
kitty
's build script does something similar to this.
termux-packages/x11-packages/kitty/build.sh
Lines 34 to 51 in c1dd8cc
local -A ver=( [libx11]="$(. "${TERMUX_SCRIPTDIR}/packages/libx11/build.sh"; echo "${TERMUX_PKG_VERSION}")" [libxcb]="$(. "${TERMUX_SCRIPTDIR}/packages/libxcb/build.sh"; echo "${TERMUX_PKG_VERSION}")" [xcb_proto]="$(. "${TERMUX_SCRIPTDIR}/packages/xcb-proto/build.sh"; echo "${TERMUX_PKG_VERSION}")" [libxkbcommon]="$(. "${TERMUX_SCRIPTDIR}/x11-packages/libxkbcommon/build.sh"; echo "${TERMUX_PKG_VERSION}")" ) local -A srcurl=( [libx11]="$(. "${TERMUX_SCRIPTDIR}/packages/libx11/build.sh"; echo "${TERMUX_PKG_SRCURL}")" [libxcb]="$(. "${TERMUX_SCRIPTDIR}/packages/libxcb/build.sh"; echo "${TERMUX_PKG_SRCURL}")" [xcb_proto]="$(. "${TERMUX_SCRIPTDIR}/packages/xcb-proto/build.sh"; echo "${TERMUX_PKG_SRCURL}")" [libxkbcommon]="$(. "${TERMUX_SCRIPTDIR}/x11-packages/libxkbcommon/build.sh"; echo "${TERMUX_PKG_SRCURL}")" ) local -A sha256=( [libx11]="$(. "${TERMUX_SCRIPTDIR}/packages/libx11/build.sh"; echo "${TERMUX_PKG_SHA256}")" [libxcb]="$(. "${TERMUX_SCRIPTDIR}/packages/libxcb/build.sh"; echo "${TERMUX_PKG_SHA256}")" [xcb_proto]="$(. "${TERMUX_SCRIPTDIR}/packages/xcb-proto/build.sh"; echo "${TERMUX_PKG_SHA256}")" [libxkbcommon]="$(. "${TERMUX_SCRIPTDIR}/x11-packages/libxkbcommon/build.sh"; echo "${TERMUX_PKG_SHA256}")" )
packages/jellyfin-server/build.sh
Outdated
git fetch --depth 1 origin "${_GIT_COMMIT[_idx]}" | ||
git -c advice.detachedHead=false checkout "${_GIT_COMMIT[_idx]}" | ||
} >/dev/null | ||
# we need a deterministic checksum, logs get in the way |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems convoluted.
What's the goal here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git fetch and checkout are specifically for doing a shallow clone of a specific commit. The >/dev/null and braces are an artifact from the update-git-checksums.sh script though, I'll remove that.
packages/jellyfin-server/build.sh
Outdated
elif [ "$TERMUX_ARCH" = "x86_64" ]; then | ||
_target_cpu="x64" | ||
elif [ "$TERMUX_ARCH" = "i686" ]; then | ||
_target_cpu="x86" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider making this a case
statement instead.
packages/jellyfin-server/build.sh
Outdated
# if [[ -f "debian/patches/series" ]]; then | ||
# quilt push -a | ||
# fi | ||
for i in $(cat debian/patches/series); do git apply --whitespace=nowarn debian/patches/$i; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need to oneliner this.
for i in $(cat debian/patches/series); do git apply --whitespace=nowarn debian/patches/$i; done | |
for i in $(< debian/patches/series); do | |
git apply --whitespace=nowarn "debian/patches/$i" | |
done |
I'd also suggest using $(<
, it's equivalent to $(cat
1.
Footnotes
-
The Bash manual has a bad habit of mentioning useful features offhandedly exactly once.
https://man.archlinux.org/man/bash.1#Command_Substitution ↩
Not sure why this closed on force pushing, might have been my git alias which resets to master to trigger the CI on my repo 🫤
|
Any process? |
I didnt review fully, but I suggest you split libe_sqlite, skiasharp, sqlitepcl.raw to its own package for other .NET packages to reuse Also do not depend on ffmpeg as you will be pulling 2 copies of ffmpeg, depend on ffmpeg's dependencies instead Or I can do it manually zzz |
Splitting the libraries into separate packages was part of my plan but it would require the rpath issue: #24271 to be resolved and I don't have much of an idea of how one could do that other than maintaining a list of CMakeLists that contain targets that need rpath to be set and using sed -i to insert add_linker_options directives after the required target()s, which is somewhat of a messy solution, which is why I was waiting for suggestions on how #24271 could be handled cleanly. To clarify, the part of #24271 relevant to this PR is that rpath for libcoreclr needs to be set to whatever location contains libraries that dotnet applications can DLLImport ( |
I felt the urge to diss a long explanation there but ended up not to since I doubt it will be helpful. You can wait infinitely or make do with adjustment here and move on. |
Alright, I'll update the PR with a commit to add rpath to libcoreclr along with commits adding separate packages for skiasharp and sqlitepcl (libe_sqlite is part of sqlitepcl)
That's mostly intentional because jellyfin needs a copy of ffmpeg to work properly, and it can use system ffmpeg though jellyfin's ffmpeg has additional features, and putting "jellyfin-ffmpeg | ffmpeg" in TERMUX_PKG_DEPENDS generates a CI error with test-buildorder-random since it's a subpackage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since upstream feels so strongly about using its own jellyfin-ffmpeg instead of system ffmpeg then just use jellyfin-ffmpeg. Preferably prioritise disk space usage than pulling unneccessary packages.
You can workaround the issue like so: (I havent check the dependency list yet)
packages/jellyfin-server/build.sh
Outdated
) | ||
TERMUX_PKG_SUGGESTS="jellyfin-ffmpeg" | ||
TERMUX_PKG_BUILD_DEPENDS="aspnetcore-targeting-pack-8.0, dotnet-targeting-pack-8.0, libcairo, pango, libjpeg-turbo, giflib, librsvg" | ||
TERMUX_PKG_DEPENDS="libc++, fontconfig, aspnetcore-runtime-8.0, dotnet-host, dotnet-runtime-8.0, sqlite, libexpat, libpng, libwebp, freetype, ffmpeg" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TERMUX_PKG_DEPENDS="libc++, fontconfig, aspnetcore-runtime-8.0, dotnet-host, dotnet-runtime-8.0, sqlite, libexpat, libpng, libwebp, freetype, ffmpeg" | |
TERMUX_PKG_DEPENDS="libc++, fontconfig, aspnetcore-runtime-8.0, dotnet-host, dotnet-runtime-8.0, sqlite, libexpat, libpng, libwebp, freetype" |
@@ -0,0 +1,4 @@ | |||
TERMUX_SUBPKG_DESCRIPTION="FFmpeg for Jellyfin with custom extensions and enhancements" | |||
TERMUX_SUBPKG_DEPENDS="chromaprint, libdrm, zstd" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TERMUX_SUBPKG_DEPENDS="chromaprint, libdrm, zstd" | |
TERMUX_SUBPKG_DEPENDS="chromaprint, fontconfig, freetype, fribidi, game-music-emu, harfbuzz, libaom, libandroid-glob, libandroid-stub, libass, libbluray, libbz2, libdav1d, libdrm, libgnutls, libiconv, liblzma, libmp3lame, libopencore-amr, libopenmpt, libopus, librav1e, libsoxr, libsrt, libssh, libtheora, libv4l, libvidstab, libvmaf, libvo-amrwbenc, libvorbis, libvpx, libwebp, libx264, libx265, libxml2, libzimg, libzmq, littlecms, ocl-icd, rubberband, svt-av1, xvidcore, zlib, zstd" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add the deps to the subpackage but chromaprint pulls in ffmpeg anyways for FFT
packages/jellyfin-server/build.sh
Outdated
popd | ||
} | ||
termux_step_pre_configure() { | ||
termux_setup_dotnet; termux_setup_gn; termux_setup_nodejs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
termux_setup_dotnet; termux_setup_gn; termux_setup_nodejs | |
TERMUX_PKG_DEPENDS+=", jellyfin-ffmpeg" | |
termux_setup_dotnet; termux_setup_gn; termux_setup_nodejs |
Probably out of the scope of this PR, but (aping how Alpine Linux does it) chromaprint could be split into chromaprint and libchromaprint by linking with fftw instead of ffmpeg for the FFT, which would make ffmpeg a dependency of the main chromaprint package only (for fpcalc), and jellyfin-ffmpeg could dep on libchromaprint to avoid pulling in ffmpeg. I'd prefer to do this after this package is merged in. |
Woops, didn't get the failed re-rerun in my notifs. I'm going to rebase if that's ok with you @truboxl |
Please do |
I will take a look splitting chromaprint It is still unacceptable large to pull 2 copies of ffmpeg |
I have done that + split two other packages in https://github.com/NORALDM/termux-packages/tree/split-packages. If you want I can cherry pick the chromaprint split + revbumps into this PR but I feel like it would be suitable for another PR. |
Ok, you can open a new PR |
Closes #9751
Supersedes #23074
Had to go a bit overboard with the post_get_source step because multiple git urls in TERMUX_PKG_SRCURL don't seem to be supported. Most patches are taken from the termux packages repo except for skia-DEPS.patch, skia-gn-build.patch, and jellyfin-defaults.patch.