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.

Sign up for GitHub

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

Jump to bottom

Fix Fedora and CentOS builds #3246

Merged
merged 2 commits into from
Dec 29, 2021
Merged

Fix Fedora and CentOS builds #3246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
215a27b
Fix fedora build version
thornbill Dec 29, 2021
25c4718
Fix centos build permissions
thornbill Dec 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions fedora/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ $(info $(shell set -x; if [ "$$(id -u)" = "0" ]; then echo "Installing git"; dnf
NAME := jellyfin-web
VERSION := $(shell set -x; sed -ne '/^Version:/s/.* *//p' $(DIR)/$(NAME).spec)
RELEASE := $(shell set -x; sed -ne '/^Release:/s/.* *\(.*\)%{.*}.*/\1/p' $(DIR)/$(NAME).spec)
GIT_VER := $(shell set -x; git describe --tags | sed -e 's/^v//' -e 's/-[0-9]*-g.*$$//')
SRPM := jellyfin-web-$(subst -,~,$(GIT_VER))-$(RELEASE)$(shell rpm --eval %dist).src.rpm
TARBALL :=$(NAME)-$(subst -,~,$(GIT_VER)).tar.gz
SRPM := jellyfin-web-$(subst -,~,$(VERSION))-$(RELEASE)$(shell rpm --eval %dist).src.rpm
TARBALL :=$(NAME)-$(subst -,~,$(VERSION)).tar.gz

epel-7-x86_64_repos := https://rpm.nodesource.com/pub_16.x/el/\$$releasever/\$$basearch/

Expand All @@ -20,9 +19,9 @@ $(DIR)/$(TARBALL):
cd $(DIR)/; \
SOURCE_DIR=.. \
WORKDIR="$${PWD}"; \
version=$(GIT_VER); \
version=$(VERSION); \
tar \
--transform "s,^\.,$(NAME)-$(subst -,~,$(GIT_VER))," \
--transform "s,^\.,$(NAME)-$(subst -,~,$(VERSION))," \
--exclude='.git*' \
--exclude='**/.git' \
--exclude='**/.hg' \
Expand All @@ -34,7 +33,6 @@ $(DIR)/$(TARBALL):
-C $${SOURCE_DIR} ./

$(DIR)/$(SRPM): $(DIR)/$(TARBALL) $(DIR)/jellyfin-web.spec
./bump_version $(GIT_VER)
cd $(DIR)/; \
rpmbuild -bs $(NAME).spec \
--define "_sourcedir $$PWD/" \
Expand Down
4 changes: 4 additions & 0 deletions fedora/jellyfin-web.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ Jellyfin is a free software media system that puts you in control of managing an
%build

%install
%if 0%{?rhel} > 0 && 0%{?rhel} < 8
# Required for CentOS build
chown root:root -R .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still going to break any properly built RPM build system.

You cannot do root things in an RPM scriptlet and SHOULD NOT build RPMs as root.

I am working on fixing all of these issues in another PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brian, I'm going to be as blunt as I can, because I think that's the only way to get this point across clearly at this point. We do not care what you say we can and cannot do, or what's "proper". We care what works. And given that this has been broken for well over a month due to your previous "improvements", we're going to go with what works. Despite pointing it out to you and asking repeatedly for you to fix it, you've argued about it and denied that it was a problem. This whole situation has (again) prompted discussions about removing official support for rpm systems since none of us use them and it's brought the whole build system to it's knees.

TLDR: Until you're willing to put in the work and hand us something that is guaranteed to work properly and not over complicate things more than they already are, stop criticizing. Your previous efforts are leaving something to be desired

%endif
npm ci --no-audit --unsafe-perm
%{__mkdir} -p %{buildroot}%{_datadir}
mv dist %{buildroot}%{_datadir}/jellyfin-web
Expand Down