Skip to content

Fix architectures lists for consistency and correctness - #1153

Merged
LaurentGoderre merged 1 commit into
nodejs:masterfrom
tianon:arches
Nov 18, 2019
Merged

Fix architectures lists for consistency and correctness#1153
LaurentGoderre merged 1 commit into
nodejs:masterfrom
tianon:arches

Conversation

@tianon

@tianontianon commented Nov 14, 2019

Copy link
Copy Markdown
Contributor

This addresses the failure from docker-library/official-images#6972 (comment).

Diff:
$ diff -u <(bashbrew cat node) <(bashbrew cat <(./generate-stackbrew-library.sh))
--- /dev/fd/63	2019-11-14 13:22:15.381053187 -0800+++ /dev/fd/62	2019-11-14 13:22:15.381053187 -0800@@ -11,16 +11,6 @@
GitCommit: fa27514a3fd775e1cb6bddac326f6f97dad05fb2
Directory: 8/jessie-slim
-Tags: 8.16.2-alpine, 8.16-alpine, 8-alpine, carbon-alpine-Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x-GitCommit: 93c5e098567620200e5a374622a86b4aff153506-Directory: 8/alpine--Tags: 8.16.2-onbuild, 8.16-onbuild, 8-onbuild, carbon-onbuild-Architectures: amd64, arm32v7, arm64v8, i386, ppc64le, s390x-GitCommit: fa27514a3fd775e1cb6bddac326f6f97dad05fb2-Directory: 8/onbuild-
Tags: 8.16.2-stretch, 8.16-stretch, 8-stretch, carbon-stretch, 8.16.2, 8.16, 8, carbon
Architectures: amd64, arm32v7, arm64v8, i386, ppc64le, s390x
GitCommit: fa27514a3fd775e1cb6bddac326f6f97dad05fb2
@@ -41,10 +31,20 @@
GitCommit: fa27514a3fd775e1cb6bddac326f6f97dad05fb2
Directory: 8/buster-slim
-Tags: 13.1.0-alpine, 13.1-alpine, 13-alpine, alpine+Tags: 8.16.2-alpine3.9, 8.16-alpine3.9, 8-alpine3.9, carbon-alpine3.9
Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x
-GitCommit: bf40ad9d1c903c9d0d7ad71f0c1d7aea92f2051a-Directory: 13/alpine+GitCommit: c6bc44e84afcdb81d9749b7b034c60e916a519ad+Directory: 8/alpine3.9++Tags: 8.16.2-alpine3.10, 8.16-alpine3.10, 8-alpine3.10, carbon-alpine3.10, 8.16.2-alpine, 8.16-alpine, 8-alpine, carbon-alpine+Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x+GitCommit: c6bc44e84afcdb81d9749b7b034c60e916a519ad+Directory: 8/alpine3.10++Tags: 8.16.2-onbuild, 8.16-onbuild, 8-onbuild, carbon-onbuild+Architectures: amd64, arm32v7, arm64v8, i386, ppc64le, s390x+GitCommit: fa27514a3fd775e1cb6bddac326f6f97dad05fb2+Directory: 8/onbuild
Tags: 13.1.0-stretch, 13.1-stretch, 13-stretch, stretch, 13.1.0, 13.1, 13, latest
Architectures: amd64, arm32v7, arm64v8, ppc64le, s390x
@@ -66,10 +66,10 @@
GitCommit: bf40ad9d1c903c9d0d7ad71f0c1d7aea92f2051a
Directory: 13/buster-slim
-Tags: 12.13.0-alpine, 12.13-alpine, 12-alpine, erbium-alpine, lts-alpine, current-alpine+Tags: 13.1.0-alpine3.10, 13.1-alpine3.10, 13-alpine3.10, alpine3.10, 13.1.0-alpine, 13.1-alpine, 13-alpine, alpine
Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x
-GitCommit: 93c5e098567620200e5a374622a86b4aff153506-Directory: 12/alpine+GitCommit: c6bc44e84afcdb81d9749b7b034c60e916a519ad+Directory: 13/alpine3.10
Tags: 12.13.0-stretch, 12.13-stretch, 12-stretch, erbium-stretch, lts-stretch, current-stretch, 12.13.0, 12.13, 12, erbium, lts, current
Architectures: amd64, arm32v7, arm64v8, ppc64le, s390x
@@ -91,6 +91,16 @@
GitCommit: c31a071c73c5cc40dc662b75a4ee9f9fc23d6a39
Directory: 12/buster-slim
+Tags: 12.13.0-alpine3.9, 12.13-alpine3.9, 12-alpine3.9, erbium-alpine3.9, lts-alpine3.9, current-alpine3.9+Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x+GitCommit: c6bc44e84afcdb81d9749b7b034c60e916a519ad+Directory: 12/alpine3.9++Tags: 12.13.0-alpine3.10, 12.13-alpine3.10, 12-alpine3.10, erbium-alpine3.10, lts-alpine3.10, current-alpine3.10, 12.13.0-alpine, 12.13-alpine, 12-alpine, erbium-alpine, lts-alpine, current-alpine+Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x+GitCommit: c6bc44e84afcdb81d9749b7b034c60e916a519ad+Directory: 12/alpine3.10+
Tags: 10.17.0-jessie, 10.17-jessie, 10-jessie, dubnium-jessie
Architectures: amd64, arm32v7
GitCommit: f5875531604b4b3b9fbc36437182781c3655c8ae
@@ -101,11 +111,6 @@
GitCommit: f5875531604b4b3b9fbc36437182781c3655c8ae
Directory: 10/jessie-slim
-Tags: 10.17.0-alpine, 10.17-alpine, 10-alpine, dubnium-alpine-Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x-GitCommit: 93c5e098567620200e5a374622a86b4aff153506-Directory: 10/alpine-
Tags: 10.17.0-stretch, 10.17-stretch, 10-stretch, dubnium-stretch, 10.17.0, 10.17, 10, dubnium
Architectures: amd64, arm32v7, arm64v8, ppc64le, s390x
GitCommit: f5875531604b4b3b9fbc36437182781c3655c8ae
@@ -126,10 +131,20 @@
GitCommit: f5875531604b4b3b9fbc36437182781c3655c8ae
Directory: 10/buster-slim
+Tags: 10.17.0-alpine3.9, 10.17-alpine3.9, 10-alpine3.9, dubnium-alpine3.9+Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x+GitCommit: c6bc44e84afcdb81d9749b7b034c60e916a519ad+Directory: 10/alpine3.9++Tags: 10.17.0-alpine3.10, 10.17-alpine3.10, 10-alpine3.10, dubnium-alpine3.10, 10.17.0-alpine, 10.17-alpine, 10-alpine, dubnium-alpine+Architectures: amd64, arm32v6, arm32v7, arm64v8, i386, ppc64le, s390x+GitCommit: c6bc44e84afcdb81d9749b7b034c60e916a519ad+Directory: 10/alpine3.10+
Tags: chakracore-8.11.1, chakracore-8.11, chakracore-8
-GitCommit: 2c75ae92f8257f7c6306a39f1e46303b29efce0d+GitCommit: 8ccd57c1457a1b47adc4d82f9fed9ad51ccef3c5
Directory: chakracore/8
Tags: chakracore-10.13.0, chakracore-10.13, chakracore-10, chakracore
-GitCommit: 2c75ae92f8257f7c6306a39f1e46303b29efce0d+GitCommit: 69c8a5f448f46f9e34d7fb577eca79ba01f6864d
Directory: chakracore/10

@tianontianon mentioned this pull request Nov 14, 2019
Closed
@SimenB

SimenB commented Nov 14, 2019

Copy link
Copy Markdown
Member

CI is unhappy - seems to be an unexpected diff or something?

@tianon

tianon commented Nov 14, 2019

Copy link
Copy Markdown
ContributorAuthor

Seems totally unrelated -- complaining that my .travis.yml doesn't match the template, which I'm guessing is somehow related to the changes in #1125 (so this test is probably failing on master also).

@tianon

tianon commented Nov 14, 2019

Copy link
Copy Markdown
ContributorAuthor

Ah, it's using architectures files to determine supported variants, and Node.js 8 onbuild disappeared and was correctly added back here.

Fix pushed.

nschonni
nschonni reviewed Nov 14, 2019
View reviewed changes
Comment threadarchitectures
i386 jessie,jessie-slim,stretch,stretch-slim,buster,buster-slim,alpine3.9,alpine3.10
ppc64le jessie,jessie-slim,stretch,stretch-slim,buster,buster-slim,alpine3.9,alpine3.10
s390x jessie,jessie-slim,stretch,stretch-slim,buster,buster-slim,alpine3.9,alpine3.10
amd64 jessie,jessie-slim,stretch,stretch-slim,buster,buster-slim,alpine3.9,alpine3.10,onbuild

@nschonninschonniNov 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

don't think "onbuild" should be added here since it's no longer supported

@tianontianonNov 14, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The issue is that node:8-onbuild still exists (and the directory still exists), so for that to continue to exist, it has to be listed here too:

$ bashbrew list node | grep onbuildnode:8.16.2-onbuildnode:8.16-onbuildnode:8-onbuildnode:carbon-onbuild

https://github.com/docker-library/official-images/blob/f396b68d8be74ecd83dac3d6aa6f18bdb7e575d8/library/node#L21-L24

@tianontianonNov 14, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

To be clear, I'm happy to remove onbuild, but that does mean this final onbuild image goes away too, and I'm guessing it was left in on purpose?

@nschonninschonniNov 14, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe I'm missing where it is being used, since "onbuild" is still in the 8/architectures, but maybe this is something that the stackbrew generation needs in both places

@tianontianonNov 14, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

#994 (comment)

So, this is where things got weird. What I found in testing was that the root file was somehow being used to determine the list of architecture+variant combinations that it was willing to try, so when I copied this file as-is to the root, all the jessie variants went away completely, which is why I've updated the root file here to simply list every possible variant for every supported architecture and opted instead to be explicit per-version which arch+variant combinations are supported.

@nschonninschonniNov 15, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks! i'll see if I can figure out if that file is removable separately

LaurentGoderre
LaurentGoderre reviewed Nov 15, 2019
View reviewed changes
Comment thread13/architectures
ppc64le alpine,stretch,stretch-slim,buster,buster-slim
s390x alpine,stretch,stretch-slim,buster,buster-slim
amd64 stretch,stretch-slim,buster,buster-slim,alpine3.10
arm32v6 alpine3.10

@LaurentGoderreLaurentGoderreNov 15, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching that @tianon! I missed it in the rebase!

@LaurentGoderre

LaurentGoderre commented Nov 15, 2019

Copy link
Copy Markdown
Member

We should merge this asap

@LaurentGoderre
LaurentGoderre merged commit 57911b4 into nodejs:masterNov 18, 2019
@tianon
tianon deleted the arches branch November 18, 2019 16:03
@nschonni

nschonni commented Nov 18, 2019

Copy link
Copy Markdown
Member

@LaurentGoderre this may be another edge case in the skipping deploy

docker-node/generate-stackbrew-library.sh

Lines 64 to 65 in dafa922

# Skip "docs" and other non-docker directories
[ -f"${version}/Dockerfile" ] || [ -f"${version}/${default_variant}/Dockerfile" ] ||continue
that needs a manual PR to the official images repo

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nschonninschonninschonni left review comments
@LaurentGoderreLaurentGoderreLaurentGoderre approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tianon@SimenB@LaurentGoderre@nschonni