-
Notifications
You must be signed in to change notification settings - Fork 14
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
Internally use objcopy --add-section
on macOS and Linux
#63
Comments
Makes total sense to me! |
+1 👏 |
Another problem I can think of is that, it won't let you embed your source code into a an executable format that's not used on that platform, like you won't be able to embed data into a Mach-O file on Windows. Pkg is currently able to produce ELF, Mach-O as well as PE executables regardless of the host platform. @jesec, is that a hard requirement of pkg? |
That is a good point indeed, and worth checking what Node.js folks think too. |
That feels like would become a dependency of And feels here like |
I didn't mention it during the meeting but it seems like the cross-platform binary creation might actually not even work if your project has native bindings, so maybe this still makes sense? To @ovflowd's point, maybe we can find a way to ship just |
Fixes: #63 Signed-off-by: Darshan Sen <[email protected]>
PR - #64 |
I'm a fairly strong -1 on this idea for several reasons. This is going to be a somewhat lengthy comment, so before getting further into it, I want to point out that That said, first and foremost, I don't think it works. When exploring ways to accomplish the injection (before the Postject project existed) I evaluated I have not explored Beyond not being suitable for the purpose (IMO), I'd like to address the advantages/disadvantages discussed here.
Is this referring to the code using LIEF in Postject, or upstream LIEF? I'd argue that relying on It's also just weird for I don't think bundling Now, it is potentially interesting to consider building
There are improvements that can be made here at the WASM level, that I haven't opened a PR for yet. That said, I don't think the speed of the injection is a major concern, and shouldn't be a major motivator for a significant implementation change like this.
See opening points, but for consistency we do not want to mix
I think this is a major drawback. Being platform-agnostic for the injection is a major benefit and makes Postject easy to get started with immediately for testing. I'd want strong reasons to abandon this benefit, especially after we did the work to use WASM and make Postject completely platform-agnostic (heck, you could provide a website that does the injection in-browser and not even require the user to install anything).
That sounds like a bug - cross-compiling is the standard solution there and is supported by tools like I'm in no way opposed to replacing LIEF or the underlying implementation - that's one of the main benefits to building to WASM and being entirely self-contained. But I don't think this is the right direction and I don't really see what is gained. |
I was referring to maintaining a pure JS implementation of a resource injector for all binary formats.
nodejs/node-gyp#829 tells me that it is not supported. 🤔 Anyways, excellent points, thanks! I'll close this and instead work on the pure JS resource injector. |
Good catch, I was thinking cross-compiling, not cross-platform. That indeed might not be possible. |
objcopy --add-section
works on Linux - https://man7.org/linux/man-pages/man1/objcopy.1.html.After https://reviews.llvm.org/D66283 landed,
llvm-objcopy
on macOS too started supporting this feature: https://llvm.org/docs/CommandGuide/llvm-objcopy.html#cmdoption-llvm-objcopy-add-sectionThis demo is using the Node.js binary that has been built from nodejs/node#45038 on macOS:
Advantages:
Disadvantage:
objcopy
is present by default on Linux but users would have to download it manually on macOS along with other tools usingbrew install llvm
.@nodejs/single-executable wdyt?
@bnoordhuis I'd also like to hear your thoughts on this since you were sharing some insights on the resource injection part in nodejs/node#45066.
The text was updated successfully, but these errors were encountered: