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

ci: add architectures update script #1341

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ttshivers
Copy link
Member

This adds an action that checks whether the architectures files are outdated and will open a PR (TODO) with the changes.
I saw this might be something potentially helpful when it was mentioned in #1312 by @nschonni .

At the moment, this grabs the available arches using bashbrew from https://github.com/docker-library/official-images for the base images. It then compares those arches to the ones stored in the */architectures file and modifies if they don't match.

TODO: Incorporate supported nodejs architectures

The script now just uses all the arches that the base image supports and nodejs might not support all of them. I haven't found a good programmatic way to get what arches nodejs supports. Some ways or ideas that I had include looking at the official dist release files, however it doesn't always have all the arches that this repo supports currently. I am interested in recommendations about the best way to do this.

Architectures file format

In this script, I used an object relating variant -> arches, while the architectures file stores data in a way that is equivalent to arch -> variants. I end up having to invert the structure to do comparisons. It seems more natural to store this data in a format like variant -> arches, since that's how it's also eventually translated to for the library files and how I get the data from bashbrew (it gives me a list of arches for a given image).
(Potentially even store it as a json file for easy nodejs script integration)

@ttshivers ttshivers force-pushed the arch_update branch 2 times, most recently from 3f78a1d to 65ed4c7 Compare September 30, 2020 02:37
@nschonni
Copy link
Member

diff --git a/14/architectures b/14/architectures
index b9eda44..9e87b80 100644
--- a/14/architectures
+++ b/14/architectures
@@ -1,8 +1,10 @@
-bashbrew-arch   variants
-amd64    stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-arm32v6  alpine3.10,alpine3.11,alpine3.12
-arm32v7  stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-arm64v8  stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-i386     alpine3.10,alpine3.11,alpine3.12
-ppc64le  buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-s390x    buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
+bashbrew-arch  variants
+arm32v6        alpine3.10,alpine3.11,alpine3.12
+mips64le       buster,buster-slim
+ppc64le        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim
+s390x          alpine3.10,alpine3.11,alpine3.12,buster,buster-slim
+amd64          alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+arm32v5        buster,buster-slim,stretch,stretch-slim
+arm32v7        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+arm64v8        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+i386           alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim

Think the arch value might need to be sorted too

@nschonni
Copy link
Member

Might help to "cheat" and fix the indenting/alignment in the architecture files, so you get a cleaner diff in the same diffs

@nschonni
Copy link
Member

Ah, I guess the initial one is going to create a diff no matter what since it also fixes the sorting of the image names.

diff --git a/14/architectures b/14/architectures
index b9eda44..bb656d5 100644
--- a/14/architectures
+++ b/14/architectures
@@ -1,8 +1,10 @@
-bashbrew-arch   variants
-amd64    stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-arm32v6  alpine3.10,alpine3.11,alpine3.12
-arm32v7  stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-arm64v8  stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-i386     alpine3.10,alpine3.11,alpine3.12
-ppc64le  buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-s390x    buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
+bashbrew-arch  variants
+amd64          alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+arm32v5        buster,buster-slim,stretch,stretch-slim
+arm32v6        alpine3.10,alpine3.11,alpine3.12
+arm32v7        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+arm64v8        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+i386           alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+mips64le       buster,buster-slim
+ppc64le        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim
+s390x          alpine3.10,alpine3.11,alpine3.12,buster,buster-slim

I guess there is a question on whether to include armv5 and mips64le

@ttshivers
Copy link
Member Author

Ah, I guess the initial one is going to create a diff no matter what since it also fixes the sorting of the image names.

diff --git a/14/architectures b/14/architectures
index b9eda44..bb656d5 100644
--- a/14/architectures
+++ b/14/architectures
@@ -1,8 +1,10 @@
-bashbrew-arch   variants
-amd64    stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-arm32v6  alpine3.10,alpine3.11,alpine3.12
-arm32v7  stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-arm64v8  stretch,stretch-slim,buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-i386     alpine3.10,alpine3.11,alpine3.12
-ppc64le  buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
-s390x    buster,buster-slim,alpine3.10,alpine3.11,alpine3.12
+bashbrew-arch  variants
+amd64          alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+arm32v5        buster,buster-slim,stretch,stretch-slim
+arm32v6        alpine3.10,alpine3.11,alpine3.12
+arm32v7        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+arm64v8        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+i386           alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+mips64le       buster,buster-slim
+ppc64le        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim
+s390x          alpine3.10,alpine3.11,alpine3.12,buster,buster-slim

I guess there is a question on whether to include armv5 and mips64le

I'll align and sort the existing ones to make it easier

Copy link
Member

@nschonni nschonni left a comment

Choose a reason for hiding this comment

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

Overall I like this. Probably need to figure out how to things might be filtered. EX: no support on 386 Linux or maybe armv5/mips

14/architectures Outdated Show resolved Hide resolved
updateArches.js Show resolved Hide resolved
@ttshivers ttshivers force-pushed the arch_update branch 2 times, most recently from 3d3f3ae to 973dcdb Compare September 30, 2020 03:10
@ttshivers
Copy link
Member Author

ttshivers commented Sep 30, 2020

Overall I like this. Probably need to figure out how to things might be filtered. EX: no support on 386 Linux or maybe armv5/mips

Yeah, that is the one thing that needs to be done. Either a manual allow list or block list or some way of handling that. Or possibly coding in a table or json file of supported nodejs version and arches and taking the intersection of those arches and the base image's arches.

@nschonni
Copy link
Member

Just pulling out the diff

diff --git a/10/architectures b/10/architectures
index d31c0c2..6753515 100644
--- a/10/architectures
+++ b/10/architectures
@@ -1,8 +1,10 @@
 bashbrew-arch  variants
 amd64          alpine3.10,alpine3.11,alpine3.9,buster,buster-slim,stretch,stretch-slim
+arm32v5        buster,buster-slim,stretch,stretch-slim
 arm32v6        alpine3.10,alpine3.11,alpine3.9
 arm32v7        alpine3.10,alpine3.11,alpine3.9,buster,buster-slim,stretch,stretch-slim
 arm64v8        alpine3.10,alpine3.11,alpine3.9,buster,buster-slim,stretch,stretch-slim
-i386           alpine3.10,alpine3.11,alpine3.9
+i386           alpine3.10,alpine3.11,alpine3.9,buster,buster-slim,stretch,stretch-slim
+mips64le       buster,buster-slim
 ppc64le        alpine3.10,alpine3.11,alpine3.9,buster,buster-slim
 s390x          alpine3.10,alpine3.11,alpine3.9,buster,buster-slim
diff --git a/12/architectures b/12/architectures
index 2d5e52a..a3170ba 100644
--- a/12/architectures
+++ b/12/architectures
@@ -1,8 +1,10 @@
 bashbrew-arch  variants
 amd64          alpine3.10,alpine3.11,alpine3.12,alpine3.9,buster,buster-slim,stretch,stretch-slim
+arm32v5        buster,buster-slim,stretch,stretch-slim
 arm32v6        alpine3.10,alpine3.11,alpine3.12,alpine3.9
 arm32v7        alpine3.10,alpine3.11,alpine3.12,alpine3.9,buster,buster-slim,stretch,stretch-slim
 arm64v8        alpine3.10,alpine3.11,alpine3.12,alpine3.9,buster,buster-slim,stretch,stretch-slim
-i386           alpine3.10,alpine3.11,alpine3.12,alpine3.9
+i386           alpine3.10,alpine3.11,alpine3.12,alpine3.9,buster,buster-slim,stretch,stretch-slim
+mips64le       buster,buster-slim
 ppc64le        alpine3.10,alpine3.11,alpine3.12,alpine3.9,buster,buster-slim
 s390x          alpine3.10,alpine3.11,alpine3.12,alpine3.9,buster,buster-slim
diff --git a/14/architectures b/14/architectures
index 09ace84..21eaa5e 100644
--- a/14/architectures
+++ b/14/architectures
@@ -1,8 +1,10 @@
 bashbrew-arch  variants
 amd64          alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+arm32v5        buster,buster-slim,stretch,stretch-slim
 arm32v6        alpine3.10,alpine3.11,alpine3.12
 arm32v7        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
 arm64v8        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
-i386           alpine3.10,alpine3.11,alpine3.12
+i386           alpine3.10,alpine3.11,alpine3.12,buster,buster-slim,stretch,stretch-slim
+mips64le       buster,buster-slim
 ppc64le        alpine3.10,alpine3.11,alpine3.12,buster,buster-slim
 s390x          alpine3.10,alpine3.11,alpine3.12,buster,buster-slim

@ttshivers
Copy link
Member Author

One reference for supported platforms is: https://github.com/nodejs/node/blob/master/BUILDING.md#platform-list

@nschonni
Copy link
Member

nschonni commented Oct 3, 2020

Using the Tier 1/Tier 2/Experimental on the multi-arch setup would be an interesting approach. Could mark the Tier 1 and maybe Tier 2 as required passing checks, but allow failures in the experimental

@ttshivers
Copy link
Member Author

Using the Tier 1/Tier 2/Experimental on the multi-arch setup would be an interesting approach. Could mark the Tier 1 and maybe Tier 2 as required passing checks, but allow failures in the experimental

Should those tiers play any role in how this action chooses variants or arches?

@nschonni
Copy link
Member

nschonni commented Oct 6, 2020

Would probably need to go back through the issues in the build issue to get the exact platform reqs for the older releases, but here would be an examples for 14:

  • Tier 1:
    • Debian x64
    • Debian arm64
    • Debian armv7
  • Tier 2:
    • Debian s390x
    • Debian ppc64le
  • Experimental
    • Debian armv6
    • Debian x86
    • Alpine x64

@ttshivers
Copy link
Member Author

How should we store those tiers? In a json file somewhere or some other format?

@SimenB
Copy link
Member

SimenB commented Oct 9, 2020

How should we store those tiers? In a json file somewhere or some other format?

It probably makes sense for the build group (or something) to store it in some machine readable format for us to consume. If we pull it into this repo it's bound to become outdated

Base automatically changed from master to main March 15, 2021 16:23
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.

3 participants