Bug 38179 - koha-common runtime dependency on yarn
Summary: koha-common runtime dependency on yarn
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords: rel_25_05_candidate
Depends on: 37303
Blocks:
  Show dependency treegraph
 
Reported: 2024-10-15 17:24 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2024-11-01 08:16 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2024-10-15 17:24:59 UTC
On main as of bug 37303, for 24.11 onwards, in order to run:

```shell
koha-translate --install <LANG>
```

Both `yarn` and `po2json` are required.

A recollection of information about it:

* Debian's version of node-po2json is not what we want: bug 37303 comment 21 => we don't want to be stuck with v0.4.5
* Yarn version might not be an issue
* Node.js version could be an issue

This change got reverted on 24.05.x (bug 38164)
Comment 1 Tomás Cohen Arazi (tcohen) 2024-10-15 17:29:59 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi (tcohen) 2024-10-15 17:30:27 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi (tcohen) 2024-10-15 18:31:08 UTC
One option here is to provide instructions for installing what is needed, as part of the install steps for Koha. Something like:

```shell
# Add Node.js version 18
wget -O- -q https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key \
      | gpg --dearmor \
      | tee /usr/share/keyrings/nodesource.gpg >/dev/null \
   && echo "deb [signed-by=/usr/share/keyrings/nodesource.gpg] https://deb.nodesource.com/node_18.x nodistro main" > /etc/apt/sources.list.d/nodesource.list

# Add Yarn repo
echo "deb https://dl.yarnpkg.com/debian/ stable main" > /etc/apt/sources.list.d/yarn.list \
   && wget -O- -q https://dl.yarnpkg.com/debian/pubkey.gpg \
      | gpg --dearmor \
      | tee /usr/share/keyrings/yarnkey.gpg >/dev/null \
   && echo "deb [signed-by=/usr/share/keyrings/yarnkey.gpg] https://dl.yarnpkg.com/debian stable main" | tee /etc/apt/sources.list.d/yarn.list

# Pin nodejs version
echo "Package: nodejs"      >  /etc/apt/preferences.d/nodejs
echo "Pin: version 18.*"    >> /etc/apt/preferences.d/nodejs
echo "Pin-Priority: 999"    >> /etc/apt/preferences.d/nodejs

# Install Node.js and Yarn
apt-get update \
   && apt-get -y install nodejs yarn

# Install required tool
yarn global add po2json
```
Comment 4 Katrin Fischer 2024-10-16 14:31:25 UTC
This is a blocker for release and needs to be resolved one way or another soon.
Comment 5 David Cook 2024-10-17 02:58:07 UTC
I'm not 100% sold on adding NodeJS to the prod dependencies, but I'm not 100% opposed either. 

There are pros and cons both ways, and I have included NodeJS on other non-Koha projects, because I've needed functionality in a prod install that I couldn't get elsewhere.

I've provided a bunch of info on Comment 27 and Comment 28 of Bug 38164, which I'll summarize here:

1. I don't think that we need "yarn" in production. 

2. We would need NodeJS as a prod dependency, and version is likely to be an issue, especially between Debian and Ubuntu and even within different versions of those OSes. (As Tomas notes, I think we'd have to mandate a NodeJS package version and specify how to get it.)

3. We can include "po2json" in our package.json, install it on our dev/build Koha servers, and bundle it into 1 single "po2json-bundle" script for production deployments, so that we don't pollute the whole server for this 1 little feature. (NOTE: We're already used to bundling things for installs, so this should be easy for the Koha community.)
Comment 6 David Cook 2024-10-17 03:00:01 UTC
Note: for a "po2json-bundle", I'd suggest that it live in /usr/share/koha/bin/nodejs_tools or something like that. Something CLI-specific and non-web accessible.
Comment 7 Kyle M Hall (khall) 2024-10-17 18:46:00 UTC
> 3. We can include "po2json" in our package.json, install it on our dev/build
> Koha servers, and bundle it into 1 single "po2json-bundle" script for
> production deployments, so that we don't pollute the whole server for this 1
> little feature. (NOTE: We're already used to bundling things for installs,
> so this should be easy for the Koha community.)

I like this. It could easily be automated as part of the build process. Could we go one step further and use node's single executable applications feature?
Comment 8 David Cook 2024-10-17 22:31:05 UTC
(In reply to Kyle M Hall (khall) from comment #7)
> I like this. It could easily be automated as part of the build process.
> Could we go one step further and use node's single executable applications
> feature?

I hadn't heard of that specific feature myself, but I was wondering if we could build a portable executable more broadly, so it would be great to try it out!
Comment 9 David Cook 2024-10-17 22:35:30 UTC
It looks like the v18.19.0 node binary itself is 88MB alone, so the final size would be bigger than that. I don't know if we'd want to wedge that into our current koha-common package, as it could slow down deployments.

But it would be easy enough to put it in its own Debian package. And for non-package installs, it wouldn't matter since it would just be installed like usual. 

So yeah... sounds good to me :)
Comment 10 Julian Maurice 2024-10-18 09:11:29 UTC
Not against node in prod, but... are we really adding node, yarn and several npm dependencies to do the same thing as one single already existing Perl script (that does not require additional dependencies) ?
What's the point ?

I read that the Perl script was unmaintained. We could maintain it, it is/was already in our code base. I don't see how the chosen solution here is better, easier and/or less work than that.

The "we will need node in prod at some point anyway" argument is not a convincing one to me.

If we do need node at some point, then fine. But for PO to JSON conversion it is not needed.

And I believe that the fact that bug 37303 was pushed without considering this is another sign that it should be reverted from main now. Big changes like this (adding node as prod dependency) should not be made in a hurry (and 24.11 will be released next month)
Comment 11 Katrin Fischer 2024-10-18 11:02:15 UTC
I think Julian has a valid point as we quickly run out of time that might be better spent elsewhere for this cycle. Could we push to fix this as a community project early next cycle maybe?

Is there another use case than the translation scripts currently to include node/yarn etc.?
Comment 12 Jonathan Druart 2024-10-18 12:35:57 UTC
I feel like I need to clarify my position.

On bug 37303 we found a quite bad bug impacting our translation system. It came from a script we embedded into our codebase that is no longer maintain upstream. And the project actually moved to a JS version. When I gave it a try, the issue was fixed and we didn't notice any other changes. It made sense to switch.
I didn't anticipate that any problems will arise.

No, we do not have time to dedicate to this for now. Yes, the single line patch (from Julian, bug 38164) is the way to go for 24.11.

However it is good to know that we are not ready for node in production, and it would be good to spend time on it. At least to know if it's a solution we could deploy in case of a future emergency. For now there is no immediate need, but... who knows when it will come again?
Comment 13 Katrin Fischer 2024-10-18 12:38:40 UTC
Hi Joubu and all,

thanks for the clarification. I am going to push the solution from bug 38164 in this case and we will leave this open for the 25.05 cycle. 

Maybe we could already add the rel_25_05_candidate keyword to keep this in a more prominent spot?