Apollon77 / node-mbus

Nodejs mbus module
MIT License
20 stars 15 forks source link

Node.js 22 issue: mbus-serial.c(90,23): warning C4090: 'function': different 'const' qualifiers #124

Closed danielpeintner closed 2 months ago

danielpeintner commented 2 months ago

Describe the bug

In node-wot we tried to add Node.js 22 as CI dependency and the run fails with

npm error   mbus-serial.c
npm error D:\a\node-wot\node-wot\node_modules\node-mbus\libmbus\mbus\mbus-serial.c(90,23): warning C4090: 'function': different 'const' qualifiers [D:\a\node-wot\node-wot\node_modules\node-mbus\build\libmbus.vcxproj]

To Reproduce
see https://github.com/eclipse-thingweb/node-wot/pull/1274 https://github.com/eclipse-thingweb/node-wot/actions/runs/8878604576/job/24558179052?pr=1274#step:5:24

Expected behavior
npm ci or npm install should work.

Screenshots & Logfiles
https://github.com/eclipse-thingweb/node-wot/actions/runs/8878604576/job/24558179052?pr=1274#step:5:24

Versions:
Tested node-mbus 2.2.2.

The file /libmbus/mbus/mbus-serial.c mentioned in the error log seems to be referenced here: https://github.com/Apollon77/node-mbus/blob/17a3b5088aceb2280911fc60a7e36e3d20c79d29/binding.gyp#L42

Apollon77 commented 2 months ago

Hn... but a warning should not a failure reason. only "errors" are. So the reasons are more likely stuff like https://github.com/eclipse-thingweb/node-wot/actions/runs/8878604576/job/24558179052?pr=1274#step:5:38 or such which is weird because windows specific ... let me test adding nodejs 22 to the build here

Apollon77 commented 2 months ago

So, yes my test shows the same but honestly ... I have n f***ing clue what MSVC wants to tell me because under linux and macos it compiles successfully ...

danielpeintner commented 2 months ago

I am not sure either but I found https://github.com/nodejs/nan/issues/968 which contains stuff like the following also

include\node\v8-function-callback.h(408,62): message : error recovery skipped: '

Maybe https://github.com/nodejs/node/pull/52794 is helping also? Not sure if it is used at all?

Apollon77 commented 2 months ago

Ohhh this finding sound interesting ... will try

Apollon77 commented 2 months ago

Ok 2.24 with the fix is on the way. But this is a great example that Node.js 22 is not yet LTS and so normally I do not really care about it because from last years experiences such stuff like here happens too often in the first time of. a new Node.jhs version and spending hours like today to just find (with your support, thank you for that) out that it will be fixed in the next Node.js version is a pain ;-)

danielpeintner commented 2 months ago

@Apollon77 Thanks for resolving the problem. I didn't want to cause lots of headache and waste of time. With node-wot we do have users that switch to the next Node.js version also very soon, especially if it is a LTS version like Node.js 22. Anyhow, I agree with you that often it is not as stable as we want it to be.

danielpeintner commented 1 month ago

Hi @Apollon77 I don't want to bother you... I just wanted to let you know that version 2.24 hasn't been published yet.

The commit has been pushed https://github.com/Apollon77/node-mbus/commit/8331f882474fec3f25e3446430d99ee667ecf737

Anyhow, it is not available on NPM yet... Are you aware of that? Thanks!

Apollon77 commented 1 month ago

@danielpeintner

especially if it is a LTS version like Node.js 22

Then tell your users that a new Node.sjs 22 is not automatically LTS, it becomes LTS then usually in October of the release year ;-)

For the update ... uups ... then seems some windows fun errored and I missed that. restarted it ... lets see

Apollon77 commented 1 month ago

Release done

danielpeintner commented 1 month ago

I just wanted to let you know that I think there is still somewhere an issue with mbus and Node.js 22 on Windows. see https://github.com/eclipse-thingweb/node-wot/pull/1274#issuecomment-2154830290

Unfortunately I will not be able to look into it before the end of June. Hence, this is just a heads-up so far.

Apollon77 commented 1 month ago

Hm .. puuhh .. that seems like the test crashes completely because it do not even log at which test step it fails...strange. No idea. tests I have in mbus lib are all working. I woud need more details here, but also limited on time, maybe earliest next week

danielpeintner commented 1 month ago

To be honest I don't know either.. it seems to crash on the C level ... but not sure..

I woud need more details here, but also limited on time, maybe earliest next week

The tests that are failing are in https://github.com/eclipse-thingweb/node-wot/blob/master/packages/binding-mbus/test/mbus-connection-test.ts and are rather basic.

You should be able to test it by checking out node-wot or maybe better the PR 1274: