| Message ID | 20220322160725.636205-1-johan.oudinet@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | package/forge: new package | expand |
Johan, All, Sorry for the late review... On 2022-03-22 17:07 +0100, Johan Oudinet spake thusly: > A native implementation of TLS (and various other cryptographic tools) > in JavaScript. I see that you used the npm registry as the source for this package. So, I'd expect it to be an nodejs package, right? If so, then it should probably depend on nodejs being enabled. Also, why do we need to get the package from the npm registry, instead of from the upstream git tree? > Signed-off-by: Johan Oudinet <johan.oudinet@gmail.com> [--SNIP--] > diff --git a/package/forge/Config.in b/package/forge/Config.in > new file mode 100644 > index 0000000000..86d4832101 > --- /dev/null > +++ b/package/forge/Config.in > @@ -0,0 +1,7 @@ > +config BR2_PACKAGE_FORGE > + bool "forge" Here, I would have expected: depends on BR2_PACKAGE_NODEJS > + help > + A native implementation of TLS (and various other > + cryptographic tools) in JavaScript. > + > + https://github.com/digitalbazaar/forge [--SNIP--] > diff --git a/package/forge/forge.mk b/package/forge/forge.mk > new file mode 100644 > index 0000000000..1872cb4e70 > --- /dev/null > +++ b/package/forge/forge.mk > @@ -0,0 +1,23 @@ > +################################################################################ > +# > +# forge > +# > +################################################################################ > + > +FORGE_VERSION = 1.3.0 > +FORGE_SOURCE = node-forge-$(FORGE_VERSION).tgz > +FORGE_SITE = https://registry.npmjs.org/node-forge/- > +FORGE_LICENSE = BSD-3-Clause, GPL-2.0 > +FORGE_LICENSE_FILES = LICENSE > + > +# Install .min.js as .js > +define FORGE_INSTALL_TARGET_CMDS > + $(INSTALL) -m 644 -D $(@D)/dist/forge.all.min.js \ > + $(TARGET_DIR)/var/www/forge.all.js > + $(INSTALL) -m 644 -D $(@D)/dist/forge.min.js \ > + $(TARGET_DIR)/var/www/forge.js > + $(INSTALL) -m 644 -D $(@D)/dist/prime.worker.min.js \ > + $(TARGET_DIR)/var/www/prime.worker.js > +endef So, I think I now see why you grabbed the package from the npm registry: the package has been built and the minified files generated. I'm afraid that I do not like that very much. Instead, you should grab the sources from th github repo, have the package depend on host-nodejes, and build the pacakge with $(NPM) build. Regards, Yann E. MORIN. > +$(eval $(generic-package)) > -- > 2.32.0 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Hi Yann, All, On Mon, Aug 22, 2022 at 4:12 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > I see that you used the npm registry as the source for this package. So, > I'd expect it to be an nodejs package, right? If so, then it should > probably depend on nodejs being enabled. > > Here, I would have expected: > > depends on BR2_PACKAGE_NODEJS No, it is not a nodejs package. It also works on the browser, so it does not depend on having nodejs in the target. > > So, I think I now see why you grabbed the package from the npm registry: > the package has been built and the minified files generated. > > I'm afraid that I do not like that very much. > > Instead, you should grab the sources from th github repo, have the > package depend on host-nodejes, and build the pacakge with $(NPM) build. Thanks for the suggestion. I didn't know we could do that in recent versions of buildroot. It is indeed a much cleaner way to handle npm packages, even though I don't like the BUILD commands to download the dependencies. I've uploaded a new version with your suggestions. Best regards,
Johan, All, On 2022-08-26 16:11 +0200, Johan Oudinet spake thusly: > On Mon, Aug 22, 2022 at 4:12 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > Instead, you should grab the sources from th github repo, have the > > package depend on host-nodejes, and build the pacakge with $(NPM) build. > > Thanks for the suggestion. I didn't know we could do that in recent > versions of buildroot. > It is indeed a much cleaner way to handle npm packages, even though I > don't like the BUILD commands to download the dependencies. Yeah, this is indeed not nice, but there is nt much we can do for now. If we get a good nderstanding of how npm-based pacages work, then we can look into introducing a download post-processing script, like we have for go and cargo, where the vendoring is done as part of the download step. But I don't think I could spot a similar (mis)feature in npm... Regards, Yann E. MORIN. > I've uploaded a new version with your suggestions. > > Best regards, > -- > Johan > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
diff --git a/DEVELOPERS b/DEVELOPERS index 942bb8fe9c..5830b45018 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -1475,6 +1475,7 @@ F: package/erlang-p1-xmpp/ F: package/erlang-p1-yaml/ F: package/erlang-p1-yconf/ F: package/erlang-p1-zlib/ +F: package/forge/ F: package/nginx-dav-ext/ F: package/vuejs/ diff --git a/package/Config.in b/package/Config.in index 0d5d763180..f2587b5c66 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1653,6 +1653,7 @@ endif source "package/duktape/Config.in" source "package/explorercanvas/Config.in" source "package/flot/Config.in" + source "package/forge/Config.in" source "package/jquery/Config.in" if BR2_PACKAGE_JQUERY menu "External jQuery plugins" diff --git a/package/forge/Config.in b/package/forge/Config.in new file mode 100644 index 0000000000..86d4832101 --- /dev/null +++ b/package/forge/Config.in @@ -0,0 +1,7 @@ +config BR2_PACKAGE_FORGE + bool "forge" + help + A native implementation of TLS (and various other + cryptographic tools) in JavaScript. + + https://github.com/digitalbazaar/forge diff --git a/package/forge/forge.hash b/package/forge/forge.hash new file mode 100644 index 0000000000..256ac5b451 --- /dev/null +++ b/package/forge/forge.hash @@ -0,0 +1,3 @@ +# Locally computed +sha256 97f0276c32b39411ad85c5762bf546ca281451eeaa93bdd383ff082e8e0181b4 node-forge-1.3.0.tgz +sha256 f63ff0e4e239244aa79280da2dd4811a0469e5e201caf5cbc0d97c3a1dff8e82 LICENSE diff --git a/package/forge/forge.mk b/package/forge/forge.mk new file mode 100644 index 0000000000..1872cb4e70 --- /dev/null +++ b/package/forge/forge.mk @@ -0,0 +1,23 @@ +################################################################################ +# +# forge +# +################################################################################ + +FORGE_VERSION = 1.3.0 +FORGE_SOURCE = node-forge-$(FORGE_VERSION).tgz +FORGE_SITE = https://registry.npmjs.org/node-forge/- +FORGE_LICENSE = BSD-3-Clause, GPL-2.0 +FORGE_LICENSE_FILES = LICENSE + +# Install .min.js as .js +define FORGE_INSTALL_TARGET_CMDS + $(INSTALL) -m 644 -D $(@D)/dist/forge.all.min.js \ + $(TARGET_DIR)/var/www/forge.all.js + $(INSTALL) -m 644 -D $(@D)/dist/forge.min.js \ + $(TARGET_DIR)/var/www/forge.js + $(INSTALL) -m 644 -D $(@D)/dist/prime.worker.min.js \ + $(TARGET_DIR)/var/www/prime.worker.js +endef + +$(eval $(generic-package))
A native implementation of TLS (and various other cryptographic tools) in JavaScript. Signed-off-by: Johan Oudinet <johan.oudinet@gmail.com> --- DEVELOPERS | 1 + package/Config.in | 1 + package/forge/Config.in | 7 +++++++ package/forge/forge.hash | 3 +++ package/forge/forge.mk | 23 +++++++++++++++++++++++ 5 files changed, 35 insertions(+) create mode 100644 package/forge/Config.in create mode 100644 package/forge/forge.hash create mode 100644 package/forge/forge.mk