All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] npm: change install directory to upstream default
@ 2018-10-19 15:22 Olaf Mandel
  2018-10-19 16:53 ` Marek Vasut
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Olaf Mandel @ 2018-10-19 15:22 UTC (permalink / raw)
  To: openembedded-core; +Cc: marex, Olaf Mandel

The node binary searches for packages in a number of locations, the last
of which is $PREFIX/lib/node (here: /usr/lib/node) from the list of
GLOBAL_FOLDERS [1]. Change the installation directory for all packages
depending on npm.bbclass to that location. This removes the need to
define the NODE_PATH variable to the non-standard /usr/lib/node_modules
value.

While the Tips for Package Managers [2] discusses installing packages to
/usr/lib/node_modules/<name>/<version>, this has several drawbacks:

 * it does not work for the REPL as mentioned in the documentation
 * it also does not work for any code _not_ installed as a global
   package under /usr/lib/node_modules (e.g. /usr/share/foo.js will not
   find any packages below /usr/lib)
 * using the non-default location and then having to set NODE_PATH
   barely saves any time: there are only two file-system lookups (to the
   legacy $HOME/.node_modules and $HOME/.node_libraries) directories
   before the library would be found

And the suggestion was made in the context of deduping the node_modules
tree by installing all packages in a flat hierarchy and using symlinks
to the correct version of each dependency. This is not what OpenEmbedded
does, so none of those benefits (deduping, cleaner packages) are being
had by shifting the installation directory to /usr/lib/node_modules.

[1]: https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders
[2]: https://nodejs.org/api/modules.html#modules_addenda_package_manager_tips

Signed-off-by: Olaf Mandel <o.mandel@menlosystems.com>
---
 meta/classes/npm.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index c351ff0866..d5ff0c6d57 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -10,7 +10,7 @@ def node_pkgname(d):
 
 NPMPN ?= "${@node_pkgname(d)}"
 
-NPM_INSTALLDIR = "${D}${libdir}/node_modules/${NPMPN}"
+NPM_INSTALLDIR = "${D}${libdir}/node/${NPMPN}"
 
 # function maps arch names to npm arch names
 def npm_oe_arch_map(target_arch, d):
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] npm: change install directory to upstream default
  2018-10-19 15:22 [PATCH] npm: change install directory to upstream default Olaf Mandel
@ 2018-10-19 16:53 ` Marek Vasut
  2018-10-22  9:50   ` [PATCH v2] " Olaf Mandel
  2018-10-22 10:03 ` ✗ patchtest: failure for npm: change install directory to upstream default (rev2) Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2018-10-19 16:53 UTC (permalink / raw)
  To: openembedded-core
  Cc: Joshua Lock, Anders Darander, Paul Eggleton, Olaf Mandel

On 10/19/2018 05:22 PM, Olaf Mandel wrote:
> The node binary searches for packages in a number of locations, the last
> of which is $PREFIX/lib/node (here: /usr/lib/node) from the list of
> GLOBAL_FOLDERS [1]. Change the installation directory for all packages
> depending on npm.bbclass to that location. This removes the need to
> define the NODE_PATH variable to the non-standard /usr/lib/node_modules
> value.
> 
> While the Tips for Package Managers [2] discusses installing packages to
> /usr/lib/node_modules/<name>/<version>, this has several drawbacks:
> 
>  * it does not work for the REPL as mentioned in the documentation
>  * it also does not work for any code _not_ installed as a global
>    package under /usr/lib/node_modules (e.g. /usr/share/foo.js will not
>    find any packages below /usr/lib)
>  * using the non-default location and then having to set NODE_PATH
>    barely saves any time: there are only two file-system lookups (to the
>    legacy $HOME/.node_modules and $HOME/.node_libraries) directories
>    before the library would be found
> 
> And the suggestion was made in the context of deduping the node_modules
> tree by installing all packages in a flat hierarchy and using symlinks
> to the correct version of each dependency. This is not what OpenEmbedded
> does, so none of those benefits (deduping, cleaner packages) are being
> had by shifting the installation directory to /usr/lib/node_modules.
> 
> [1]: https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders
> [2]: https://nodejs.org/api/modules.html#modules_addenda_package_manager_tips
> 
> Signed-off-by: Olaf Mandel <o.mandel@menlosystems.com>
> ---
>  meta/classes/npm.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
> index c351ff0866..d5ff0c6d57 100644
> --- a/meta/classes/npm.bbclass
> +++ b/meta/classes/npm.bbclass
> @@ -10,7 +10,7 @@ def node_pkgname(d):
>  
>  NPMPN ?= "${@node_pkgname(d)}"
>  
> -NPM_INSTALLDIR = "${D}${libdir}/node_modules/${NPMPN}"
> +NPM_INSTALLDIR = "${D}${libdir}/node/${NPMPN}"
>  
>  # function maps arch names to npm arch names
>  def npm_oe_arch_map(target_arch, d):
> 

Expanding the CC a bit ...

-- 
Best regards,
Marek Vasut


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] npm: change install directory to upstream default
  2018-10-19 16:53 ` Marek Vasut
@ 2018-10-22  9:50   ` Olaf Mandel
  2018-10-22 10:10     ` [PATCH v3] " Olaf Mandel
  0 siblings, 1 reply; 12+ messages in thread
From: Olaf Mandel @ 2018-10-22  9:50 UTC (permalink / raw)
  To: openembedded-core
  Cc: Marek Vasut, Anders Darander, Joshua Lock, Paul Eggleton, Olaf Mandel

The node binary searches for packages in a number of locations, the last
of which is $PREFIX/lib/node (here: /usr/lib/node) from the list of
GLOBAL_FOLDERS [1]. So change the installation directory for all
packages depending on npm.bbclass to that location. This removes the
need to define the NODE_PATH variable to the non-standard
/usr/lib/node_modules value.

While the Tips for Package Managers [2] discusses installing packages to
/usr/lib/node_modules/<name>/<version>, this has several drawbacks:

 * it does not work for the REPL as mentioned in the documentation
 * it also does not work for any code _not_ installed as a global
   package under /usr/lib/node_modules (e.g. /usr/share/foo.js will not
   find any packages below /usr/lib)
 * using the non-default location and then having to set NODE_PATH
   barely saves any time: there are only two file-system lookups (to the
   legacy $HOME/.node_modules and $HOME/.node_libraries) directories
   before the library would be found

And the suggestion was made in the context of deduping the node_modules
tree by installing all packages in a flat hierarchy and using symlinks
to the correct version of each dependency. This is not what OpenEmbedded
does, so none of those benefits (deduping, cleaner packages) are being
had by shifting the installation directory to /usr/lib/node_modules.

The choice of a "proper" installation path is not helped by npm
installing to /usr/lib/node_modules if asked to install globally. Still,
using the location expected by nodejs (/usr/lib/node) seems the right
choice.

[1]: https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders
[2]: https://nodejs.org/api/modules.html#modules_addenda_package_manager_tips

Signed-off-by: Olaf Mandel <o.mandel@menlosystems.com>
---
 meta/classes/npm.bbclass | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index c351ff0866..30febcffb2 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -10,7 +10,7 @@ def node_pkgname(d):
 
 NPMPN ?= "${@node_pkgname(d)}"
 
-NPM_INSTALLDIR = "${D}${libdir}/node_modules/${NPMPN}"
+NPM_INSTALLDIR = "${libdir}/node/${NPMPN}"
 
 # function maps arch names to npm arch names
 def npm_oe_arch_map(target_arch, d):
@@ -52,9 +52,10 @@ npm_do_install() {
 	# changing the home directory to the working directory, the .npmrc will
 	# be created in this directory
 	export HOME=${WORKDIR}
-	mkdir -p ${NPM_INSTALLDIR}/
+	mkdir -p ${D}${libdir}/node_modules
 	npm pack .
 	npm install --prefix ${D}${prefix} -g --arch=${NPM_ARCH} --target_arch=${NPM_ARCH} --production --no-registry ${NPMPN}-${PV}.tgz
+	mv ${D}${libdir}/node_modules ${D}${libdir}/node
 	if [ -d ${D}${prefix}/etc ] ; then
 		# This will be empty
 		rmdir ${D}${prefix}/etc
@@ -62,13 +63,13 @@ npm_do_install() {
 }
 
 python populate_packages_prepend () {
-    instdir = d.expand('${D}${libdir}/node_modules/${NPMPN}')
+    instdir = d.expand('${D}${NPM_INSTALLDIR}')
     extrapackages = oe.package.npm_split_package_dirs(instdir)
     pkgnames = extrapackages.keys()
     d.prependVar('PACKAGES', '%s ' % ' '.join(pkgnames))
     for pkgname in pkgnames:
         pkgrelpath, pdata = extrapackages[pkgname]
-        pkgpath = '${libdir}/node_modules/${NPMPN}/' + pkgrelpath
+        pkgpath = '${NPM_INSTALLDIR}/' + pkgrelpath
         # package names can't have underscores but npm packages sometimes use them
         oe_pkg_name = pkgname.replace('_', '-')
         expanded_pkgname = d.expand(oe_pkg_name)
@@ -84,7 +85,7 @@ python populate_packages_prepend () {
 }
 
 FILES_${PN} += " \
-    ${libdir}/node_modules/${NPMPN} \
+    ${NPM_INSTALLDIR} \
 "
 
 EXPORT_FUNCTIONS do_compile do_install
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✗ patchtest: failure for npm: change install directory to upstream default (rev2)
  2018-10-19 15:22 [PATCH] npm: change install directory to upstream default Olaf Mandel
  2018-10-19 16:53 ` Marek Vasut
@ 2018-10-22 10:03 ` Patchwork
  2018-10-22 10:33 ` ✗ patchtest: failure for npm: change install directory to upstream default (rev3) Patchwork
  2018-10-26 17:46 ` [PATCH] npm: change install directory to upstream default Dan McGregor
  3 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-22 10:03 UTC (permalink / raw)
  To: Olaf Mandel; +Cc: openembedded-core

== Series Details ==

Series: npm: change install directory to upstream default (rev2)
Revision: 2
URL   : https://patchwork.openembedded.org/series/14610/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at daba6c5a99)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] npm: change install directory to upstream default
  2018-10-22  9:50   ` [PATCH v2] " Olaf Mandel
@ 2018-10-22 10:10     ` Olaf Mandel
  0 siblings, 0 replies; 12+ messages in thread
From: Olaf Mandel @ 2018-10-22 10:10 UTC (permalink / raw)
  To: openembedded-core
  Cc: Marek Vasut, Anders Darander, Joshua Lock, Paul Eggleton, Olaf Mandel

The node binary searches for packages in a number of locations, the last
of which is $PREFIX/lib/node (here: /usr/lib/node) from the list of
GLOBAL_FOLDERS [1]. So change the installation directory for all
packages depending on npm.bbclass to that location. This removes the
need to define the NODE_PATH variable to the non-standard
/usr/lib/node_modules value.

While the Tips for Package Managers [2] discusses installing packages to
/usr/lib/node_modules/<name>/<version>, this has several drawbacks:

 * it does not work for the REPL as mentioned in the documentation
 * it also does not work for any code _not_ installed as a global
   package under /usr/lib/node_modules (e.g. /usr/share/foo.js will not
   find any packages below /usr/lib)
 * using the non-default location and then having to set NODE_PATH
   barely saves any time: there are only two file-system lookups (to the
   legacy $HOME/.node_modules and $HOME/.node_libraries) directories
   before the library would be found

And the suggestion was made in the context of deduping the node_modules
tree by installing all packages in a flat hierarchy and using symlinks
to the correct version of each dependency. This is not what OpenEmbedded
does, so none of those benefits (deduping, cleaner packages) are being
had by shifting the installation directory to /usr/lib/node_modules.

The choice of a "proper" installation path is not helped by npm
installing to /usr/lib/node_modules if asked to install globally. Still,
using the location expected by nodejs (/usr/lib/node) seems the right
choice.

[1]: https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders
[2]: https://nodejs.org/api/modules.html#modules_addenda_package_manager_tips

Signed-off-by: Olaf Mandel <o.mandel@menlosystems.com>
---
 meta/classes/npm.bbclass | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
index c351ff0866..30febcffb2 100644
--- a/meta/classes/npm.bbclass
+++ b/meta/classes/npm.bbclass
@@ -10,7 +10,7 @@ def node_pkgname(d):
 
 NPMPN ?= "${@node_pkgname(d)}"
 
-NPM_INSTALLDIR = "${D}${libdir}/node_modules/${NPMPN}"
+NPM_INSTALLDIR = "${libdir}/node/${NPMPN}"
 
 # function maps arch names to npm arch names
 def npm_oe_arch_map(target_arch, d):
@@ -52,9 +52,10 @@ npm_do_install() {
 	# changing the home directory to the working directory, the .npmrc will
 	# be created in this directory
 	export HOME=${WORKDIR}
-	mkdir -p ${NPM_INSTALLDIR}/
+	mkdir -p ${D}${libdir}/node_modules
 	npm pack .
 	npm install --prefix ${D}${prefix} -g --arch=${NPM_ARCH} --target_arch=${NPM_ARCH} --production --no-registry ${NPMPN}-${PV}.tgz
+	mv ${D}${libdir}/node_modules ${D}${libdir}/node
 	if [ -d ${D}${prefix}/etc ] ; then
 		# This will be empty
 		rmdir ${D}${prefix}/etc
@@ -62,13 +63,13 @@ npm_do_install() {
 }
 
 python populate_packages_prepend () {
-    instdir = d.expand('${D}${libdir}/node_modules/${NPMPN}')
+    instdir = d.expand('${D}${NPM_INSTALLDIR}')
     extrapackages = oe.package.npm_split_package_dirs(instdir)
     pkgnames = extrapackages.keys()
     d.prependVar('PACKAGES', '%s ' % ' '.join(pkgnames))
     for pkgname in pkgnames:
         pkgrelpath, pdata = extrapackages[pkgname]
-        pkgpath = '${libdir}/node_modules/${NPMPN}/' + pkgrelpath
+        pkgpath = '${NPM_INSTALLDIR}/' + pkgrelpath
         # package names can't have underscores but npm packages sometimes use them
         oe_pkg_name = pkgname.replace('_', '-')
         expanded_pkgname = d.expand(oe_pkg_name)
@@ -84,7 +85,7 @@ python populate_packages_prepend () {
 }
 
 FILES_${PN} += " \
-    ${libdir}/node_modules/${NPMPN} \
+    ${NPM_INSTALLDIR} \
 "
 
 EXPORT_FUNCTIONS do_compile do_install
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✗ patchtest: failure for npm: change install directory to upstream default (rev3)
  2018-10-19 15:22 [PATCH] npm: change install directory to upstream default Olaf Mandel
  2018-10-19 16:53 ` Marek Vasut
  2018-10-22 10:03 ` ✗ patchtest: failure for npm: change install directory to upstream default (rev2) Patchwork
@ 2018-10-22 10:33 ` Patchwork
  2018-10-22 10:45   ` Olaf Mandel
  2018-10-26 17:46 ` [PATCH] npm: change install directory to upstream default Dan McGregor
  3 siblings, 1 reply; 12+ messages in thread
From: Patchwork @ 2018-10-22 10:33 UTC (permalink / raw)
  To: Olaf Mandel; +Cc: openembedded-core

== Series Details ==

Series: npm: change install directory to upstream default (rev3)
Revision: 3
URL   : https://patchwork.openembedded.org/series/14610/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at daba6c5a99)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ✗ patchtest: failure for npm: change install directory to upstream default (rev3)
  2018-10-22 10:33 ` ✗ patchtest: failure for npm: change install directory to upstream default (rev3) Patchwork
@ 2018-10-22 10:45   ` Olaf Mandel
  2018-10-22 22:45     ` Richard Purdie
  0 siblings, 1 reply; 12+ messages in thread
From: Olaf Mandel @ 2018-10-22 10:45 UTC (permalink / raw)
  To: openembedded-core

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hello,

Am 22.10.2018 um 12:33 schrieb Patchwork:
> * Issue             Series does not apply on top of target branch
> [test_series_merge_on_head] Suggested fix    Rebase your series on
> top of targeted branch Targeted branch  master (currently at
> daba6c5a99)
> 
The v2 of the patch does not apply because the master branch already
contains v1 of the patch. I didn't notice this and resubmitted v2 as
v3 after a rebase (to the sumo-branch, not the master branch). So v2
and v3 are identical except for the parent commit hash.

The problem is: v1 does not actually work. What is the procedure to
handle this? Should I make a patch that works on top of the v1 already
in master?

Thanks,
Olaf
- -- 
Olaf Mandel

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEErGOsr28GGMKRE/TmVL2dtGJSHf8FAlvNqj8ACgkQVL2dtGJS
Hf+zIgf9EFUbmTpQXt1J1rTdK5EQM0o4Jewb7UzGE4XmlSYvi74hem8kjGpYGlXu
46Ec5DR7DgvBzPc2lAw5npVxgC+2PXovVOpne/xnZtwvPM6rKd+AzNNYCm6n52aZ
eGz67KF10+ypSFz76yQ5sxFIc7qivwKmJuzLuB89rFtZRv5aPV0sxZz36f+Duyoi
s0hgdgKCB9IUiXvzVgeCbC3kTFGDuYFGTsBL+X/4YAvgeBoABUJSxtmjScgArk+N
h1Ph9frGqS/jK1v4nU2MW76w/vwhooiRHBxRm/fffVkJiJYXmIqedYWLbsL872q9
QRCJM68mEf4e9dLOepNXbNsfhs2e+w==
=VLfd
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ✗ patchtest: failure for npm: change install directory to upstream default (rev3)
  2018-10-22 10:45   ` Olaf Mandel
@ 2018-10-22 22:45     ` Richard Purdie
  2018-10-23 10:30       ` Olaf Mandel
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2018-10-22 22:45 UTC (permalink / raw)
  To: Olaf Mandel, openembedded-core

On Mon, 2018-10-22 at 12:45 +0200, Olaf Mandel wrote:
> Hello,
> 
> Am 22.10.2018 um 12:33 schrieb Patchwork:
> > * Issue             Series does not apply on top of target branch
> > [test_series_merge_on_head] Suggested fix    Rebase your series on
> > top of targeted branch Targeted branch  master (currently at
> > daba6c5a99)
> > 
> 
> The v2 of the patch does not apply because the master branch already
> contains v1 of the patch. I didn't notice this and resubmitted v2 as
> v3 after a rebase (to the sumo-branch, not the master branch). So v2
> and v3 are identical except for the parent commit hash.
> 
> The problem is: v1 does not actually work. What is the procedure to
> handle this? Should I make a patch that works on top of the v1
> already
> in master?

Normally I'd suggest a new patch against master. In this case I've
created one and queued it in next.

Cheers,

Richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: ✗ patchtest: failure for npm: change install directory to upstream default (rev3)
  2018-10-22 22:45     ` Richard Purdie
@ 2018-10-23 10:30       ` Olaf Mandel
  0 siblings, 0 replies; 12+ messages in thread
From: Olaf Mandel @ 2018-10-23 10:30 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hello,

Am 23.10.2018 um 00:45 schrieb Richard Purdie:
> On Mon, 2018-10-22 at 12:45 +0200, Olaf Mandel wrote:
>> [...] What is the procedure to handle this? Should I make a
>> patch that works on top of the v1 already in master?
> 
> Normally I'd suggest a new patch against master. In this case I've 
> created one and queued it in next.
> 
Thank you! And sorry about the noise.

Best regards,
Olaf
- -- 
Olaf Mandel

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEErGOsr28GGMKRE/TmVL2dtGJSHf8FAlvO+B4ACgkQVL2dtGJS
Hf8tXgf/aOMaNVwsdOd+X5d4v81/cw5cquOiGAHscPBCtqyJnfHUjigBQ8OXzqMq
j01RA0G4ML8iIB4HfWBZVtqANIqsPaflRwhG07ykvX2N/VdiMyJ36FBKYmjmsMlF
3Fjr7BF9V2MOEqu2Ddte1S7OD8DAoU+99oUHUhsPSDiubBd1YZhwu9J0KPRXW8wC
uaUww2X7Ung3IVQnGCM6aU8Mvx3KZoDZ92hkhillRHQr9TTJYzXPdI9H7LgKQmI5
FB5JyFwtqMBZbtjv49Rrnkm0apCf1NzZT64RBknKsPL5Afd1wVJ20x8UeZM6+BWR
gJqXgQk+tsremI4tYji9GBn3ST8Udw==
=jWFl
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] npm: change install directory to upstream default
  2018-10-19 15:22 [PATCH] npm: change install directory to upstream default Olaf Mandel
                   ` (2 preceding siblings ...)
  2018-10-22 10:33 ` ✗ patchtest: failure for npm: change install directory to upstream default (rev3) Patchwork
@ 2018-10-26 17:46 ` Dan McGregor
  2018-10-29 10:39   ` Olaf Mandel
  3 siblings, 1 reply; 12+ messages in thread
From: Dan McGregor @ 2018-10-26 17:46 UTC (permalink / raw)
  To: o.mandel; +Cc: marex, Patches and discussions about the oe-core layer

On Fri, 19 Oct 2018 at 09:29, Olaf Mandel <o.mandel@menlosystems.com> wrote:
>
> The node binary searches for packages in a number of locations, the last
> of which is $PREFIX/lib/node (here: /usr/lib/node) from the list of
> GLOBAL_FOLDERS [1]. Change the installation directory for all packages
> depending on npm.bbclass to that location. This removes the need to
> define the NODE_PATH variable to the non-standard /usr/lib/node_modules
> value.
>
> While the Tips for Package Managers [2] discusses installing packages to
> /usr/lib/node_modules/<name>/<version>, this has several drawbacks:
>
>  * it does not work for the REPL as mentioned in the documentation
>  * it also does not work for any code _not_ installed as a global
>    package under /usr/lib/node_modules (e.g. /usr/share/foo.js will not
>    find any packages below /usr/lib)
>  * using the non-default location and then having to set NODE_PATH
>    barely saves any time: there are only two file-system lookups (to the
>    legacy $HOME/.node_modules and $HOME/.node_libraries) directories
>    before the library would be found
>
> And the suggestion was made in the context of deduping the node_modules
> tree by installing all packages in a flat hierarchy and using symlinks
> to the correct version of each dependency. This is not what OpenEmbedded
> does, so none of those benefits (deduping, cleaner packages) are being
> had by shifting the installation directory to /usr/lib/node_modules.
>
> [1]: https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders
> [2]: https://nodejs.org/api/modules.html#modules_addenda_package_manager_tips
>
> Signed-off-by: Olaf Mandel <o.mandel@menlosystems.com>
> ---
>  meta/classes/npm.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/classes/npm.bbclass b/meta/classes/npm.bbclass
> index c351ff0866..d5ff0c6d57 100644
> --- a/meta/classes/npm.bbclass
> +++ b/meta/classes/npm.bbclass
> @@ -10,7 +10,7 @@ def node_pkgname(d):
>
>  NPMPN ?= "${@node_pkgname(d)}"
>
> -NPM_INSTALLDIR = "${D}${libdir}/node_modules/${NPMPN}"
> +NPM_INSTALLDIR = "${D}${libdir}/node/${NPMPN}"

Ï realise this has already gone în, but should this be
${nonarch_libdir}, or does it use /usr/lib64 or /usr/libx32 correctly?

>
>  # function maps arch names to npm arch names
>  def npm_oe_arch_map(target_arch, d):
> --
> 2.11.0
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] npm: change install directory to upstream default
  2018-10-26 17:46 ` [PATCH] npm: change install directory to upstream default Dan McGregor
@ 2018-10-29 10:39   ` Olaf Mandel
  2018-10-31 16:38     ` Dan McGregor
  0 siblings, 1 reply; 12+ messages in thread
From: Olaf Mandel @ 2018-10-29 10:39 UTC (permalink / raw)
  To: Dan McGregor; +Cc: marex, Patches and discussions about the oe-core layer

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hello Dan,

Am 26.10.2018 um 19:46 schrieb Dan McGregor:
> On Fri, 19 Oct 2018 at 09:29, Olaf Mandel
> <o.mandel@menlosystems.com> wrote:
>> -NPM_INSTALLDIR = "${D}${libdir}/node_modules/${NPMPN}" 
>> +NPM_INSTALLDIR = "${D}${libdir}/node/${NPMPN}"
> 
> Ï realise this has already gone în, but should this be 
> ${nonarch_libdir}, or does it use /usr/lib64 or /usr/libx32
> correctly?
> 
For me, libdir resolves to /usr/lib, so I didn't notice any problems.
And the "lib"-part is hardcoded in NodeJS (lib/module.js:
path.resolve(prefixDir, 'lib', 'node')).

So if ${nonarch_libdir} is always ${prefix}/lib even if ${libdir}
isn't, then that should be changed.

Best regards,
Olaf Mandel
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEErGOsr28GGMKRE/TmVL2dtGJSHf8FAlvW43UACgkQVL2dtGJS
Hf8gLAf/aIQ5z219f1Tqtp5Oc1mBr4IN3T/Uzk6MlKKtqfQGS2KBF7CqWLUKNXT2
/eKa/v7H55yTWC6PSlJjx7HMzJK9mhwZ84LwAQ4rpj6ZBn1RYjv4DBdgPxlQN+1N
4kAoWklpgBut0eNtb9hniwf2VhF7rkI8eLVdNcL1SR7JroDahSnjVGozuE/asxK0
yJHHaz3cbe6QooYKeyi4y6lM1wJfP5Wny0rOCMh3rWvsexf7G7itki0MoRU7dNVv
C2zK+/KVjhYbLWSRijSYjGRFpMzKb9AddBxkpIZ4z7vjAlbbbYN1iw3RdPoiPYhP
rwvtSk3Wwe4McsVUjRaEFxwcdCPQQQ==
=1BcQ
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] npm: change install directory to upstream default
  2018-10-29 10:39   ` Olaf Mandel
@ 2018-10-31 16:38     ` Dan McGregor
  0 siblings, 0 replies; 12+ messages in thread
From: Dan McGregor @ 2018-10-31 16:38 UTC (permalink / raw)
  To: o.mandel; +Cc: marex, Patches and discussions about the oe-core layer

On Mon, 29 Oct 2018 at 04:39, Olaf Mandel <o.mandel@menlosystems.com> wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Hello Dan,
>
> Am 26.10.2018 um 19:46 schrieb Dan McGregor:
> > On Fri, 19 Oct 2018 at 09:29, Olaf Mandel
> > <o.mandel@menlosystems.com> wrote:
> >> -NPM_INSTALLDIR = "${D}${libdir}/node_modules/${NPMPN}"
> >> +NPM_INSTALLDIR = "${D}${libdir}/node/${NPMPN}"
> >
> > Ï realise this has already gone în, but should this be
> > ${nonarch_libdir}, or does it use /usr/lib64 or /usr/libx32
> > correctly?
> >
> For me, libdir resolves to /usr/lib, so I didn't notice any problems.
> And the "lib"-part is hardcoded in NodeJS (lib/module.js:
> path.resolve(prefixDir, 'lib', 'node')).
>
> So if ${nonarch_libdir} is always ${prefix}/lib even if ${libdir}
> isn't, then that should be changed.

Okay, that makes sense. The behaviour of nonarch_libdir is as you
described, sp on certain targets (ie x86_64 or aarch64) with multilib
enabled ${libdir} would resolve to /usr/lib64.

>
> Best regards,
> Olaf Mandel
> -----BEGIN PGP SIGNATURE-----
>
> iQEzBAEBCAAdFiEErGOsr28GGMKRE/TmVL2dtGJSHf8FAlvW43UACgkQVL2dtGJS
> Hf8gLAf/aIQ5z219f1Tqtp5Oc1mBr4IN3T/Uzk6MlKKtqfQGS2KBF7CqWLUKNXT2
> /eKa/v7H55yTWC6PSlJjx7HMzJK9mhwZ84LwAQ4rpj6ZBn1RYjv4DBdgPxlQN+1N
> 4kAoWklpgBut0eNtb9hniwf2VhF7rkI8eLVdNcL1SR7JroDahSnjVGozuE/asxK0
> yJHHaz3cbe6QooYKeyi4y6lM1wJfP5Wny0rOCMh3rWvsexf7G7itki0MoRU7dNVv
> C2zK+/KVjhYbLWSRijSYjGRFpMzKb9AddBxkpIZ4z7vjAlbbbYN1iw3RdPoiPYhP
> rwvtSk3Wwe4McsVUjRaEFxwcdCPQQQ==
> =1BcQ
> -----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-10-31 16:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-19 15:22 [PATCH] npm: change install directory to upstream default Olaf Mandel
2018-10-19 16:53 ` Marek Vasut
2018-10-22  9:50   ` [PATCH v2] " Olaf Mandel
2018-10-22 10:10     ` [PATCH v3] " Olaf Mandel
2018-10-22 10:03 ` ✗ patchtest: failure for npm: change install directory to upstream default (rev2) Patchwork
2018-10-22 10:33 ` ✗ patchtest: failure for npm: change install directory to upstream default (rev3) Patchwork
2018-10-22 10:45   ` Olaf Mandel
2018-10-22 22:45     ` Richard Purdie
2018-10-23 10:30       ` Olaf Mandel
2018-10-26 17:46 ` [PATCH] npm: change install directory to upstream default Dan McGregor
2018-10-29 10:39   ` Olaf Mandel
2018-10-31 16:38     ` Dan McGregor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.