Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
danielpeintner opened this issue May 9, 2024 · 12 comments · Fixed by #125

Comments

@danielpeintner
Copy link

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 eclipse-thingweb/node-wot#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:

'./libmbus/mbus/mbus-serial.c',

@Apollon77
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

danielpeintner commented May 9, 2024

I am not sure either but I found nodejs/nan#968 which contains stuff like the following also

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

Maybe nodejs/node#52794 is helping also? Not sure if it is used at all?

@Apollon77
Copy link
Owner

Ohhh this finding sound interesting ... will try

@Apollon77 Apollon77 mentioned this issue May 9, 2024
@Apollon77
Copy link
Owner

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
Copy link
Author

@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
Copy link
Author

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
8331f88

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

@Apollon77
Copy link
Owner

@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
Copy link
Owner

Release done

@danielpeintner
Copy link
Author

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 eclipse-thingweb/node-wot#1274 (comment)

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
Copy link
Owner

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
Copy link
Author

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:

  • npm ci
  • npm run build
  • npm run test -w packages/binding-mbus/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants