openhab / openhabian

openHABian - empowering the smart home, for Raspberry Pi and Debian systems
https://community.openhab.org/t/13379
ISC License
821 stars 252 forks source link

Install npm to fix frontail install #1598

Closed mstormi closed 2 years ago

mstormi commented 2 years ago

Fixes: #1597

Signed-off-by: Markus Storm markus.storm@gmx.net

mstormi commented 2 years ago

Not sure why this broke all of the sudden but I would rather just install npm at the same time as nodejs so why dot you move this to lines 47 & 49.

See original code, it wasn't executed when either NodeJS was already installed (which it was in Raspi OS) OR when it was NOT a RPi0. So effectively it never was. I don't wanna break that logic by moving this down.

if node_is_installed || ! is_armv6l; then return 0; fi

ecdye commented 2 years ago

Not sure why this broke all of the sudden but I would rather just install npm at the same time as nodejs so why dot you move this to lines 47 & 49.

See original code, it wasn't executed when either NodeJS was already installed (which it was in Raspi OS) OR when it was NOT a RPi0. So effectively it never was. I don't wanna break that logic by moving this down.

if node_is_installed || ! is_armv6l; then return 0; fi

But this new code has the same problem does it not? If NodeJS is installed it won't execute and it nodeJS is not installed it and we are not on a RPi0 we will install npm before we install NodeJS, right? That is what I don't want if anything we should install both at the same time.

mstormi commented 2 years ago

Yes it has the same behavior. But that has not been a problem so far so why do you call it a problem? And then why change? And I don't get why you insist on installing both at the same time. Why isn't the current proposal code (doing npm first nodejs later) fine ?

I don't mind changing but I think I didn't get your point so as you can edit, I suggest you do to suit your idea.

ecdye commented 2 years ago

Yes it has the same behavior. But that has not been a problem so far so why do you call it a problem? And then why change? And I don't get why you insist on installing both at the same time. Why isn't the current proposal code (doing npm first nodejs later) fine ?

I don't mind changing but I think I didn't get your point so as you can edit, I suggest you do to suit your idea.

I just think that it is much cleaner and more effective to install them both at the same time rather than doing one then the other. It also reduces the amount of code overall.

mstormi commented 2 years ago

Please edit at will and merge. It's very late over here and I need to get some sleep but it's an ongoing problem so please don't wait for me.

mstormi commented 2 years ago

did you test it works ? nodejs_is_installed at the beginning checks for node OR npm so if node is there but npm is not it still won't install, will it? That's why I took the apparently longer route I did.

ecdye commented 2 years ago

Yes it appears to work for me, but I can change it to ensure that if one or the other is not there to just install both again.

mstormi commented 2 years ago

I'm still unsure we talk about the right point so again, to be explicit about the important point: did you test with a fresh image? if at the early stage in automated installation where we have nothing but Raspi OS, the first line in nodejs_setup() calls node_is_installed() and returns (i.e. skips installation !) whenever node_is_installed() returns true which is when EITHER node OR npm is installed. So if node is there but npm is not, it won't get installed, leading to the frontail error later. Or am I wrong here ?

(and note Raspi OS can also change at any time in the future, too, to have either of these packages installed or not, and I want a permanent, future proof solution to work independent of what's in the image or not)

ecdye commented 2 years ago

Yes I did test and it worked on a fresh image. I'm going to make the change so that both npm and nodeJS must be installed to let the check pass. if one or the other is missing it will reinstall once I make the change.

WolfgangSn commented 2 years ago

Is that "apt-get install --yes nodejs npm" correct/ok ? As far as I see with npm is part / included in nodejs package from deb.nodesource.com so npm is not required right ? Besides that it looks like installing nodejs from deb.nodesource and npm from Rasbian OS ( openhabian ) introduces not resolvable dependencies: apt -s install nodejs npm The following packages have unmet dependencies: nodejs : Conflicts: npm but 5.8.0+ds6-4+deb10u2 is to be installed libnode64 : Conflicts: nodejs-legacy which is a virtual package, provided by:

The following actions will resolve these dependencies:

 Keep the following packages at their current version:

1) libnode-dev [Not Installed]
2) libnode64 [Not Installed]
3) node-gyp [Not Installed]
4) npm [Not Installed]

ecdye commented 2 years ago

OK so really what needs to happen is when npm or nodejs get mixed up and are not both installed just reinstall nodejs let me issue the fix.

ecdye commented 2 years ago

@WolfgangSn try again now.