Paths

Table of Contentst

[keyserver] Run `source-nvm.sh` using `bash` in `Docker` container
ClosedPublic
Actions

Authored by • abosh on Jul 1 2022, 6:05 PM.

Details

Reviewers

atul
ashoat

Commits

rCOMM3f930534e2ad: [keyserver] Run `source-nvm.sh` using `bash` in `Docker` container

Summary

Relevant Linear issue with full context here. This diff modifies the Dockerfile in keyserver to execute source-nvm.sh in the environment specified by the shebang in source-nvm.sh instead of in the current environment of the container, which uses sh.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• abosh created this revision.Jul 1 2022, 6:05 PM

Herald added subscribers: • adrian, tomek. · View Herald TranscriptJul 1 2022, 6:05 PM

• abosh added inline comments.Jul 1 2022, 6:06 PM

keyserver/Dockerfile
97 ↗	(On Diff #14101)	A minor change tacked onto this diff because the linter was complaining:

• abosh attached a referenced file: F89798: image.png. (Show Details)Jul 1 2022, 6:06 PM

• abosh edited the summary of this revision. (Show Details)

• abosh edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B10217: Diff 14101.Jul 1 2022, 6:18 PM

• abosh requested review of this revision.Jul 1 2022, 6:18 PM

• abosh added a child revision: D4422: [keyserver] Use `[[ ]]` instead of `[ ]` in `source-nvm.sh`.Jul 1 2022, 6:39 PM

Makes sense

keyserver/Dockerfile
89 ↗	(On Diff #14101)	Should we just do `keyserver/bash/source-nvm.sh`? Does the `cd` persist into the next command? (I think not)

This revision is now accepted and ready to land.Jul 2 2022, 6:52 PM

ashoat mentioned this in D4422: [keyserver] Use `[[ ]]` instead of `[ ]` in `source-nvm.sh`.Jul 2 2022, 6:53 PM

• abosh added inline comments.Jul 3 2022, 9:05 AM

keyserver/Dockerfile
89 ↗	(On Diff #14101)	Great question! It should be fine. From the Docker docs: The `WORKDIR` instruction sets the working directory for any `RUN`, `CMD`, `ENTRYPOINT`, `COPY` and `ADD` instructions that follow it in the `Dockerfile`. Thus, the next `COPY` command will not be affected by the `cd` here. Read more here.

• abosh marked an inline comment as done.Jul 3 2022, 9:05 AM

Closed by commit rCOMM3f930534e2ad: [keyserver] Run `source-nvm.sh` using `bash` in `Docker` container (authored by • abosh). · Explain WhyJul 3 2022, 10:31 AM

This revision was automatically updated to reflect the committed changes.

• abosh added a commit: rCOMM3f930534e2ad: [keyserver] Run `source-nvm.sh` using `bash` in `Docker` container.

ashoat added inline comments.Jul 12 2022, 3:25 PM

keyserver/Dockerfile
89	Based on this comment on D4429, I suspect that `./source-nvm.sh` doesn't actually work to set the version of Node. But that's actually fine – we don't really rely on that here... we're running `./source-nvm.sh` just to trigger `nvm` to download Node so it's cached in Docker. We rely on `bash/run-prod.sh` to handle calling `nvm` correctly. The only effect here will be that commands like `yarn workspace landing prod` (Webpack) and `yarn workspace keyserver prod-build` (Babel, GeoIP download) run using Node 16.13 instead of 16.9. This is probably fine

• abosh added a reverting change: D4526: [keyserver] Revert D4422 and D4429.Jul 13 2022, 12:55 PM

• abosh added a reverting change: rCOMMc2ddd80af740: [keyserver] Revert D4422 and D4429.Jul 14 2022, 8:11 AM

Revision Contents
Changeset List

Path

Size

keyserver/

Dockerfile

4 lines

Diff 14120

View Options

[keyserver] Run `source-nvm.sh` using `bash` in `Docker` containerClosedPublicActions