anaconda-webui and cockpit appear to violate Javascript and CSS compilation guidelines #1496
Labels
No labels
announce
bootstrap
Closed As
accepted
Closed As
duplicate
Closed As
exceptionexpired
Closed As
fixed
Closed As
invalid
Closed As
nothingtodo
Closed As
permanentexception
Closed As
rejected
Closed As
temporaryexception
Closed As
wontfix
cmake
committee
draftneeded
hasdraft
meeting
meson
needinfo
Priority
In Committee
Priority
Needs Review
Priority
Waiting For Reporter
rust
writeup
No milestone
No project
No assignees
6 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
packaging/guidelines#1496
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The packaging guidelines say:
"If a JavaScript library typically is shipped as minified or compiled code, it MUST be compiled or minified as part of the RPM build process. Shipping pre-minified or pre-compiled code is unacceptable in Fedora."
I think anaconda-webui is violating this.
If I try and build anaconda-webui with a patch that changes one of its javascript source files - e.g. https://patch-diff.githubusercontent.com/raw/rhinstaller/anaconda-webui/pull/1037.patch - the build fails like this:
that is, when the package's
%installstep runsmake install, some part of the upstream build system notices that a node.js 'build' step -NODE_ENV=production ./build.js- is required, and tries to run it (and it fails, clearly indicating the spec is not set up to handle actually doing the compilation).In the logs of official builds of anaconda-webui - e.g. this build.log from this build - there is no trace of any node.js compilation happening in the
%installstep, which implies that the build system realizes the pre-compiled assets in the tarball - they're in theanaconda-webui/distsubdirectory - are up to date with the sources, and skips compiling them. That means the build works. But it is, IMO, very clearly against the guidelines.The 'meat' of the shipped packages appears to be the files:
and these files are just copied straight from the
dist/folder of the tarball, by the looks of it.As well as being a violation of the guidelines, this is a practical problem, because it makes it very difficult for me to test changes to anaconda-webui by doing a modified package build...because I can't do one.
CC @kkoukiou
oh, I should note there is a similar rule for CSS which the package also appears to violate:
"Pure CSS frameworks can be included as-is. CSS frameworks that use an alternative language that compiles to CSS, such as LESS, must compile to CSS as part of the build process.
Packages containing CSS should make the best effort to regenerate any precompiled/minimized CSS wherever possible, as this leads to more maintainable packages. Where this would result in a significant hardship, the bundled pregenerated CSS may be shipped with a specfile comment explaining the decision. This does not eliminate the requirement to validate licenses of bundled content."
To me that is poorly drafted, as it's unclear how the "best effort" language in the second paragraph interacts with the "must" language in the first paragraph, but either way, the package doesn't comply, as there's no spec file comment explaining any "significant hardship".
The initial package review does not appear to have ever considered the situation wrt these guidelines.
I think this is a cockpit thing in general, probably all the cockpit packages need to be re-evaluated.
Yes, I was about to file another issue saying the same thing: cockpit appears to violate the same guidelines. Exactly the same test proves it: try building a cockpit package with a patch to any of the javascript sources in the
pkg/directory. The build will fail:CC @martinpitt
The guidelines also say
and there's a similar guideline for CSS. So it seems the guidelines are self-contradictory here. At the very least, there needs to be a specfile comment explaining the decision, which seems to also be missing from the anaconda-webui package.
In general, I think there needs to be more guidance and tooling for packaging "web stuff." It's good to have rules, but in this case in particular, they're not backed up with clear guidance on how an everyday packager (e.g., a Go packager packaging an app such as openbao or prometheus or forgejo that also includes web assets alongside Go source code or a Python packager building Sphinx docs or a package like cockpit/anaconda-webui) can handle web assets while following the bundling and licensing guidelines.
The reason the Go packaging change is taking so long (in addition to me needing to find a block of time where I can sit down and finish writing the guidelines) is because we wanted to make sure there was tooling and practical guidance in place to handle bundling and licensing and security updates before changing the guidelines and retiring all the library packages.
I (at the time), after some discussion and seeing other packages use the same idiom, felt this to be under the 'hardship' part of the guideline and didn't fully consider the implications to downstream patching.
Apologies since that discussion should've taken place in the BZ and the comment should've been there.
Agreed, I'm working currently on packaging a web application with associated (and upstream patched and built javascript assets) and I've been having a hard time finding the normal approach though this issue clarifies some bits :)
Yeah, we are painfully aware of this situation. We've been trying to take stabs at this for years. I made quite some progress in December here https://github.com/martinpitt/cockpit-files/tree/distrobuild , but it needs more work. The biggest blocker is https://bugzilla.redhat.com/show_bug.cgi?id=2387212 - without that, we'd have to ship esbuild in node_modules/ as part of the Fedora upstream tarball, and that sucks as it's an arch specific binary.
In Debian, I got node_modules/ down to basically PatternFly and dart-sass, plus very few very small extra modules which are unproblematic . Unfortunately PatternFly itself has a zillion dependencies, and dart-sass is quite big as well (see https://bugs.debian.org/1051470 and https://bugs.debian.org/998024 , same situation in Fedora). Fedora also does not package React (Debian does).
So the current plan to fix this on our side is:
That's a lot of bundling (which is also frowned upon), but the best that we can realistically do.
TL/DR: Packaging web apps in distros is hard 😢
Thanks for the info. Is there a note in the spec explaining this? I took a quick look and didn't see one, but I may have missed it.
I worked on Fedora's esbuild package today, and got it working. Updated https://bugzilla.redhat.com/show_bug.cgi?id=2387212 and sent https://src.fedoraproject.org/rpms/golang-github-evanw-esbuild/pull-request/13
Once that lands in current Fedora releases, that resolves the biggest blocker of building the js bundles during rpm build. There's some more missing bits, but I'm working on them.
More yak shaving, I'm afraid. Before we can fix the esbuild package (without hacks), we need to fix nodejs packaging: https://bugzilla.redhat.com/show_bug.cgi?id=2404330
I guess @martinpitt forgot to update this, but good news: this is fixed in cockpit now! Many thanks for that.
Hopefully anaconda-webui can follow suit.
I just opened #1539 to try to fix the self-contradictory guidelines. Is there anything else to track here?
We merged the PR to make the guidelines internally consistent and follow the previous decision to allow pre-generated CSS and JS, but there's still an open question of whether we want to keep this exception, because it makes it impossible to patch/rebuild things downstream (as was pointed out in this ticket), and also, giving "significant hardship" as a reason for an exception is... a hardship to enforce.
I think the biggest issue/cause of "hardship" is lacking proper tooling to deal with packaging of web assets. I wrote tools to properly deal with the JS assets in https://git.sr.ht/~gotmax23/rpm-python-yt-dlp-ejs/tree since nothing in nodejs-packaging worked. forgejo also required custom tooling and eventually switched to using pre-compiled web assets. Every package shouldn't have to reinvent the wheel.
I wonder if we should not treat pre-minified / "compiled" JavaScript as essentially equivalent to binary blobs? Both are basically opaque and impossible to patch downstream. Making a distinction here just because the former is technically a plaintext format doesn't really make sense IMO.
I agree. They basically are blobs.
What about transpiled typescript? The package source used for
openclawis from https://registry.npmjs.org which contains a pre-builtdistdirectory of javascript files that were transpiled from the actual typescript sources as part of the upstream release process. It's not a big minified one-line blob (so you can read it in a normal text editor), but it's not the original source code. either, and you still can't directly apply upstream patches to it. The Guidelines recommend npmjs as the default package source for nodejs- packages, even though it's apparently common to have code that's been modified in some way compared to the upstream git repo.Also, perhaps there should be a distinction for non-essential bundled Javascript (e.g., a copy of JQuery in docs) as opposed to pre-building an entire web app that's a core functionality of the package.