Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onbuild does not work with npm-shrinkwrap.json #65

Closed
fakewaffle opened this issue Nov 17, 2015 · 7 comments
Closed

onbuild does not work with npm-shrinkwrap.json #65

fakewaffle opened this issue Nov 17, 2015 · 7 comments

Comments

@fakewaffle
Copy link

npm installs packages from a npm-shrinkwrap.json if one exists. They way in which the Dockerfile for onbuild is structured, npm will never install from npm-shrinkwrap.json.

https://github.com/nodejs/docker-node/blob/master/0.12/onbuild/Dockerfile#L6

@pesho
Copy link
Contributor

pesho commented Nov 17, 2015

Indeed. @nodejs/docker does anyone remember why we copy only package.json first, before running npm install, and only then copy the rest?

P.S. Ah yes, it's done in order to take advantage of the build-cache.

@Starefossen
Copy link
Member

This is similar to what is suggested here which says that there are no optional copy:

ONBUILD COPY package.json npm-shrinkwrap.json /usr/src/app/

This will fail for every project without an npm-shrinkwrap.json. This leaves us with three options:

Option 1: Copy everything before installing:

ONBUILD COPY . /usr/src/app/
ONBUILD RUN npm install

This will effectively disable Docker's build-cache and all modules will have to be re-installed for each change to the source code. The reason we use COPY package.json /usr/src/app/ is to only having to run npm install whenever package.json is changed.

Option 2: Make an :onbuild-shrinkwrap Image

ONBUILD COPY npm-shrinkwrap.json /usr/src/app/
ONBUILD RUN npm install

This will create more images that we will have to maintain and we already have quite a few:

5 supported versions x 4 tags = 20 different images

Option 3: Leave this up to those (few) who needs it

The onbuild image variant is ment as a quickly get started with Dockerizing an application, and is not recommended by Docker Inc. for long term / production usage. There is an open issue #66 to discourage excessive use of the onbuild image.

@retrohacker
Copy link
Contributor

We do have a 4th option. Not saying that it is any good.

Option 4: Use go's filepath.Match to add all json files.

ONBUILD COPY *.json /usr/src/app/
ONBUILD RUN npm install

This has the disadvantage that all json files in the base directory of the project will be added, thus any change to any json file will trigger an npm install. It doesn't look like there is a way to do the equivalent of (package|npm-shrinkwrap).json.

@retrohacker
Copy link
Contributor

I'm in favor of Option 3.

@fakewaffle
Copy link
Author

It's only awkward since one would assume that npm install would behave as expected - even though it doesn't since npm-shrinkwrap.json is never copied over.

Option 3 is fine as long as there is that additional documentation.

@chorrell
Copy link
Contributor

Yeah, Option 3 seems like the best way to go.

@openam
Copy link

openam commented Aug 3, 2016

FWIW I think there is an option 5.

Option 5: copy package.json and optionally the shrinkwrap

ONBUILD COPY package.json *pm-shrinkwrap.json /usr/src/app/
ONBUILD RUN npm install

There would still a similar problem as with option 4, but the chance of having multiple files ending in pm-shrinkwrap.json is.

The other thing I've noticed is if you have a .npmrc file it should be copied over too. Possibly add .npmr? to the list of things being copied over.

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

No branches or pull requests

6 participants