Conversation
|
Review requested:
|
|
This change would be highly desirable from the point of view of the @nodejs/docker team as The mismatch between "official" status on Docker hub and "Experimental" status in Node.js was raised in 2023 in nodejs/docker-node#2011 by @BethGriggs and remains to this day unresolved.
|
|
What would be the semverness of such change? |
|
I assume that it would be
semver-minor
|
|
It's not a functional change but an interesting question. I'd say +1 to what Mike said - probably considered semver-minor - even though there are no code changes it's similar to new functionality being added which doesn't affect anything else. I certainly don't see any reason for it to be considered semver-major, and I am anticipating here that it would apply to all current release lines in order to resolve the complications on the docker-node side. |
|
Adding a label for consistency with other build platforms. |
|
For anyone approving this it would be good to have your view on the lifecycle/Alpine version topic from #62764 (comment) - the TL;DR is: should we update the Alpine build level of the release machines (presumably increasing the musl version requirement) during the lifecycle of a node release? |
There was a problem hiding this comment.
the TL;DR is: should we update the Alpine build level of the release machines (presumably increasing the musl version requirement) during the lifecycle of a node release?
Given that the actual "versions" column of the Alpine support does not specify the Alpine version, other the kernel and musl version, I'd say "yes".
In addition, there is a note about the platform version support:
Lines 103 to 105 in bfb2fa7
Having thought about it for the last few days I was starting to lean the other way :-) We could take an approach of "bouncing along the floor" of the Alpine versions, updating by one each time a version goes out. I don't think we have any other platforms where a release has the potential to actively stop working in a customer's environment during a Node lifecycle (as it presumably would if they stayed on the out of support version when we bumped it up in the release CI assuming it would require at least the same version of musl at runtime vs build time which is generally the case for glibc) I'm also in the Build WG so we'd have to keep on top of this in order to make it work so that may be influencing my thoughts on this a little ... I'd say I'm about 60:40 towards keeping it constant at the moment but more input is welcome! |
|
Maybe it should be a different PR, but should the other architectures be added, but remain as "Experimental in the table https://github.com/nodejs/docker-node/blob/3b323804ad9882860ef2b14a36ccda1c65886a8b/versions.json
|
According to my comment in #62764 (comment) the other architectures aren't tested, so any extension to architectures in addition to This is in line with https://github.com/nodejs/docker-node#musl-builds-for-alpine which currently states: musl builds for Alpine
That section would need to be updated for As it stands at the moment, the statement " |
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
|
Would this be ready to merge after resolving conflicts? Node.js 22, 24 & 26 have now all had their run to produce official musl builds: https://nodejs.org/download/release/latest-v22.x/node-v22.23.3-linux-x64-musl.tar.gz |
|
Rebased after the length of the table was changed in https://github.com/nodejs/node/pull/65427/changes |
MikeMcC399
left a comment
There was a problem hiding this comment.
LGTM and thanks for all the effort to make this happen!
It clears the ground to resolve the long-standing issue of x64 Node.js Alpine Linux Docker images being experimental, although they're published to https://hub.docker.com/_/node as "official images".
This comment was marked as resolved.
This comment was marked as resolved.
|
Updated to 3.22 and removed the extra experimental line |
|
Should the PR have the
semver-minor
Also, what about
notable-change
|
Commit Queue failedThe pull request was removed from the Commit Queue and labeled
commit-queue-failed
Full Commit Queue output |
|
@sxa You've got merge commits in the branch. You'll need to manually squash and rebase to get rid of them. |
|
Added |
Signed-off-by: Stewart X Addison <sxa@ibm.com>
Commit Queue failedThe pull request was removed from the Commit Queue and labeled
commit-queue-failed
Full Commit Queue output |
Signed-off-by: Stewart X Addison <sxa@ibm.com> PR-URL: #63737 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Ruy Adorno <ruy@vlt.sh> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Richard Lau <richard.lau@ibm.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
|
Landed in bcf5850 |
As per #62764
FYI @nodejs/tsc - I've listed the version associated with the versions of Alpine we currently build against.