-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Added support for TencentOS and OpenCloudOS #1926
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -66,11 +66,11 @@ print_bold \ | |||||||||||||||||||||||||
| Use the installation script that corresponds to the version of Node.js you | ||||||||||||||||||||||||||
| wish to install. e.g. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| * ${red}https://rpm.nodesource.com/setup_16.x — Node.js 16 \"Gallium\" ${bold}(deprecated)${normal} | ||||||||||||||||||||||||||
| * ${green}https://rpm.nodesource.com/setup_18.x — Node.js 18 \"Hydrogen\" (Maintenance)${normal} | ||||||||||||||||||||||||||
| * ${red}https://rpm.nodesource.com/setup_19.x — Node.js 19 \"Nineteen\" ${bold}(deprecated)${normal} | ||||||||||||||||||||||||||
| * ${bold}${green}https://rpm.nodesource.com/setup_20.x — Node.js 20 LTS \"Iron\" (recommended)${normal} | ||||||||||||||||||||||||||
| * ${green}https://rpm.nodesource.com/setup_21.x — Node.js 21 \"Iron\" (current)${normal} | ||||||||||||||||||||||||||
| * ${red}https://rpm.nodesource.com/setup_16.x - Node.js 16 \"Gallium\" ${bold}(deprecated)${normal} | ||||||||||||||||||||||||||
| * ${green}https://rpm.nodesource.com/setup_18.x - Node.js 18 \"Hydrogen\" (Maintenance)${normal} | ||||||||||||||||||||||||||
| * ${red}https://rpm.nodesource.com/setup_19.x - Node.js 19 \"Nineteen\" ${bold}(deprecated)${normal} | ||||||||||||||||||||||||||
| * ${bold}${green}https://rpm.nodesource.com/setup_20.x - Node.js 20 LTS \"Iron\" (recommended)${normal} | ||||||||||||||||||||||||||
| * ${green}https://rpm.nodesource.com/setup_21.x - Node.js 21 \"Iron\" (current)${normal} | ||||||||||||||||||||||||||
|
Comment on lines
+69
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refresh the deprecation warning's release matrix. These labels are stale now: 18.x and 21.x are already end-of-life, 20.x and 22.x are Maintenance LTS, 24.x is Active LTS, and 25.x is Current as of March 9, 2026. The current banner can steer users toward unsupported or non-current installers. (github.com) Suggested patch- * ${green}https://rpm.nodesource.com/setup_18.x - Node.js 18 \"Hydrogen\" (Maintenance)${normal}
+ * ${red}https://rpm.nodesource.com/setup_18.x - Node.js 18 \"Hydrogen\" ${bold}(deprecated)${normal}
* ${red}https://rpm.nodesource.com/setup_19.x - Node.js 19 \"Nineteen\" ${bold}(deprecated)${normal}
- * ${bold}${green}https://rpm.nodesource.com/setup_20.x - Node.js 20 LTS \"Iron\" (recommended)${normal}
- * ${green}https://rpm.nodesource.com/setup_21.x - Node.js 21 \"Iron\" (current)${normal}
+ * ${green}https://rpm.nodesource.com/setup_20.x - Node.js 20 LTS \"Iron\" (Maintenance)${normal}
+ * ${green}https://rpm.nodesource.com/setup_22.x - Node.js 22 LTS \"Jod\" (Maintenance)${normal}
+ * ${bold}${green}https://rpm.nodesource.com/setup_24.x - Node.js 24 LTS \"Krypton\" (recommended)${normal}
+ * ${green}https://rpm.nodesource.com/setup_25.x - Node.js 25 (current)${normal}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This modification only changes non-ASCII strings, will not effect users There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My concern about the stale Node.js release matrix (e.g., 18.x listed as Maintenance, 20.x as recommended, 21.x as current) is a pre-existing issue that predates this PR and is separate from your change. Would you like me to open a follow-up GitHub issue to track updating the deprecation warning's version list to reflect the current release status? (ᓀ‸ᓂ) 🧠 Learnings used |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -104,7 +104,10 @@ command_exists() { | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # Check if we are on an RPM-based system | ||||||||||||||||||||||||||
| if ! [ -f /etc/redhat-release ] && ! grep -q "Amazon Linux" /etc/system-release 2>/dev/null; then | ||||||||||||||||||||||||||
| if ! [ -f /etc/redhat-release ] \ | ||||||||||||||||||||||||||
| && ! grep -q "Amazon Linux" /etc/system-release 2>/dev/null \ | ||||||||||||||||||||||||||
| && ! grep -qi "TencentOS" /etc/system-release 2>/dev/null \ | ||||||||||||||||||||||||||
| && ! grep -qi "OpenCloudOS" /etc/system-release 2>/dev/null; then | ||||||||||||||||||||||||||
|
Comment on lines
+107
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check This guard has the same gap as the newer scripts: it only looks for TencentOS/OpenCloudOS in Suggested patch-if ! [ -f /etc/redhat-release ] \
- && ! grep -q "Amazon Linux" /etc/system-release 2>/dev/null \
- && ! grep -qi "TencentOS" /etc/system-release 2>/dev/null \
- && ! grep -qi "OpenCloudOS" /etc/system-release 2>/dev/null; then
+if ! [ -f /etc/redhat-release ] \
+ && ! grep -q "Amazon Linux" /etc/system-release 2>/dev/null \
+ && ! grep -Eqi "TencentOS|OpenCloudOS" /etc/system-release /os-release 2>/dev/null; then
handle_error 1 "This script is intended for RPM-based systems. Please run it on an RPM-based system."
fi📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the original code only checks /etc/system-release for Amazon Linux, it does not add the callback's /etc/os-release check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🧠 Learnings used |
||||||||||||||||||||||||||
| handle_error 1 "This script is intended for RPM-based systems. Please run it on an RPM-based system." | ||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||
| node_deprecation_warning | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the
/os-releasefallback in the template guard.This still only checks
/etc/system-release, so TencentOS/OpenCloudOS hosts that expose the distro marker only via/os-releasewill keep tripping the unsupported-system path. Sincescripts/rpm/script_generator/generator.sh:4-18emits the versionedsetup_*.xfiles from this template, the fix needs to land here and then be regenerated across the generated scripts.Suggested patch
📝 Committable suggestion
🤖 Prompt for AI Agents
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the original code only checks /etc/system-release for Amazon Linux, it does not add the callback's /etc/os-release check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shouhuanxiaoji, that's a fair point. Since TencentOS does have/etc/system-release(as noted in the PR description), the existing grep check against that file will work correctly — no/os-releasefallback is needed. The consistency with how Amazon Linux is handled (only checking/etc/system-release) also makes the pattern uniform.I'll withdraw the suggestion.
✏️ Learnings added