Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

Merge some fixes found in forks #1093

Closed
robertsLando opened this issue Mar 25, 2021 · 16 comments
Closed

Merge some fixes found in forks #1093

robertsLando opened this issue Mar 25, 2021 · 16 comments

Comments

@robertsLando
Copy link
Contributor

robertsLando commented Mar 25, 2021

I would merge this fixes I found in forks:

Also I would like to ask @leafac if he is interested helping improve this project.

He is creating caxa that is another nodejs tool to compile applications. I think his approach could be considered here too and will allow use to include native .node modules inside the executable. I think that if we join our forces good things could happen :)

@robertsLando
Copy link
Contributor Author

cc @leerob @jesec @hipstersmoothie

@robertsLando
Copy link
Contributor Author

robertsLando commented Mar 25, 2021

@leafac
Copy link

leafac commented Mar 25, 2021

I think the forks you mentioned have already been merged.

I’m in for a collaboration between pkg and caxa.

@robertsLando
Copy link
Contributor Author

robertsLando commented Mar 25, 2021

Nope at least I don't see the promises fix in master. Could you make a PR for that specific fix so we can split the #1095 in multiple ones?

@leerob
Copy link
Member

leerob commented Mar 25, 2021

Works for me!

@robertsLando
Copy link
Contributor Author

I’m in for a collaboration between pkg and caxa.

What you mean? :) How could we contribute together?

@jesec
Copy link
Contributor

jesec commented Mar 25, 2021

caxa uses a very different method. I think we should keep our own method for the time being. It is well tested in production and trusted by our users.

IMO we should be cautious about the other changes. They may be breaking and can erode confidence in the project if merged without being thoroughly tested, and they probably will not bring us significant immediate benefits. pkg is around for a long time, and it is safe to say that most cases are already handled.

My top priority at the moment is to figure out a CI setup and ship new Node binaries. There are multiple critical security updates, and the associated security vulnerabilities are being exploited in the wild.

@hipstersmoothie
Copy link
Contributor

My top priority at the moment is to figure out a CI setup and ship new Node binaries.

Yeah I think this should be top priority for pkg. New features should be at a minimum, focus should mainly be on:

  • security updates
  • node updates
  • maturing CI so we can be more confident in the above updates

@leerob
Copy link
Member

leerob commented Mar 25, 2021

Agreed - there's some tech debt to fix first 😄

@robertsLando
Copy link
Contributor Author

I agree too @jesec The fork I found are mainly fixes not features. What I would consider btw is #1096 as it sort out the main issue with pkg: .node files

@hipstersmoothie
Copy link
Contributor

Yeah going over the issue I saw many issues that related to .node files

@jesec
Copy link
Contributor

jesec commented Mar 25, 2021

Well. The distinction between additions and fixes is not always clear. pkg does not support some use cases before, and that more or less becomes the “expected behavior” over time. So those new changes can be effectively “feature additions” instead of “fixes”.

I am AFK right now due to some “real life” things.

Feel free to work on CI workflows for pkg-fetch. Check my pkg-fetch repo for some ideas.

I can review, test and improve the workflows next week. Before that, while I can comment on the changes when I have time, I am not able to make changes.

@leafac
Copy link

leafac commented Mar 25, 2021

Nope at least I don't see the promises fix in master. Could you make a PR for that specific fix so we can split the #1095 in multiple ones?

Weird: #455 (comment) / #958 (comment)

What you mean? :) How could we contribute together?

caxa does some things better than pkg: Support all Node.js versions, not having to patch the Node.js build, support native extensions in a more natural way, not having to specify which assets to include in the binary, smaller binary sizes, and so forth.

pkg does some things better than caxa: Protect the source code by using a V8 snapshot, faster build times, and so forth.

It’d be nice if we could come together and build a tool that was the best of both worlds.

@btsimonh
Copy link

'Support all Node.js versions' - why? - surely most people have issue with it not supporting the latest LTS +? (for ages, I was stuck on 6.9.11, a great version, but updating to 12 was complex in my (private) nexe branch. 14 is next, but not looking forward to it).

'Protect the source code by using a V8 snapshot' - using the snapshot to include the protected code really screws osx distribution (and windows signing?), and is not protected.... The 'extra code' must be outside of the main exe - but for OSX this is exceedingly difficult (yes, there are solutions, but I can't share :(, suffice to say in our application EVERY application must be different, and this is basically officially impossible when talking OSX - and in Windows Signed, you need to be creative.).

Ref native modules, again, be creative. The biggest issue is not the native modules - that is easy. It's the dependencies of those modules..... in Windows, they can all go alongside, no problem. in OSX and Linux, a right pain, but not impossible (sorry, again, proprietary, and difficult for every module - probably more than pkg user could take).

I'm (modified) nexe base, and have been observing pkg for some time. The native is the most difficult part (for some things easy, but try Canvas, Tfjs, Opencv). I think you are heading the the right direction.... keep up the good work!

@robertsLando
Copy link
Contributor Author

Weird: #455 (comment) / #958 (comment)

@leafac Seems that the native addons one has been merged, the promises fix was not

It’d be nice if we could come together and build a tool that was the best of both worlds.

Personally I'm +1 for any kind of collaboration, I think that join forces is more productive then have multiple different projects, btw we should define a roadmap and decide what changes need to be done

@JeffJassky
Copy link

JeffJassky commented Mar 26, 2021

@btsimonh Regarding OSX code signing/notarizing: I know there are some issues currently. I don't know many details but I have a solution that actually works well for my distribution needs.

My distribution

My main app (electron) is signed and notarized. It's a dumb front-end terminal with little business logic. Once the user opens it, it fetches my pkg executable from an S3 server. Once fetched, my main app spawns the executable as a subprocess - which in my case, is my code-protected API business logic.

Why does it work?

Executables that are spawned by a signed/notarized apps aren't subject to the same security scrutiny. I don't know if this is intended behavior for macOS or if this is an oversight. I also know it's not a solution for every distribution need - but I thought I'd share in case it works for some, or spawns some ideas.

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

No branches or pull requests

7 participants