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

[readme] add docker tips #2318

Merged
merged 1 commit into from
Feb 3, 2025
Merged

Conversation

cdaringe
Copy link
Contributor

Problem

It's common to install node in docker, and using NVM is quite helpful in this regard. Installing and using NVM isn't always. straightforward.

Solution

Make it easier for users to setup nvm in docker by droppin' some docs.

Ref: https://stackoverflow.com/questions/25899912/how-to-install-nvm-in-docker

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdaringe
Copy link
Contributor Author

@ljharb , I'm open to your suggestion, but those docker files fundamentally are for a different intent. Those are for running a first-class NVM image, where the above instructions teach the user how to make any image a NVM image. Implicitly, those docker files do the required work, but they do so with noise interspersed, and without a clear focus on the requirements to make NVM work in any image. Perhaps simply sprinkling on some extra verbage with reference to those would be useful? I don't think references to those alone are the correct solve.

README.md Outdated
Comment on lines 97 to 98
# patch PATH
ENV PATH $NVM_DIR/versions/node/v$NODE_VERSION/bin:$PATH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not belong in docs; nobody should be manually editing the PATH, only sourcing nvm.sh and letting nvm do it.

Suggested change
# patch PATH
ENV PATH $NVM_DIR/versions/node/v$NODE_VERSION/bin:$PATH

it's also not necessary here since nvm install handles it in the next RUN statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't each run in docker happen in a new shell? iirc, mutating your env in a RUN doesn't persist in downstream runs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - but instead of hardcoding the PATH, is there a way the RUN that sources nvm.sh could export the resulting PATH for further RUNs to use?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb @cdargine ENV PATH="$NVM_DIR/versions/node/v$NODE_VERSION/bin:$PATH" sholdnt this export the path to resulting run commands ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

he's looking for something like ENV=$(/path/to/nvm env) I think. Feel free to push to or close this or, btw.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb, i have been thinking about it, and I don't see a way forward otherwise, which is disappointing. ENV stanzas do not accept child commands, so we are left with only build args, the file system, and RUN blocks as sources of dynamic input, however only build arg inputs can be pulled into the image ENV definition from what i can tell. or, better said, i cannot find a way to dynamically promote content from within a build step to the image ENV. if ENV PATH=$PATH;$(nvm path) worked, we would have a clean solve. it does not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, a paragraph in the docs that explains these things clearly (including to me, who has minimal docker experience) might be a good way to resolve this PR?

Copy link
Contributor

@blole blole Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found a pretty neat way of installing nvm in a docker container by leveraging the bash environment variable $BASH_ENV, this way we don't even need to make the bash in the container into an interactive/login shell.

FROM ubuntu:22.10
SHELL ["/bin/bash", "-o", "pipefail", "-c"]

ENV BASH_ENV /root/.env
RUN touch $BASH_ENV
RUN echo '. "'"$BASH_ENV"'"' >> ~/.bashrc # only needed if executing interactively into the container

RUN apt-get update && apt-get install -y curl
RUN curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.2/install.sh | PROFILE="$BASH_ENV" bash

COPY .nvmrc .nvmrc
RUN nvm install
RUN node -v

This way nvm+node is easily available both later in the Dockerfile, and when executing into the container

> echo 19.0.1 > .nvmrc
> docker build -t foo .
...
Step 9/10 : RUN nvm install
Found '//.nvmrc' with version <19.0.1>
...
Now using node v19.0.1 (npm v8.19.2)
Creating default alias: default -> 19.0.1 (-> v19.0.1 *)
...
Step 10/10 : RUN node -v
v19.0.1
> docker run --rm -it --entrypoint bash foo
root@4e5088bdde4d:/# tail -1 ~/.bashrc
. "/root/.env"
root@4e5088bdde4d:/# node -v
v19.0.1

Maybe it's neat enough to fit in the readme?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does seem pretty compelling.

Do you think that would be a replacement for this PR? If so, please post a link to a branch (NOT a new PR) and i'll pull in the changes. If not, please file a new PR :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think it could be.

Sure!
https://github.com/blole/nvm/tree/docs/install-in-docker

I tried my hand at updating the Dockerfile in the repo as well, it seems to work like I'd expect, but I'm not sure what people use it for, so I can't really verify that.

 - covers installation using BASH_ENV

Co-authored-by: Christopher Dieringer <cdaringe@users.noreply.github.com>
Co-authored-by: Björn Holm <blollle@gmail.com>
@ljharb ljharb marked this pull request as draft December 23, 2022 20:24
@ljharb ljharb force-pushed the master branch 2 times, most recently from c6cfc3a to c20db2a Compare June 10, 2024 18:13
@ljharb ljharb force-pushed the docs/install-in-docker branch from 8b14f36 to b77fcec Compare February 3, 2025 19:53
@ljharb ljharb marked this pull request as ready for review February 3, 2025 19:53
@ljharb ljharb changed the title docs: add docker tips [readme] add docker tips Feb 3, 2025
@ljharb ljharb merged commit b77fcec into nvm-sh:master Feb 3, 2025
169 of 171 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants