Added support for TencentOS and OpenCloudOS#1926
Added support for TencentOS and OpenCloudOS#1926shouhuanxiaoji wants to merge 3 commits intonodesource:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR broadens RPM-based system detection across multiple scripts to also recognize TencentOS and OpenCloudOS via case-insensitive checks of /etc/system-release, and adds Tencent/OpenCloud images to the RPM CI matrix; a minor deprecation message formatting tweak was made in one setup script. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/rpm/setup_25.x (1)
1-40: Consider consolidating shared logic into a reusable helper.The RPM detection block, logger function, error handler, and command_exists function are duplicated across all
setup_*.xscripts. As new distributions like TencentOS/OpenCloudOS are added, each script must be updated individually (as this PR demonstrates with 8+ files).Consider extracting these common functions into a shared sourced script (e.g.,
common.sh) or using a template generator to reduce maintenance burden and ensure consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/rpm/setup_25.x` around lines 1 - 40, Extract the duplicated helpers (log, handle_error, command_exists) and the RPM detection block into a shared script (e.g., common.sh) and have each setup_*.x script source that file at the top; specifically move the functions log, handle_error, command_exists and the RPM detection if-block into common.sh, export any needed environment/config variables (or return non-zero on unsupported OS) and replace the in-file copies with a single "source common.sh" line, then remove the duplicated blocks from setup_25.x and the other setup_*.x scripts so future distro additions (TencentOS/OpenCloudOS) only need to update common.sh.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/rpm/script_generator/base_script.sh`:
- Around line 35-38: The conditional guard beginning with "if ! [ -f
/etc/redhat-release ] && ! grep -q \"Amazon Linux\" /etc/system-release ..."
misses fallbacks that check /etc/os-release for TencentOS/OpenCloudOS; update
that if-condition to also grep /etc/os-release (e.g., add "&& ! grep -qi
\"TencentOS\" /etc/os-release 2>/dev/null && ! grep -qi \"OpenCloudOS\"
/etc/os-release 2>/dev/null") so the template recognizes those distros when only
/etc/os-release is present, then regenerate the derived setup_*.x files from the
generator so the change propagates.
In `@scripts/rpm/setup_16.x`:
- Around line 107-110: The guard in the if condition that checks for non-RPM
systems only inspects /etc/system-release and /etc/redhat-release but misses
distributions identified in /etc/os-release; update the conditional around the
multi-line if (the block starting with "if ! [ -f /etc/redhat-release ] \") to
also check /etc/os-release for the same strings ("Amazon Linux", "TencentOS",
"OpenCloudOS") using grep -q /etc/os-release 2>/dev/null (case-insensitively
where used) so machines identified via /etc/os-release are correctly detected
and the RPM-system check behaves the same as newer scripts.
- Around line 69-73: The release-status lines in scripts/rpm/setup_16.x are
stale (the URL/status labels for setup_18.x, setup_20.x, setup_21.x, etc.);
update each URL line (e.g., the literal lines containing
"https://rpm.nodesource.com/setup_16.x",
"https://rpm.nodesource.com/setup_18.x",
"https://rpm.nodesource.com/setup_19.x",
"https://rpm.nodesource.com/setup_20.x",
"https://rpm.nodesource.com/setup_21.x") to reflect the current Node.js release
matrix as of 2026-03-09: mark 18.x and 21.x as EOL, mark 20.x and 22.x as
Maintenance LTS, add or mark 24.x as Active LTS and 25.x as Current, and adjust
colors/annotations (red/green/bold) accordingly so the recommended installer
points to the current LTS (24.x) or Current (25.x) as intended.
---
Nitpick comments:
In `@scripts/rpm/setup_25.x`:
- Around line 1-40: Extract the duplicated helpers (log, handle_error,
command_exists) and the RPM detection block into a shared script (e.g.,
common.sh) and have each setup_*.x script source that file at the top;
specifically move the functions log, handle_error, command_exists and the RPM
detection if-block into common.sh, export any needed environment/config
variables (or return non-zero on unsupported OS) and replace the in-file copies
with a single "source common.sh" line, then remove the duplicated blocks from
setup_25.x and the other setup_*.x scripts so future distro additions
(TencentOS/OpenCloudOS) only need to update common.sh.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f8e8f6f-9477-4b25-98f5-d76e085486a9
📒 Files selected for processing (11)
scripts/rpm/script_generator/base_script.shscripts/rpm/setup_16.xscripts/rpm/setup_18.xscripts/rpm/setup_20.xscripts/rpm/setup_21.xscripts/rpm/setup_22.xscripts/rpm/setup_23.xscripts/rpm/setup_24.xscripts/rpm/setup_25.xscripts/rpm/setup_current.xscripts/rpm/setup_lts.x
| 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 |
There was a problem hiding this comment.
Add the /os-release fallback in the template guard.
This still only checks /etc/system-release, so TencentOS/OpenCloudOS hosts that expose the distro marker only via /os-release will keep tripping the unsupported-system path. Since scripts/rpm/script_generator/generator.sh:4-18 emits the versioned setup_*.x files from this template, the fix needs to land here and then be regenerated across the generated scripts.
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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/rpm/script_generator/base_script.sh` around lines 35 - 38, The
conditional guard beginning with "if ! [ -f /etc/redhat-release ] && ! grep -q
\"Amazon Linux\" /etc/system-release ..." misses fallbacks that check
/etc/os-release for TencentOS/OpenCloudOS; update that if-condition to also grep
/etc/os-release (e.g., add "&& ! grep -qi \"TencentOS\" /etc/os-release
2>/dev/null && ! grep -qi \"OpenCloudOS\" /etc/os-release 2>/dev/null") so the
template recognizes those distros when only /etc/os-release is present, then
regenerate the derived setup_*.x files from the generator so the change
propagates.
There was a problem hiding this comment.
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.
@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-release fallback 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
Learnt from: shouhuanxiaoji
Repo: nodesource/distributions PR: 1926
File: scripts/rpm/script_generator/base_script.sh:35-38
Timestamp: 2026-03-09T13:32:24.267Z
Learning: In the nodesource/distributions repository, TencentOS has /etc/system-release (and /os-release at root, but NOT /etc/os-release). The RPM-based detection scripts (scripts/rpm/script_generator/base_script.sh and generated setup_*.x files) only check /etc/system-release for OS identification (consistent with Amazon Linux detection), which is sufficient for TencentOS and OpenCloudOS since both provide /etc/system-release.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| * ${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} |
There was a problem hiding this comment.
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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * ${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} | |
| * ${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} | |
| * ${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} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/rpm/setup_16.x` around lines 69 - 73, The release-status lines in
scripts/rpm/setup_16.x are stale (the URL/status labels for setup_18.x,
setup_20.x, setup_21.x, etc.); update each URL line (e.g., the literal lines
containing "https://rpm.nodesource.com/setup_16.x",
"https://rpm.nodesource.com/setup_18.x",
"https://rpm.nodesource.com/setup_19.x",
"https://rpm.nodesource.com/setup_20.x",
"https://rpm.nodesource.com/setup_21.x") to reflect the current Node.js release
matrix as of 2026-03-09: mark 18.x and 21.x as EOL, mark 20.x and 22.x as
Maintenance LTS, add or mark 24.x as Active LTS and 25.x as Current, and adjust
colors/annotations (red/green/bold) accordingly so the recommended installer
points to the current LTS (24.x) or Current (25.x) as intended.
There was a problem hiding this comment.
This modification only changes non-ASCII strings, will not effect users
There was a problem hiding this comment.
@shouhuanxiaoji, you're correct — the change in this diff is purely cosmetic, replacing non-ASCII em dashes (—) with standard ASCII hyphens (-), and that won't affect users at runtime.
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
Learnt from: alexio-dev
Repo: nodesource/distributions PR: 1873
File: README.md:265-279
Timestamp: 2025-07-15T16:29:59.102Z
Learning: DEB setup scripts incorrectly configure N|Solid preferences for all Node.js versions including non-LTS versions (21.x, 23.x, 24.x), while RPM scripts correctly only configure N|Solid for LTS versions (18.x, 20.x, 22.x). This inconsistency should be resolved by updating DEB scripts to match RPM behavior.
Learnt from: riosje
Repo: nodesource/distributions PR: 1862
File: scripts/rpm/setup_24.x:68-83
Timestamp: 2025-05-07T13:18:52.097Z
Learning: The N|Solid repository should only be configured for LTS versions of Node.js (currently 18.x, 20.x, and 22.x). Non-LTS versions like 24.x should not have the N|Solid repository configured.
Learnt from: shouhuanxiaoji
Repo: nodesource/distributions PR: 1926
File: scripts/rpm/script_generator/base_script.sh:35-38
Timestamp: 2026-03-09T13:32:24.267Z
Learning: In the nodesource/distributions repository, TencentOS has /etc/system-release (and /os-release at root, but NOT /etc/os-release). The RPM-based detection scripts (scripts/rpm/script_generator/base_script.sh and generated setup_*.x files) only check /etc/system-release for OS identification (consistent with Amazon Linux detection), which is sufficient for TencentOS and OpenCloudOS since both provide /etc/system-release.
| 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 |
There was a problem hiding this comment.
Check /os-release here too.
This guard has the same gap as the newer scripts: it only looks for TencentOS/OpenCloudOS in /etc/system-release, so machines identified via /os-release still fail the RPM-system check.
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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/rpm/setup_16.x` around lines 107 - 110, The guard in the if condition
that checks for non-RPM systems only inspects /etc/system-release and
/etc/redhat-release but misses distributions identified in /etc/os-release;
update the conditional around the multi-line if (the block starting with "if ! [
-f /etc/redhat-release ] \") to also check /etc/os-release for the same strings
("Amazon Linux", "TencentOS", "OpenCloudOS") using grep -q /etc/os-release
2>/dev/null (case-insensitively where used) so machines identified via
/etc/os-release are correctly detected and the RPM-system check behaves the same
as newer scripts.
There was a problem hiding this comment.
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.
@shouhuanxiaoji, that's a fair point — the existing pattern consistently uses /etc/system-release for Amazon Linux detection, and since TencentOS and OpenCloudOS both provide /etc/system-release, applying the same check there is the correct and consistent approach. My suggestion to also check /os-release was misaligned with this established convention. Apologies for the noise!
🧠 Learnings used
Learnt from: shouhuanxiaoji
Repo: nodesource/distributions PR: 1926
File: scripts/rpm/script_generator/base_script.sh:35-38
Timestamp: 2026-03-09T13:32:24.267Z
Learning: In the nodesource/distributions repository, TencentOS has /etc/system-release (and /os-release at root, but NOT /etc/os-release). The RPM-based detection scripts (scripts/rpm/script_generator/base_script.sh and generated setup_*.x files) only check /etc/system-release for OS identification (consistent with Amazon Linux detection), which is sufficient for TencentOS and OpenCloudOS since both provide /etc/system-release.
Learnt from: alexio-dev
Repo: nodesource/distributions PR: 1873
File: README.md:265-279
Timestamp: 2025-07-15T16:29:59.102Z
Learning: DEB setup scripts incorrectly configure N|Solid preferences for all Node.js versions including non-LTS versions (21.x, 23.x, 24.x), while RPM scripts correctly only configure N|Solid for LTS versions (18.x, 20.x, 22.x). This inconsistency should be resolved by updating DEB scripts to match RPM behavior.
|
Hi @shouhuanxiaoji , This PR is looking good, thanks for the interest to contribute on the project. |
I have tried to add support for OS in the CI code, but opencloudos only has minimal support |
|
the same issue on OpenCloudOS. The installation script fails to detect it as an RPM-based system. This PR fixes it for me. Would love to see this merged？ |
4 participants
OpenCloudOS is a derivative distribution of TencentOS, so the description in this article applies to both distributions.
TencentOS use rpm as package management, but it has not /etc/os-release, only has /etc/system-release and /os-release, so added a patch to support.
Summary by CodeRabbit
New Features
Documentation
Chores