Added Fedora compatibility#1310
Conversation
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
WalkthroughThis PR extends the project's setup automation and documentation to support Fedora/RedHat-based Linux distributions alongside existing Windows and Debian-based support. Detection logic identifies the distribution, distro-specific package managers install dependencies, and service setup was refactored to use absolute paths from a calculated repository root. ChangesFedora/RedHat Linux Distribution Support
Sequence Diagram(s)sequenceDiagram
participant setup_js as scripts/setup.js
participant os as Detected_Distro
participant bash as scripts/setup.sh
setup_js->>os: check /etc/debian_version,/etc/fedora-release,/etc/redhat-release
os->>setup_js: distro identified (debian|fedora|redhat) or unsupported
setup_js->>bash: invoke bash setup (when supported)
bash->>os: use apt/dnf branch to install dependencies
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
docs/Script_Setup_Guide.md (1)
7-8: 💤 Low valueConsider adding a Fedora setup video guide.
The documentation now officially supports Fedora/RedHat-based Linux, but there's no corresponding video guide (only Windows and Ubuntu videos are listed). Consider adding a Fedora setup video to maintain consistency and help users on that platform.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/Script_Setup_Guide.md` around lines 7 - 8, Add a Fedora/RedHat video link line to the same bullet list where the existing "[Windows]" and "[Ubuntu (Debian)]" entries appear in the docs (so users on Fedora have parity with other platforms); insert a new list item like "[Fedora (RedHat)](link-to-video)" matching the existing format and ordering, ensuring the text label and URL follow the style used by the "[Windows]" and "[Ubuntu (Debian)]" entries.scripts/setup.sh (1)
94-101: 💤 Low valueNote: Node.js versions may differ between Debian and Fedora.
The Debian path uses NodeSource to install Node.js 18.x, while Fedora installs from distribution repositories. This may result in different Node.js versions across platforms. If the project requires a specific Node.js version, consider documenting the minimum/maximum supported versions or using a version manager consistently across distributions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/setup.sh` around lines 94 - 101, The install paths in scripts/setup.sh (OS_TYPE check with the Debian branch using NodeSource setup_18.x and the Fedora branch using distro packages) can produce inconsistent Node.js versions; update the script so both branches install a consistent required Node version (e.g., use NodeSource or NodeSource-equivalent repo for Fedora or install and use a version manager like nvm/volta across all OSes) and/or add clear documentation of the supported Node.js range; specifically modify the Debian branch that calls setup_18.x and the Fedora/RedHat branch (the branches under the OS_TYPE conditional) to either invoke the same NodeSource installer or bootstrap nvm/volta and install the pinned version, and add a note in the script header indicating the chosen required Node.js version constraint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/Manual_Setup_Guide.md`:
- Around line 171-176: Update the section title string "Fedora:" to
"Fedora/RedHat:" for consistency with the rest of the docs; locate the block
that currently reads "Fedora:" above the bash snippet (the exact header string
"Fedora:") and change it to "Fedora/RedHat:" while leaving the install command
(sudo dnf install -y glib2-devel pkgconf-pkg-config) unchanged.
- Around line 155-159: The header "Fedora:" in the troubleshooting section is
inconsistent with other docs; change the header text to "Fedora/RedHat:" (or
"Fedora/RedHat-based:") so it matches terminology used in CONTRIBUTING.md,
Script_Setup_Guide.md, and setup.md; locate the header string "Fedora:" near the
glib2-devel mesa-libGL install snippet and update it accordingly.
In `@scripts/setup.sh`:
- Line 114: The script currently runs cd "$REPO_ROOT/backend" without checking
success; add guard/error handling around that change-directory step (the cd
"$REPO_ROOT/backend" invocation) so the script verifies the backend directory
exists and the cd succeeded and immediately exits with a clear error if not
(e.g., test -d "$REPO_ROOT/backend" or check the cd exit status and echo a
descriptive message and exit 1) to prevent subsequent commands from running in
the wrong location.
- Line 134: The script currently runs cd "$REPO_ROOT/frontend" without checking
for failure, so if the frontend dir is missing subsequent npm install runs in
the wrong place; update the block containing cd "$REPO_ROOT/frontend" to verify
the directory change (e.g., test the return status of cd or use a conditional
like cd ... || { echo "Failed to enter frontend directory"; exit 1; }) before
running npm install, mirroring the error handling pattern used for the src-tauri
block so the script exits with an error if the cd fails.
- Line 124: The script blindly runs cd "$REPO_ROOT/sync-microservice" which can
silently fail; update the setup.sh around the cd command to check its exit
status and handle errors: after attempting cd "$REPO_ROOT/sync-microservice"
verify the command succeeded (e.g., if the cd fails), emit a clear error message
and exit with a non-zero status so subsequent steps don't run in the wrong
directory; reference the existing cd "$REPO_ROOT/sync-microservice" invocation
when locating where to add this check and error handling.
---
Nitpick comments:
In `@docs/Script_Setup_Guide.md`:
- Around line 7-8: Add a Fedora/RedHat video link line to the same bullet list
where the existing "[Windows]" and "[Ubuntu (Debian)]" entries appear in the
docs (so users on Fedora have parity with other platforms); insert a new list
item like "[Fedora (RedHat)](link-to-video)" matching the existing format and
ordering, ensuring the text label and URL follow the style used by the
"[Windows]" and "[Ubuntu (Debian)]" entries.
In `@scripts/setup.sh`:
- Around line 94-101: The install paths in scripts/setup.sh (OS_TYPE check with
the Debian branch using NodeSource setup_18.x and the Fedora branch using distro
packages) can produce inconsistent Node.js versions; update the script so both
branches install a consistent required Node version (e.g., use NodeSource or
NodeSource-equivalent repo for Fedora or install and use a version manager like
nvm/volta across all OSes) and/or add clear documentation of the supported
Node.js range; specifically modify the Debian branch that calls setup_18.x and
the Fedora/RedHat branch (the branches under the OS_TYPE conditional) to either
invoke the same NodeSource installer or bootstrap nvm/volta and install the
pinned version, and add a note in the script header indicating the chosen
required Node.js version constraint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c287e1d-14fc-46e5-8ea7-a5d095dd44f8
📒 Files selected for processing (6)
CONTRIBUTING.mddocs/Manual_Setup_Guide.mddocs/Script_Setup_Guide.mddocs/setup.mdscripts/setup.jsscripts/setup.sh
|
LGTM @DhruvK278 can you also share a screen recording of a clean installation using the script for Fedora? |
I have updated the PR. |
| curl -fsSL https://deb.nodesource.com/setup_18.x | sudo -E bash - | ||
| sudo apt-get install -y nodejs | ||
| elif [ -f "/etc/fedora-release" ] || [ -f "/etc/redhat-release" ]; then | ||
| sudo dnf install -y nodejs npm |
There was a problem hiding this comment.
Debian strictly uses Node version 18
curl -fsSL https://deb.nodesource.com/setup_18.x on line 97.
I would recommend the same for Fedora as well, to maintain consistency across distributions.
Addressed Issues:
Fixes #1309
Previously, the automated
setup.shscript only supported Debian-based Linux distributions (like Ubuntu) and would exit with an "Unsupported OS" warning on Fedora. Additionally, manual configuration guides only listed Debian-specific package names. This PR introduces native Fedora support to the automated setup workflow and manual documentation.Key changes
setup.sh:Recording
Here is the recording of the setup
https://drive.google.com/file/d/1y8EW4RuzapZdQbPAOH6oh0LXLx8aBLEW/view?usp=sharing
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
Checklist
Summary by CodeRabbit
Documentation
Chores