Skip to content

Docker issues #104

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

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -1 +1,12 @@
.git
.gitignore
Dockerfile
lib
node_modules
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we want to not put node_modules into the container?

Right now yarn doesn't use it as a cache but it could in the future. See the comment in the Dockerfile.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we can add it back later.

Copy link
Author

Choose a reason for hiding this comment

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

https://medium.com/@rdsubhas/docker-for-development-common-problems-and-solutions-95b25cae41eb

In linux we have linux binaries, on mac we have mac binaries, on windows we have exe files.

NPM is a hot mess, so node_modules should be git + docker ignored.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about this? I thought node_modules only contained source code? Not actual binaries.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zerdos is right. Check the folder node_modules/.bin/. There are modules that build executables in there (the node-sass package is one that comes to mind. I don't think code-server uses it, but there are many other examples).

**/dist
**/out
.DS_Store
*.DS_Store
Copy link

Choose a reason for hiding this comment

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

why an additional starred .DS_Store?
it is not necessary

Suggested change
*.DS_Store

release
**/yarn-error.log
Copy link

Choose a reason for hiding this comment

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

Shouldn't any logs be ignored?

Suggested change
**/yarn-error.log
**/*.log

**/node_modules
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ dist
out
.DS_Store
release
yarn-error.log
Copy link

Choose a reason for hiding this comment

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

Same as with .dockerignore, all log-files should be ignored

Suggested change
yarn-error.log
**/*.log

35 changes: 25 additions & 10 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,32 +1,47 @@
FROM node:8.15.0
FROM node:8.15.1
Copy link
Contributor

Choose a reason for hiding this comment

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

8.15.1 will not build as nexe does not support it.

https://github.com/nexe/nexe/releases


# Install VS Code's deps. These are the only two it seems we need.
RUN apt-get update && apt-get install -y \
libxkbfile-dev \
libsecret-1-dev

# Ensure latest yarn.
RUN npm install -g [email protected]

WORKDIR /src
COPY . .

# In the future, we can use https://github.com/yarnpkg/rfcs/pull/53 to make yarn use the node_modules
# directly which should be fast as it is slow because it populates its own cache every time.
RUN yarn && yarn task build:server:binary
# This takes ages and always dies in the end. :(
# RUN yarn --frozen-lockfile && yarn task build:server:binary


# Make the debugging easier - and the rebuilds faster
# and our life happier lets break up the build to sequential parts
RUN yarn --frozen-lockfile

RUN yarn task vscode:install
RUN yarn task build:copy-vscode
RUN yarn task build:web
RUN yarn task build:bootstrap-fork
RUN yarn task build:default-extensions
RUN yarn task build:server:bundle
RUN yarn task build:app:browser
RUN yarn task build:server:binary:package

# We deploy with ubuntu so that devs have a familiar environemnt.
FROM ubuntu:18.10
WORKDIR /root/project
COPY --from=0 /src/packages/server/cli-linux /usr/local/bin/code-server

COPY --from=0 /src/packages/server/cli-linux-x64 /usr/local/bin/code-server

EXPOSE 8443

RUN apt-get update && apt-get install -y \
openssl \
net-tools

RUN apt-get install -y locales && \
locale-gen en_US.UTF-8

# We unfortunately cannot use update-locale because docker will not use the env variables
# configured in /etc/default/locale so we need to set it manually.
ENV LANG=en_US.UTF-8

# Unfortunately `.` does not work with code-server.
CMD code-server $PWD
CMD code-server $PWD
5 changes: 5 additions & 0 deletions build/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@ const ensureClean = register("vscode:clean", async (runner) => {

const status = await runner.execute("git", ["status", "--porcelain"]);
if (status.stdout.trim() !== "") {

// this is an uggly workaround - inside docker git clean throws an error message because of this directory
// related to git modules.
await runner.execute("rm", ["-rf", "node_modules/spdlog"]);

const clean = await runner.execute("git", ["clean", "-f", "-d", "-X"]);
if (clean.exitCode !== 0) {
throw new Error(`Failed to clean git repository: ${clean.stderr}`);
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
"description": "Run VS Code remotely.",
"scripts": {
"build:rules": "cd ./rules && tsc -p .",
"packages:install": "cd ./packages && yarn",
"postinstall": "npm-run-all --parallel packages:install build:rules",
"packages:install": "cd ./packages && yarn --frozen-lockfile",
"postinstall": "npm-run-all packages:install build:rules",
Copy link
Contributor

Choose a reason for hiding this comment

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

I want a rationale why the parallel flag have to be removed.

"start": "cd ./packages/server && yarn start",
"task": "ts-node -r tsconfig-paths/register build/tasks.ts",
"test": "cd ./packages && yarn test"
Expand Down
Loading