node plugin: run build in pull phase to download dependencies. #762

Merged
merged 1 commit into from Aug 30, 2016

Conversation

Projects
None yet
4 participants
Contributor

smoser commented Aug 26, 2016

This runs the build in the pull phase to download all needed dependencies.
There is no way to 'npm install --download-only'.

Additionally, we add the --cache-min=Infinity which is required.
Without it, the run in the build phase will try to reach the npm registry.

Lastly, instead of '-g', use the more self-documenting long form '--global'.

Can one of the admins verify this patch?

Can one of the admins verify this patch?

Contributor

smoser commented Aug 26, 2016

I dont know if this is exactly what sergio was looking for when he suggested this, but I have verfified that after a successful npm install, the second doees not need network access. Tested that in a container like this:

#!/bin/bash
set -e
set -o pipefail

LOG="$PWD/my.log"
cleanup() { network_up; }
network_up() { sudo ifup eth0; }
network_down() { sudo ifdown eth0; }
msg() { echo "$@" >> "$LOG"; echo "$@" 1>&2; }
run() {
    local envcmd="" n=$SECONDS
    msg "[start]" "${@}"
    "$@"
    r=$?
    msg "[finished ret=$r took=$(($SECONDS-n))]" "$@"
    return $r
}
trap cleanup EXIT
npm_install=( npm --cache-min=Infinity install azure-cli )

# do it as would be in the pull section
network_up
rm -Rf pull
mkdir pull
msg pull network-up
( cd pull && run "${npm_install[@]}" ) 2>&1 | tee pull.log

# now with network down
msg build network-down
network_down
rm -Rf build
mkdir build
( cd build && run "${npm_install[@]}" ) 2>&1 | tee build.log
snapcraft/plugins/nodejs.py
@@ -96,6 +96,8 @@ def pull(self):
super().pull()
os.makedirs(self._npm_dir, exist_ok=True)
self._nodejs_tar.download()
+ # we build in 'pull' phase to download all dependencies
+ self.build()
@elopio

elopio Aug 26, 2016

Member

This for sure solves the problem, but I'm concerned because I doubt that build was designed to be called from pull. There could be many unintended consequences, and the clean_pull is now a little worse for sure. We now can remove the build completely, and do just pull. That sounds weird. I'm not sure, this is a huge thing for a friday evening :)

@sergiusens

sergiusens Aug 26, 2016

Collaborator

El viernes, 26 de agosto de 2016 16h'32:09 ART, Leo Arias
notifications@github.com escribió:

@@ -96,6 +96,8 @@ def pull(self):
super().pull()
os.makedirs(self._npm_dir, exist_ok=True)
self._nodejs_tar.download()

  •    # we build in 'pull' phase to download all dependencies
    
  •    self.build()
    

This for sure solves the problem, but I'm concerned because I
doubt that build was designed to be called from pull. There
could be many unintended consequences, and the clean_pull is now
a little worse for sure. We now can remove the build completely,
and do just pull. That sounds weird. I'm not sure, this is a
huge thing for a friday evening :)

This is exactly what the python plugins do.
I would call an internal function though.

Enviado con Dekko desde mi dispositivo Ubuntu

node plugin: run build in pull phase to download dependencies.
Launchpad builders have internet access during the pull phase only.
Currently, node modules that have any external dependencies will
fail their build because they cannot reach the remote resources.

There is no way to tell npm to download without building, so
this runs the build in the pull phase to download all needed dependencies.
Adding --cache-min=Infinity is done so that the build phase invocation
of npm install will fully use cache and not try to reach the npm registry.

Lastly, instead of '-g', use the more self-documenting long form '--global'.
Contributor

smoser commented Aug 29, 2016

I updated to to address sergiusens' comment.
Separated the build into _npm_install and call it from build and pull.

Collaborator

sergiusens commented Aug 30, 2016

ok to test

@sergiusens sergiusens merged commit 0b32830 into snapcore:master Aug 30, 2016

4 checks passed

autopkgtest integration Success
Details
autopkgtest snaps Success
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.0007%) to 97.817%
Details

kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017

node plugin: run build in pull phase to download dependencies. (#762)
Launchpad builders have Internet access during the pull phase only.
Currently, node modules that have any external dependencies will
fail their build because they cannot reach the remote resources.

There is no way to tell npm to download without building, so
this runs the build in the pull phase to download all needed dependencies.
Adding --cache-min=Infinity is done so that the build phase invocation
of npm install will fully use cache and not try to reach the npm registry.

Lastly, instead of '-g', use the more self-documenting long form '--global'.

LP: #1612005
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment