Skip to content

feat: only include the architectures supported in version.json#2494

Open
nschonni wants to merge 6 commits into
nodejs:mainfrom
nschonni:image-conditions
Open

feat: only include the architectures supported in version.json#2494
nschonni wants to merge 6 commits into
nodejs:mainfrom
nschonni:image-conditions

Conversation

@nschonni

@nschonni nschonni commented May 9, 2026

Copy link
Copy Markdown
Member

Description

Motivation and Context

There is likely still some drift between the values in architectures and version.json, but move more towards only including architectures in the Dockerfile that have been marked supported.

Testing Details

Example Output(if appropriate)

Types of changes

  • Documentation
  • Version change (Update, remove or add more Node.js versions)
  • Variant change (Update, remove or add more variants, or versions of variants)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Other (none of the above)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • All new and existing tests passed.

@nschonni nschonni marked this pull request as draft May 9, 2026 22:48
@nschonni nschonni force-pushed the image-conditions branch from aded288 to 4598df7 Compare July 4, 2026 22:48
@nschonni nschonni marked this pull request as ready for review July 4, 2026 22:50
@nschonni

nschonni commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

Although there is still a Shellcheck issue, I'm marking this as "ready" for a review, because with the upcoming Alpine changes, it probably would be good to make sure that what is marked as supported at the core matches what we're building here.

@nschonni

nschonni commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

I left them out of the diff, but if this is going to land, I'd add the keychanges from #2551 and add them as a separate commit to do one big rebuild at the same time

@MikeMcC399 MikeMcC399 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As already noted there are several linting errors from ShellCheck that would need to be resolved.

@MikeMcC399

MikeMcC399 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

We don't have an agreed strategy for making changes like this that affect the Dockerfiles but in themselves don't require new Docker builds.

For the @sxa key, Stuart submitted PR #2551 to change the key only (no Dockerfiles). I then did a dummy PR with updated Dockerfiles to run CI tests, which I closed unmerged after the tests were successful.

If that sounds like a good general strategy, we could maybe formalize it separately. If applied here, it would mean submitting a separate PR that only updated the Dockerfile templates and update.sh without updating any Dockerfiles. This PR would remain as a test that doesn't get merged. It's more complicated that way, but avoids the churn of PR creation when not wanted.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants