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

feat: node 20.14.0 #37

Merged
merged 23 commits into from
Aug 28, 2024
Merged

feat: node 20.14.0 #37

merged 23 commits into from
Aug 28, 2024

Conversation

robertsLando
Copy link
Member

@robertsLando robertsLando commented Jun 13, 2024

Seems that latest nodejs 18/20 builds were failing because of old GLIBC versions. I have upgraded both Dockerfile.linux and Dockerfile.linuxcross in order to use gcc 10, this allowed also to remove a workaround added on patches to disable _SYS_RANDOM_H and msign-return-address=all cflag

@robertsLando robertsLando changed the title Node20.14.0 feat: node 20.14.0 Jun 13, 2024
@robertsLando
Copy link
Member Author

@Shogan @sean-stage @Rob3rtS could someone of you give a try to this please?

@robertsLando
Copy link
Member Author

robertsLando commented Jun 13, 2024

@robertsLando
Copy link
Member Author

@axi92 working on this

@robertsLando

This comment was marked as resolved.

@matthias-heller
Copy link

I just tried the patch: The issue is the same as in

#32

node:internal/errors:541
throw error;
^

TypeError [ERR_INVALID_ARG_TYPE]: The "paths[0]" argument must be of type string. Received undefined
at Object.resolve (node:path:171:9)
at resolveMainPath (node:internal/modules/run_main:35:38)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:160:24)
at node:internal/main/run_main_module:28:49 {
code: 'ERR_INVALID_ARG_TYPE'
}

Node.js v20.14.0

When I run the .exe with e.g. myTest.exe app.js then the exe executes the given app.js instead of the internal embedded code when starting myTest.exe only I get the above error

@robertsLando
Copy link
Member Author

@matthias-heller Same for the 18.20.2? Could you test that too please?

@robertsLando
Copy link
Member Author

I have a feel that's something happening only with windows exec as I'm not able to reproduce it with linux

@robertsLando
Copy link
Member Author

@Shogan I know you also got that issue, did you tested the win exe too?

@matthias-heller
Copy link

I have the issue with linux also. 18.20.2 I can't test as the effort in our environment would be quite big

@robertsLando
Copy link
Member Author

@matthias-heller Are you able to provide a minimal script that once packaged reproduces the issue? Because I'm not able to reproduce this

@robertsLando
Copy link
Member Author

I opened the exe file in a binary editor.

Did you opened the builded exe or the patched nodejs exe?

@matthias-heller
Copy link

matthias-heller commented Jul 25, 2024

@robertsLando: I opened the built exe file which I also started

@robertsLando
Copy link
Member Author

@matthias-heller the last thing to test IMO is to try building an exe with a previous nodejs version to see what you see inside it and compare. All this things are very strange

@matthias-heller
Copy link

matthias-heller commented Jul 25, 2024

@robertsLando:
I did the following:
I downloaded the prebuilt binary for node-20.11.1 renamed it to built-20.14.0-win-x64 and run a pkg bundeling.
The final executable is working good.

The stuff which I can compare inside the binary looks same. The byte code content looks totally different. I used the same version of pkg which is 5.12.0

I could see inside the final binary built with node.js 20.14.0 still entries which are the place holders. This is not happening with node.js 20.11.1

screenshot

@robertsLando
Copy link
Member Author

The byte code content looks totally different

This is expected:

Bytecode (reproducibility)

By default, your source code is precompiled to v8 bytecode before being written to the output file. To disable this feature, pass --no-bytecode to pkg.

Why would you want to do this?

If you need a reproducible build process where your executable hashes (e.g. md5, sha1, sha256, etc.) are the same value between builds. Because compiling bytecode is not deterministic (see here or here) it results in executables with differing hashed values. Disabling bytecode compilation allows a given input to always have the same output.

You could try to build it using --no-bytecode so at least you will see the code in clear

@robertsLando
Copy link
Member Author

robertsLando commented Jul 25, 2024

I could see inside the final binary built with node.js 20.14.0 still entries which are the place holders. This is not happening with node.js 20.11.1

Ok I think this is the most important thing now. Could you try to manually replace those entries values on the final binary with the same values you got in the working one?

@matthias-heller
Copy link

matthias-heller commented Jul 25, 2024

@robertsLando: I did, but changing a binary typically won't work, and this is the same situation here. So I tried and the executable crashes down.

@CodeRunsTheWorld
Copy link

CodeRunsTheWorld commented Jul 25, 2024 via email

@matthias-heller
Copy link

matthias-heller commented Jul 25, 2024

Yes I did. But this is inside the bytecode section I can't quarantee that the stuff before the string which is bytecode is still okay. But even without bytecode it crashes as then I will change the filesize anyway.
Also I think doing is would just show that it works, but the root cause has to be found and solved.

So at least we know what the problem is. The four variables which should be filled while building the binary evaluate to 0 and it is not a matter of '// PRELUDE_SIZE //' | 0 but realle PRELUDE_SIZE and all the other variables are missing

@robertsLando
Copy link
Member Author

robertsLando commented Jul 25, 2024

I confirm that also on linux build that PRELUDE_SIZE is not present, this means that for some reason the injectPlaceholder fails

https://github.com/yao-pkg/pkg/blob/ba407efef1d2e611d4c192b93295dcaa023fea79/lib/producer.ts#L51

If so it meanst the inject should throw here:

https://github.com/yao-pkg/pkg/blob/ba407efef1d2e611d4c192b93295dcaa023fea79/lib/producer.ts#L62

@matthias-heller
Copy link

@robertsLando: Thank you for validating under linux. Are the other three place holders missing as well in linux?

@robertsLando
Copy link
Member Author

@robertsLando: Thank you for validating under linux. Are the other three place holders missing as well in linux?

Yep

@matthias-heller
Copy link

Okay great at least we found the problem. So my roughly two days of investigating in my freetime was not waste. Now we have to solve it

@robertsLando
Copy link
Member Author

Yeah and thanks for that 🙏🏼 I wasted the same time making the other archs working 😆 Maintaining this repo is just a pain but unfortunately there is no real alternative right now as node SEA doesn't suite well for complex applications (yet).

Anyway even if we discovered that I stll have no clue why that happens only on windows :(

@matthias-heller
Copy link

Yeah and thanks for that 🙏🏼 I wasted the same time making the other archs working 😆 Maintaining this repo is just a pain but unfortunately there is no real alternative right now as node SEA doesn't suite well for complex applications (yet).

Anyway even if we discovered that I stll have no clue why that happens only on windows :(

Yeah, but I thought under linux the place holders are also not filled, so maybe you can have a look why this happens and when it is somehow solved, I can try it under windows as well. If I could offer any help let me know.

@marcus-j-davies
Copy link

marcus-j-davies commented Jul 25, 2024

Just a stab in the dark here (and I am more then aware I could be looking like a twit/cowboy)

LIne endings.... between *nix and windows, could that be messing with buffer lengths somehow, maybe in 20.14 they treated this differently ?

given the difference in length between \n and \r\n

could it be pkg interprets these differently? - at least something in 20.14 causing it todo so

@matthias-heller
Copy link

Any news on this topic?

@robertsLando
Copy link
Member Author

@matthias-heller Unfortunately nope, I was thinking about merging this and do a new release then wait for people asking why windows isn't working to ask them help 😆 I know it's not cool but I'm running out of ideas and time is limited

@robertsLando robertsLando merged commit f35e749 into main Aug 28, 2024
18 checks passed
@robertsLando robertsLando deleted the node20.14.0 branch August 28, 2024 14:04
@robertsLando
Copy link
Member Author

Doing a new release now. Will be ready tomorrow

https://github.com/yao-pkg/pkg-fetch/actions/runs/10598344967

@robertsLando
Copy link
Member Author

There will be a little of delay for the release as I found out that macos-11 runner has been dropped last month so I'm testing new runners here: #40

@robertsLando
Copy link
Member Author

Thanks to @faulpeltz the windows issue has been fixed: yao-pkg/pkg#86 🎉

Now let's try to fix macos one

@robertsLando
Copy link
Member Author

Just a quick update here. I finally managed to make all build working and new release is coming with latest nodejs 18 and 20.17.0 support 🎉

Just need to wait for this to finish now

Please consider sending some support here: https://github.com/sponsors/robertsLando 🙏🏼 ❤️

@robertsLando
Copy link
Member Author

pkg 5.13.0 is ou now with nodejs 20.17.0 and 18.20.4 support 🎉

@nulano
Copy link

nulano commented Oct 24, 2024

Can you please update the README to show that the built binaries now require glibc 2.28 (i.e. RHEL 8) instead of glibc 2.17 (i.e. RHEL 7)?

@robertsLando
Copy link
Member Author

@nulano PR?

@nulano
Copy link

nulano commented Oct 25, 2024

@nulano PR?

Sure, see #58

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 this pull request may close these issues.

5 participants