[New] Introduce Docker environment for nvm #1472

PeterDaveHello · 2017-04-02T01:08:22Z

This should help nvm development and testing

ljharb · 2017-04-02T02:44:54Z

This definitely belongs in a separate repo; I don't use Docker myself and can't commit to maintaining it. Separately, a docker file for developing nvm will rapidly be misused as a docker file for using nvm, and that won't be good for anybody.

PeterDaveHello · 2017-04-02T03:36:27Z

hmmm ... we still have problem on running tests locally, I believe that docker can help in this part. As nvm is under a person account not an organization account, I'm not sure how to make it separated, or maybe just be unofficial, but for the local testing works, it'll be definitely good to use docker to isolate the environment with very low overhead and complexity IMO, what do you think?

PeterDaveHello · 2017-04-02T03:41:55Z

The reason of this PR 😅 : #1339 (comment)

I'd be happy to review adding one, but I don't use any of those tools myself.

ljharb · 2017-04-02T04:09:04Z

Regardless of docker or no, the tests need to be fixed so they can be run and pass locally regardless of NVM_DIR location.

Also, Mac OS X is one of the most important platforms for nvm, and that can't be tested with docker; which means that id rather perfect the generic dev story before focusing on a container one.

You are correct about that comment, and I appreciate the PR. I will leave it open, but do not plan to merge it in the medium term.

PeterDaveHello · 2017-04-02T06:05:53Z

I thought there is a PR trying to add MacOS test in Travis CI already, there should be no conflicts about testing on MacOS and adding Dockerfile, am I right?
I think it's better to have a isolated environment for testing, not only working directory, it won't take the focus or mess up the development work, but will only make it cleaner.

PeterDaveHello · 2017-04-02T06:38:20Z

As it's for development environment usage, the docker image will be 1.3GB fat and it needs about 15 mins to be built, not thin and not quick at all, so I don't agree that it will rapidly be misused as a docker file for using nvm, it makes no sense.

ljharb · 2017-04-02T06:42:52Z

Fair points. I don't think these files need to be downloaded by users who install nvm using git, though. Can you think of a way to avoid that?

PeterDaveHello · 2017-04-02T06:48:34Z

If you wanna do that, yes, we can use sparseCheckout, and we should checkout only nvm-exec, nvm.sh & bash_completion, other files aren't needed, agree?

ljharb · 2017-04-02T06:56:08Z

I'd prefer an exclusion list approach, rather than an inclusion list approach - git updating needs to grab new file by default, so that i can split things up into separate files in the future.

Can we use sparseCheckout and check out everything except certain folders?

PeterDaveHello · 2017-04-02T06:57:48Z

Nope I don't think so.

ljharb · 2017-04-02T06:58:54Z

Then we'll need another solution, I think.

PeterDaveHello · 2017-04-02T07:01:03Z

I think we just don't need to be so afraid of misusing, should we? They have no reason to misuse it :)

ljharb · 2017-04-02T07:52:42Z

.dockerignore

@@ -0,0 +1,17 @@
+HEAD
+.cache
+v*


what about versions/?

will be also ignored as expected

oh duh, because it starts with v. k

Good to go now?

ljharb · 2017-04-02T07:52:55Z

.dockerignore

+.urchin_stdout
+test/**/test_output
+
+node_modules/


shouldn't docker already ignore everything in .gitignore?

PeterDaveHello · 2017-04-02T07:58:10Z

Dockerfile

+#
+# This is for development usage only, please not that it'll use about
+# 1.2 GB disk space and about 15 minutes to build this image, and this
+# is not a Dockerfile to build a Dockerized nvm for production usage.


I added notes here to remind people not to misuse it.

PeterDaveHello · 2017-04-06T20:09:32Z

btw @ljharb, I think we can update sparseCheckout in each versions' install script if you really need that, not a problem at all

PeterDaveHello · 2017-04-09T07:14:53Z

Actually I just found that this really really help me debug nvm problems, as it's purely isolated and I can spawn multi containers to test at the same time, which saved me many hours, hope we can give it a try, make the contribution easier for the others.

ljharb

What command would I run to test this out locally?

ljharb · 2017-04-16T06:17:14Z

Dockerfile

+RUN echo 'export PATH="~/.cabal/bin/:${PATH}"'                                >> $HOME/.bashrc
+
+# nvm
+COPY . /home/nvm/.nvm/


Is /home/nvm a path for inside the docker image?

Yes, it's inside docker.

PeterDaveHello · 2017-04-16T08:35:35Z

@ljharb try docker build -t nvm . at the root of nvm, then you'll have a docker image named nvm, if you need it be tagged like tagged commit as version in git, try docker build -t nvm:version .

Use docker images to get the result, it'll looks like:

REPOSITORY         TAG                 IMAGE ID            CREATED             SIZE
nvm                latest              9ca4c57a97d8        7 days ago          1.22 GB

PeterDaveHello · 2017-05-05T20:11:09Z

Docker images published here: https://hub.docker.com/r/peterdavehello/nvm/ wtih v0.33.2 tag.

PeterDaveHello · 2017-06-23T03:09:55Z

@ljharb is there anything else I can do here? Thanks.

ljharb · 2017-06-26T05:34:13Z

Is there any value in constraining this to a docker subdirectory?

PeterDaveHello · 2017-06-26T14:07:21Z

@ljharb do you mean you're considering to put it in a docker folder? I don't think we need to do that as our root is the same the repo root.

ljharb · 2017-06-26T18:23:40Z

Yes, so that docker-related files don't clutter up the main repo root.

PeterDaveHello · 2017-06-27T09:24:22Z

I'm not sure if it has some value, as we are not going to build production environment with many different base like nodejs would build with different operating systems which would need to be separated.

Personally, the current status looks good enough to me.

[New] Introduce Docker environment for nvm

3ac49e5

PeterDaveHello force-pushed the Dockerfile branch 4 times, most recently from 85340ca to ef6537d Compare April 2, 2017 07:44

ljharb reviewed Apr 2, 2017

View reviewed changes

PeterDaveHello force-pushed the Dockerfile branch from ef6537d to 6034765 Compare April 2, 2017 07:55

PeterDaveHello commented Apr 2, 2017

View reviewed changes

PeterDaveHello force-pushed the Dockerfile branch 7 times, most recently from e03ea3d to 5fe9e3a Compare April 2, 2017 08:11

PeterDaveHello force-pushed the Dockerfile branch from 5fe9e3a to b416489 Compare April 9, 2017 04:42

ljharb reviewed Apr 16, 2017

View reviewed changes

PeterDaveHello force-pushed the Dockerfile branch from b416489 to f452ac5 Compare April 16, 2017 09:41

PeterDaveHello mentioned this pull request Apr 26, 2017

How to run the tests locally? #1339

Open

PeterDaveHello force-pushed the Dockerfile branch from f452ac5 to b479b8b Compare May 5, 2017 19:46

ljharb force-pushed the Dockerfile branch from b479b8b to 3ac49e5 Compare June 28, 2017 07:09

ljharb merged commit 3ac49e5 into nvm-sh:master Jun 28, 2017

PeterDaveHello deleted the Dockerfile branch June 28, 2017 09:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[New] Introduce Docker environment for nvm #1472

[New] Introduce Docker environment for nvm #1472

PeterDaveHello commented Apr 2, 2017

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017 •

edited

Loading

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017 •

edited

Loading

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

ljharb Apr 2, 2017

PeterDaveHello Apr 2, 2017

ljharb Apr 3, 2017

PeterDaveHello Apr 3, 2017

ljharb Apr 2, 2017

PeterDaveHello Apr 2, 2017

PeterDaveHello Apr 2, 2017

PeterDaveHello commented Apr 6, 2017

PeterDaveHello commented Apr 9, 2017

ljharb left a comment

ljharb Apr 16, 2017

PeterDaveHello Apr 16, 2017

PeterDaveHello commented Apr 16, 2017

PeterDaveHello commented May 5, 2017

PeterDaveHello commented Jun 23, 2017

ljharb commented Jun 26, 2017

PeterDaveHello commented Jun 26, 2017

ljharb commented Jun 26, 2017

PeterDaveHello commented Jun 27, 2017

@@ @@ -0,0 +1,17 @@ @@
+              HEAD
+              .cache
+              v*

[New] Introduce Docker environment for nvm #1472

[New] Introduce Docker environment for nvm #1472

Conversation

PeterDaveHello commented Apr 2, 2017

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017 • edited Loading

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017 • edited Loading

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

ljharb commented Apr 2, 2017

PeterDaveHello commented Apr 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PeterDaveHello commented Apr 6, 2017

PeterDaveHello commented Apr 9, 2017

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PeterDaveHello commented Apr 16, 2017

PeterDaveHello commented May 5, 2017

PeterDaveHello commented Jun 23, 2017

ljharb commented Jun 26, 2017

PeterDaveHello commented Jun 26, 2017

ljharb commented Jun 26, 2017

PeterDaveHello commented Jun 27, 2017

PeterDaveHello commented Apr 2, 2017 •

edited

Loading

PeterDaveHello commented Apr 2, 2017 •

edited

Loading