binary executable content in node_modules #3622
Labels
No labels
Closed As
Accepted
Closed As
Duplicate
Closed As
Insufficient data
Closed As
Invalid
Closed As
Rejected
document it
fast track
meeting
next release
nonresponsive maintainer
packager revocation
pending announcement
provenpackager
python 2 exception
self contained change
stalled
system wide change
updates policy exception
vote-in-progress
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
fesco/tickets#3622
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 nodejs-packaging-bunder does not strip executable binary files from a node_modules directory before creating a "source" archive for dist-git. This has resulted in a number of Fedora packages being built from archives that include pre-built executables which are not allowed by policy.
In some cases, the prohibited content is not essential to the build process and removing it will not further complicate the RPM build process. In others, it might be possible to strip the binaries and to add "npm rebuild" to the build and check sections of the rpm spec. However, this will not always work and it may be necessary to build more npm modules from source. And that task might be difficult where specific versions of components are required in a node_modules tree.
Steps to Reproduce:
https://bugzilla.redhat.com/show_bug.cgi?id=2491084
https://bugzilla.redhat.com/show_bug.cgi?id=2491085
https://bugzilla.redhat.com/show_bug.cgi?id=2491086
https://bugzilla.redhat.com/show_bug.cgi?id=2491087
https://bugzilla.redhat.com/show_bug.cgi?id=2491088
I've started work to illustrate some of the changes that will be required, here:
https://codeberg.org/gordonmessmer/nodejs-undici/compare/rawhide...strip-node-modules
... but the process of stripping executable binaries probably belongs in nodejs-packaging-bundler, not in individual package "sources.sh" scripts
Re. the main subject of the FESCo ticket, I think that prebuilt binaries should be removed from npm vendor archives before they are created and uploaded to the lookaside cache, but the Guidelines don't strictly require this as far as I understand them. https://docs.fedoraproject.org/en-US/packaging-guidelines/what-can-be-packaged/#prebuilt-binaries-or-libraries only says that they need to be removed in
%prepand not used during the build process (as opposed to legally problematic content which is not allowed to be uploaded at all).If these packages (it would be helpful to have a list of package names in this ticket addition to the bugzillas that people have to click through) are not removing the prebuilt binaries during %prep, either, then yes, that's something that the package maintainers need to address/fix. If that doesn't happen in a reasonable amount of time, FESCo can decide to orphan or retire them.
Re. general issues with NodeJS packaging in Fedora, I have more to say here and agree with the sentiment on the devel thread that there's not enough tooling or guidance or Guidelines to properly package "web stuff" In Fedora. There were efforts to change the Guidelines in https://pagure.io/fesco/issue/3177 but that introduced new problems (packaging/guidelines#1496). So this is something that FESCo and/or the FPC need to discuss (in a separate ticket, probably).
@gordonmessmer, is this supposed to be an exhaustive list of packages with this issue or are there potentially others?
It's probably a complete list, but I built it by getting the "all specs" archive, looking for specs that reference "nm-prod.tgz" or "nm-dev.tgz", cloning those repositories, fetching their source archives with fedpkg, extracting those, and then:
Yeah, that makes sense. I was thinking that if we wanted this to be done consistently, then it probably should be part of the bundler, but we could also provide a macro for %prep and require nodejs packages to use it.
Thanks for clarifying. That sounds like a reasonable way to detect most of these cases. There's probably other packages that don't use the nodejs-packaging scripts that have similar problems.
In any case, what should FESCo do here? The main levers we have are to orphan/retire packages if the maintainers are unresponsive to serious bugs (which we should probably wait to do, since it's only been a couple days). I agree that the tooling should be improved here, but that's mainly up to the nodejs SIG to decide on and implement. Any issues with Packaging Guidelines are up to the FPC, which has been discussing issues with nodejs and web assets packaging recently as well.
I hope that the resolution is that we find an acceptable process for node.js packages, not that we retire them. Specifically, I'd like to finish the review in bz#2464801.
I apologize if I've filed this in the wrong queue. It sounds like the issue should be referred to the FPC? I can copy the ticket to https://forge.fedoraproject.org/packaging/guidelines/issues
Is it FESCo or FPC's responsibility to determine a timeline for resolution?
Sorry to answer your question with a question :), but what precisely is the blocker for that review request, and what do you mean by "timeline for resolution"? If the question is whether pre-built binaries can be used in the build process (or shipped to users in the RPM), I think the answer is unambiguously no. Cases where this occur should be caught/fixed during the package submission process, and the existing packages with issues that you identified need to be fixed also.
Not having an acceptable packaging process. :)
How much time do we have to fix those packages?
I've been trying to work out a process for these packages. Best I've come up with so far is that any modules that contain binaries probably need to be removed entirely and packaged from source, not from npm.
Yeah, there's no argument from me here.
Got it. I think that's under FESCo's purview. As far as I know, there's not really a set policy for this. FESCo has in the past retired packages for serious bugs that have gone unaddressed by the maintainers (e.g., the decision about the Deepin DE stack), but this is reserved for serious cases and only after attempts to work with the package maintainers have been exhausted. I think it's slightly early to start having this conversation about the nodejs packages listed here, since the bugs have only just been filed. Let's see what other FESCo members think.
Re. unblocking your review request:
Yes, agreed. That was done recently for nodejs-esbuild (part of golang-github-evanw-esbuild.src).
In the case of libraries that are pulled in as part of dev dependencies but only used for linting or other upstream development purposes, removing them from package.json before creating the vendor archive can help reduce problematic dependencies. That what I did in https://git.sr.ht/~gotmax23/rpm-python-yt-dlp-ejs/tree.
If you have changes to the nodejs-packaging bundling script that you need for your package, I'd say just include your modified version as a package Source and use that until the changes can get merged into nodejs-packaging.
This was discussed a bit during this week's FPC meeting.
One of the suggestions for how to deal with this situation was
Would this make sense as a way forward?
@decathorpe To whom is the question directed? Those are all things I've suggested, so naturally they make sense to me. 😄
The "flag date" suggests there should be a ticket to track status, and it sounds like this is more a matter for FPC than FESCo. Is https://forge.fedoraproject.org/packaging/guidelines/issues the right place for that? (I would like to avoid creating yet another ticket in the wrong queue.)
In this case, to other FESCo members.
Because if this makes sense, I would make that an "official" proposal. This should probably also involve an announcement to the devel mailing list so there is possibility for feedback before we vote.
Responsibilities for packaging guidelines are delegated from FESCo to FPC, but FPC doesn't really have powers to do enforcement, so having a FESCo ticket (and eventually, a releng ticket, if / when retirements become necessary) should be OK - plus an FPC ticket / PR for actually changing the guidelines as / if needed.
@decathorpe wrote in #3622 (comment):
I'm not opposed to setting to flag date, but yes, there definitely needs to be room for feedback here, particularly from these maintainers and the packaging tooling maintainers before we set/propose one. Is your proposal meant to be executed in steps (i.e., we wouldn't set the flag date until the first step has happened)? Otherwise, I'm weary of setting a flag date if nobody is going to actually work on improving the tooling, and we're basically just agreeing to retire everything at that point (or to have to keep pushing back the flag date until the tooling improvements materialize).
Also, if we say that packagers have to strip out these binaries and separately package them and their build dependencies (particularly binaries in build tools like esbuild and rollup), does that create additional hardship which would activate the exception from #3177 that allows packages to ship pre-built Javascript bundles and not have to bother with npm deps at all? At least for
openclawwhich I looked at yesterday, it does not have this problem because it already uses pre-compiled/transpiled javascript and doesn't actually build anything to begin with... So I think FESCo and/or the FPC should figure that out here as well. It's not clear to me why FESCo decided to make the exception in #3177 to begin with since the Packaging Guidelines fall under the FPC. The FPC is currently discussing removing this exception in packaging/guidelines#1496.Yes, it was supposed to be sequential steps.
The "hardship" exception does not override the hard prohibition on using prebuilt executables, so this shouldn't be a problem.
For cases that don't "build" anything and only contain transpiled code - those are likely running afoul of other rules anyway (i.e. those for pregenerated code: https://docs.fedoraproject.org/en-US/packaging-guidelines/what-can-be-packaged/#pregenerated-code).
The rules for pre-built libraries and binaries, and for pregenerated code might need definitions that are less ambiguous. Especially "code". I am not confident, as a reviewer, if minified JS should be treated the same as JS that has been transpiled from TypeScript, or the same as Wasm.
Offhand, the rules kind of seem like "pre-built binaries or libraries" might mean "files that end up in a 'binary' rpm", while "pregenerated code" might refer to files that are only used during the build phase, whether as scripts like configure or Makefile, or as code such as bison/lex/protobuf output.
If that were the case, then minified JS, and JS that had been transpiled from TypeScript or any other language, and Wasm are all effectively the same and none of them would be allowed. Minified JS might sometimes be identified by its filename, but I'm not actually sure if we can reliably identify transpiled JS.
But if minified JS is allowed, then that's probably not the correct interpretation of those two sections, and I'm not sure how to treat transpiled code given the statement, "It is suggested, but not required, that such code be regenerated as part of the build process"