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.

Sign up for GitHub

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

Use Corepack for selecting package manager #28

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open

Use Corepack for selecting package manager #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions Dockerfile.base
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
FROM dockerhubproxy.schibsted.io/node:NODE_VERSION_TEMPLATE-alpine
ARG YARN_VERSION
# ARG YARN_VERSION

LABEL maintainer "Simen Bekkhus <[email protected]>"
trygve-lie marked this conversation as resolved.
Show resolved Hide resolved
LABEL no.finn.docker.node-version "$NODE_VERSION"
LABEL no.finn.docker.yarn-version "$YARN_VERSION"
# LABEL no.finn.docker.yarn-version "$YARN_VERSION"

ENV NODE_ENV=production PATH="/home/node/scripts:${PATH}" SECRETS_DIR="/var/run/secrets/fiaas"

Expand All @@ -14,7 +14,7 @@ WORKDIR /home/node/src

RUN apk upgrade -U && \
apk add --no-cache dumb-init ca-certificates wget bash && \
npm i -g pnpm && \
# npm i -g pnpm && \
update-ca-certificates

COPY scripts /home/node/scripts/
Expand Down
8 changes: 3 additions & 5 deletions Dockerfile.onbuild
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,18 @@ ONBUILD ARG YARN_VERSION
# Allow installation as non-root user
ONBUILD RUN npm config set unsafe-perm true

# All but package.json is optional
ONBUILD COPY package.json yarn.lock* .yarnrc* .npmrc* npm-shrinkwrap.json* package-lock.json* pnpm-lock.yaml* ./
ONBUILD COPY . ./

# Install dependencies for native builds
# This is in one giant command to keep the image size small
# TODO: When Finnbuild uses Docker 1.13, we can use --squash, which means this won't have to be one giant command
ONBUILD RUN apk upgrade -U && \
apk add --no-cache --virtual build-dependencies make gcc g++ python git || \
apk add --no-cache --virtual build-dependencies make gcc g++ python3 git && \
corepack enable && \
corepack prepare --activate --all && \
install-dependencies.sh && \
rm /usr/local/bin/yarn && npm uninstall --loglevel warn --global pnpm && npm uninstall --loglevel warn --global npm && \
apk del build-dependencies

ONBUILD COPY . ./
Copy link
Member

Choose a reason for hiding this comment

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

Possibly better to do the copy as late as possible so Docker can reuse the layer which installs build dependencies, corepack and node dependencies when only changing some production code (this has nothing to do with package managers).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then I think we need to copy any lock file here and all the other files later. I am not 100% sure about that but we definetly need package.json copied here before we run any of the commands. I would really love if we could make it so that we don't have to hard code each package managers lock files into this. Any suggestions on how to avoid hard coding such?


ONBUILD RUN chown -R node:node .
ONBUILD USER node
2 changes: 1 addition & 1 deletion release.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fi

versions=(${VERSION//./ })

tag=containers.schibsted.io/finntech/node
tag=containers.schibsted.io/finntech/nodejs
onbuild_tag="$tag:onbuild"
test_tag="$tag:test"
test_onbuild_tag="$test_tag-onbuild"
Expand Down
34 changes: 4 additions & 30 deletions scripts/install-dependencies.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,9 @@
#!/usr/bin/env bash

# This script does `yarn install` if a `yarn.lock` file is present, otherwise `npm install`

set -e

if [[ -n "${FAIL_ON_DIRTY_LOCKFILE}" ]]; then
if [[ "${YARN_VERSION:0:2}" == "2." ]]; then
YARN_OPTS="--immutable"
else
YARN_OPTS="--frozen-lockfile"
fi
NPM_CMD="ci"
else
YARN_OPTS=""
NPM_CMD="install"
fi
# Execute install with corepack, return true to ignore errors from non matching package managers
Copy link
Contributor

Choose a reason for hiding this comment

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

Any way we could allow users to provide args/flags for these 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.

Do you mean to corepack or the package manager? Or both.

Copy link
Contributor

Choose a reason for hiding this comment

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

The package manager commands. My concern is if a user wants to pass some args to their package manager of choice, how do we enable that?

Not sure if Docker's ENV would work to pass args down, or if there's another way.

e.g. pseudocode

corepack yarn install $OPTS || true
corepack pnpm install $OPTS || true
corepack npm ci $OPTS       || true

corepack yarn install || true
corepack pnpm install || true
corepack npm ci || true

if [[ -f "/home/node/src/yarn.lock" || -f "/home/node/src/.yarnrc.yml" || -f "/home/node/src/.yarnrc" ]]; then
if [[ -n $YARN_VERSION ]]; then
yarn set version $YARN_VERSION
fi
yarn install $YARN_OPTS
# Check if the installed tree is correct. Install all dependencies if not
yarn check --verify-tree || NODE_ENV=development yarn install
yarn cache clean
elif [[ -f "/home/node/src/pnpm-lock.yaml" ]]; then
pnpm i --prefer-frozen-lockfile --prod
elif [[ -f "/home/node/src/package-lock.json" || -f "/home/node/src/npm-shrinkwrap.json" ]]; then
npm $NPM_CMD
npm cache clean --force
else
npm install
npm cache clean --force
fi