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
Cleanup and standardise Fedora build (web) #3571
Cleanup and standardise Fedora build (web) #3571
Conversation
c21b024
to
46c2b3d
Compare
46c2b3d
to
eb007f8
Compare
Kudos, SonarCloud Quality Gate passed! |
Rewrite so we don't need to constantly update with every new Fedora release. This is especially useful when Fedora and Jellyfin release cycles don't line up. Version selection is as follows: * TARGET environment variable, which is currently used already * Currently running Fedora version * Hardcoded Fallback version that can be updated occasionally
* move actual building process to %build * remove AutoReqProv as the package purely contains text files and fonts. There's no dependencies to begin with. This feature is also intended as sort of a "last resort" and we don't need this here. * define LICENSE as %license, which automatically puts it in a standardised directory
* when running Jellyfin as a user from a terminal without passing arguments, it would not find the web-files. This moves them to the expected/default location. * fixes jellyfin#2059
1c36aa0
to
26ec0e8
Compare
Rebased onto v10.8.0 Tests done✔️ ❌ This fails due to jellyfin-web/fedora/jellyfin-web.spec Lines 33 to 36 in e727eed
But from what I can tell this was introduced in #3246 on purpose, although I'm unclear in how this relates to the referenced issue #3222. Either way, I hesitate to remove those lines for now seeing as they were intended to fix issues with the deployment. And since these packages are essentially just a collection of text files, even installing the Fedora Package on EPEL (although not exactly clean) should be fine. ❌ These both fail on the same
Unclear why this is or how to fix it. |
Might be wrong, as I am not 100% familiar what exactly the prepare script does, but the last time I started the CI migration from ADO to GHA I set the EnvVar Edit: Just confirmed the |
Seems like that doesn't produce any build at all (which the long comment in the prepare.js states as well, so not sure why disabling it even works).
I'm assuming the change is related to Searching for |
my bad, overlooked the detail of the old NPM version, yeah that can cause problems. This is how I got web to build in my earlier attempts to migrate from ADO to GHA: jellyfin-web/.github/workflows/publish-container.yaml Lines 69 to 77 in e4b8321
This ended up producing the static JS & HTML files to throw into an NGINX container, so it should be what we are looking for here too. (obviously omit the |
Just tried that and it didn't work either... it doesn't seem to like the webpack in general:
The debug log it mentions: At least we're not alone, Brian's COPR builds are failing as well: What throws me off is this:
So I checked the webpack docs and it says
So for sanity I checked both versions again.
RHEL8 (well, Alma, but still):
So this should be fine? What's up with "not supported" then, I don't get it. |
given this is NodeJs we talking about and usually we compile web with v14 and b4 that with v12 it may very well be that we are using APIs or features introduced in versions of NodeJs after v10. Given that, I assume we wont have any luck with RHEL 8 unless we consider using Code Streams (IIRC that was what RedHat dubed the faster updating supported versions of packages). |
Oh well, see I didn't even check when that version released, apparently 2018 :D And it went EOL in April last year. But, no point thinking about it. At the end of the day it's your call if you want to merge regardless and just let RHEL be broken. After all, it's just a collection of web files so as mentioned earlier people can just install one of the Fedora packages on RHEL instead. It's not very clean, but I'm fairly certain it would work, since once it's all packed up, Node isn't required anymore right? |
RedHat being RedHat, keeping very old version alive due to their LTS support. That is the answer why they have such an old version that NodeJS may no longer support but RedHat will. Regardless, they introduced Application Streams with REHL 8, which allow you to exactly fast track and install newer versions if you need them.
edit: 10.8.1 not happening today, gives us time to reflect if it would break CI. (noticed we build against CentOS (unsure the version or if Stream) so I will check if the changes will run in practically what the CI would run and see if it borks xD |
I saw that before but I was confused by Azure's OS info in the
Guessing this is just because it's Docker and the CentOS Docker is on CentOS 7. CentOS 7 works just fine for the CI because of the TLDR it shouldn't break the CI (unless you go CentOS 8, which might break it, I don't know). |
Yeah, ADO nor GHA offer any other Linux Distro runners than Ubuntu, to work around that we use a container to build, as you already spotted. I will still simply validate on my local podman host, so we are extra sure CI wont fail during releasing 😉 |
Fedora 36 doesn't seem to ship make, so add it manually.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Cleanup and standardise Fedora build (web) (cherry picked from commit c20243c) Signed-off-by: Bill Thornton <billt2006@gmail.com>
Changes
/web
and points the env-var there.latest
36
Fedora image in Fedora Docker builds36
as requested in server-PR reviewRelated Issues