feat : upgrades pkg version to sign binary

botpress / nlu

This repo contains every ML/NLU related code written by Botpress in the NodeJS environment. This includes the Botpress Standalone NLU Server.

23 stars 21 forks source link

feat : upgrades pkg version to sign binary #177

Closed ptrckbp closed 2 years ago

ptrckbp commented 2 years ago

This issue https://github.com/vercel/pkg/commit/0b55f9adba1f52d2aed171ca902d44fffa114af0 is preventing me from including the binary inside the electron app.

I tried packaging, and placing the nlu binary within the master branch of botpress/botpress and it works flawlessly, but that's just one environment (Intel Macos). Please let me know if you need me to test this somehow.

franklevasseur commented 2 years ago

Hi @ptrckbp

As you have probably guessed, even if this change is small, it is heavy with consequences.

I personally had a lot of trouble with this version of pkg (5.5.x) on the cloud branch. This issue thread bears witness to it. For the same reason, I believe it's safer to use the tilde rather than the caret to avoid a minor release that breaks the product.

When changing pkg, not only do we need to try multiple platforms, but it is also important to try the following actions:

training
predicting
start a lang server and use tokenize / vectorize routes

Would pkg version ~5.2.x work for your electron app ?

ptrckbp commented 2 years ago

I was afraid of this kind of issue. It's going to be time consuming to make sure it works everywhere.

I have cheaper alternatives solution paths in front of me (no-bytecode releases versions, and curling directly from Electron) that i'll try first to see if I can circumvent the issue.

Thanks

franklevasseur commented 2 years ago

@ptrckbp

Alright, I'll close your PR for now. Let's just reopen it if we change our mind.

I just want to reiterate that the pkg version ~5.2.x works and I've been testing it for a while now. If it works for you, we can just use this one instead.