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

Add buster :) #1055

Closed
wants to merge 1 commit into from
Closed

Add buster :) #1055

wants to merge 1 commit into from

Conversation

zerdos
Copy link

@zerdos zerdos commented Jul 12, 2019

No description provided.

@tvainika
Copy link
Contributor

tvainika commented Jul 13, 2019

This PR changes default Debian version of Node.js 8.x and Node.js 10.x images, which are under active LTS maintenance. Earlier when Debian 8 jessie LTS support ended, Node.js 8.x and Node.js 9.x images changed underlying operating system version causing issues #936, #937 and #940 opened as expectations were not met.

In my opinion both Node.js 8.x and Node.js 10.x default images should be kept aliases to whatever operating system version those were first released during LTS start. That is node:8 should equal to node:8-stretch (and node:8-slim = node:8-stretch-slim) until Node 8.x support ends. Same for node:10 and node:10-slim images.

Debian normal security support promises Debian 9 stretch support until June 2020, and Debian LTS security support up to June 2022 for Debian 9 stretch. Node.js 10.x LTS ends 2021-04-01 and Node.js 8.x LTS ends as soon as 2019-12-31.

However Node.js 12.x LTS ends later than Debian 9 stretch LTS, so it might be preferred solution to switch node:12 and node:12-slim images to use Debian 10 buster, and to do the switch before Node.js 12.x enters active LTS period on 2019-10-22.

Also it would be useful to write down the principles how the operating system version are selected and updated just like CONTRIBUTING.md describes update cycle for Node.js, NPM and Yarn.

This is just my 0.02 cents as I was bitten by tag change as I explained in my comment for #937.

@@ -1,4 +1,4 @@
baseuri https://nodejs.org/dist
default_variant stretch
default_variant buster
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with adding a buster variant, but please don't change the default version.

- update.sh . alpine # Update the alpine variant for all versions
- update.sh -t # Update .travis.yml only
- update.sh # Update all images
- update.sh -s # Update all images, skip updating Alpine and Yarn
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove these whitespace changes.

@@ -17,7 +17,7 @@ RUN ARCH= && dpkgArch="$(dpkg --print-architecture)" \
&& rm "node-v$NODE_VERSION-linux-$ARCH.tar.xz" SHASUMS256.txt \
&& ln -s /usr/local/bin/node /usr/local/bin/nodejs

ENV YARN_VERSION 1.6.0
ENV YARN_VERSION 1.16.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this change. This PR should only be about adding buster.

@@ -1,4 +1,4 @@
FROM buildpack-deps:jessie
FROM buildpack-deps:stretch
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this change, it doesn't' belong here.

@@ -17,7 +17,7 @@ RUN ARCH= && dpkgArch="$(dpkg --print-architecture)" \
&& rm "node-v$NODE_VERSION-linux-$ARCH.tar.xz" SHASUMS256.txt \
&& ln -s /usr/local/bin/node /usr/local/bin/nodejs

ENV YARN_VERSION 1.12.3
ENV YARN_VERSION 1.16.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this change please.

@@ -1,4 +1,4 @@
FROM node:8.16.0-stretch
FROM node:8.16.0-buster
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this change please. This PR should not be changing the default variant.

Also the onbuild variant is deprecated, so this change has little value anyway.

@@ -45,7 +45,7 @@ RUN buildDeps='xz-utils' \
&& apt-get purge -y --auto-remove $buildDeps \
&& ln -s /usr/local/bin/node /usr/local/bin/nodejs

ENV YARN_VERSION 1.15.2
ENV YARN_VERSION 1.16.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this change please.

@@ -42,7 +42,7 @@ RUN ARCH= && dpkgArch="$(dpkg --print-architecture)" \
&& rm "node-v$NODE_VERSION-linux-$ARCH.tar.xz" SHASUMS256.txt.asc SHASUMS256.txt \
&& ln -s /usr/local/bin/node /usr/local/bin/nodejs

ENV YARN_VERSION 1.15.2
ENV YARN_VERSION 1.16.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this change please

@@ -144,24 +144,17 @@ jobs:

- stage: Build
before_script: *auto_skip
name: 11 on alpine
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this. 11 should not be dropped from travis as part of this PR

@@ -144,24 +144,17 @@ jobs:

- stage: Build
before_script: *auto_skip
name: 11 on alpine
name: 10 on buster
Copy link
Contributor

Choose a reason for hiding this comment

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

These and the following changes to add the buster variant should also be in the travis.yml.template file which is the source template for the .travis.yml file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants