victronenergy / venus

Victron Energy Unix/Linux OS
https://github.com/victronenergy/venus/wiki
568 stars 70 forks source link

signalk-server or npm: shrinkwrap doesn't work #1129

Closed mpvader closed 1 year ago

mpvader commented 1 year ago

And as a result, our builds are not reproducible.

I thought I had seen this before, but then didn't look into it, but now know it for sure: the shrinkwraps we use to lock down all versions of dependencies used when building signalk-server (and I expect other similar recipes) doesn't work.

What happened this time is that a newer version of canboatjs, which is an indirect dependency of signalk-server, was released three days ago, which changes the version of the serialport package it uses, see the second commit from the top here:

https://github.com/canboat/canboatjs/commits/v1.28.3

And apparently from 9.. to 11.., the serialport package includes prebuilt binaries, since OE QA gave this:

ERROR: mc:beaglebone:signalk-server-1.46.3-1 do_package_qa: QA Issue: Architecture did not match (x86-64, expected ARM) on /work/beaglebone-ve-linux-gnueabi/signalk-server/1.46.3-1/packages-split/signalk-server/usr/lib/node_modules/signalk-server/node_modules/@serialport/bindings-cpp/prebuilds/linux-x64/node.napi.musl.node
Architecture did not match (x86-64, expected ARM) on /work/beaglebone-ve-linux-gnueabi/signalk-server/1.46.3-1/packages-split/signalk-server/usr/lib/node_modules/signalk-server/node_modules/@serialport/bindings-cpp/prebuilds/linux-x64/node.napi.glibc.node
Architecture did not match (AArch64, expected ARM) on /work/beaglebone-ve-linux-gnueabi/signalk-server/1.46.3-1/packages-split/signalk-server/usr/lib/node_modules/signalk-server/node_modules/@serialport/bindings-cpp/prebuilds/linux-arm64/node.napi.armv8.node
Architecture did not match (AArch64, expected ARM) on /work/beaglebone-ve-linux-gnueabi/signalk-server/1.46.3-1/packages-split/signalk-server/usr/lib/node_modules/signalk-server/node_modules/@serialport/bindings-cpp/prebuilds/android-arm64/node.napi.armv8.node [arch]

I've fixed that with a quick hotfix, deleting all prebuilds except for linux-arm, but the real issue, which is that our builds are now not reproducible, is still open and needs fixing.

The versions that are supposed to be used when building the current version of signalk-server all defined in here:

https://github.com/victronenergy/meta-victronenergy/blob/master/meta-third-party/recipes-extended/signalk-server/signalk-server/npm-shrinkwrap.json

And thats canboatjs 1.28.1, and serialport 9.2.8.

mpvader commented 1 year ago

I tested jhofstee his change, works fine wrt making npm stick to the shrinkwrap file. To validate that, I looked at the canboatjs dependency of signalk-server. With the old version, v1.8.3 was used. OK according the package.json but not what the shrinkwrap file mandates.

And with the change in place it installs v1.8.1. As it should.

Also tested signalk-server functionality: it works fine with the changes.

node-red-contrib-victron recipe needed a change due to an updated location. Note that I don't know if that location change in itself is an issue. @dirkjanfaber will be able to tell.

And node-red has some other issue, with files no longer being installed to /usr/bin; which actually breaks it, so that needs looking into. This commit fixes the build issue, but results in an image without the node-red binary being in /usr/bin:

@4000000064dea3103259752c *** starting node-red-venus ***
@4000000064dea31033cacbf4 *** Waiting for localsettings...
@4000000064dea31034f6307c *** Starting in normal mode
@4000000064dea31037169b94 /usr/bin/node-red-venus.sh: line 118: /usr/bin/node-red: No such file or directory
jhofstee commented 1 year ago

Yes, I could have mentioned that, since I am aware I don't handle the the bin case, simply because signalk doesn't seem to have them and building a complete large image takes a long long time on my laptop since linking nodejs will trigger the OEM killer in a parallel build. Anyway, hence it lives in a branch for now, but obviously we should fix this in the near future.

dirkjanfaber commented 1 year ago

So far I haven't seen any issues with this, but I cannot rule out that I am overlooking something. On my build (including the jhofstee/npm-shrinkwrap branch), I still see a symbolic link for /usr/bin/node-red.

root@einstein:~# ls -al /usr/bin/node-red
lrwxrwxrwx    1 root     root            35 Aug 21 12:09 /usr/bin/node-red -> ../lib/node_modules/node-red/red.js
root@einstein:~#

AFAIK the node-red-pi was never used.

mpvader commented 1 year ago

Indeed we don’t use node-red-pi.

as discussed on the phone: this ha not the highest prio, and be aware that Jeroen his new version of npm-install does not put anything in /usr/bin.

(close-read his code when going to work on this)

mpvader commented 1 year ago

Nice!