github actions: Use Cargo.toml and rust-toolchain.toml#482
Conversation
Signed-off-by: mulhern <amulhern@redhat.com>
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
1b6ac1e to
0312e3b
Compare
5217bde to
92e3714
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR migrates the project's CI workflows from ChangesRust Toolchain Configuration & Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
♻️ Duplicate comments (1)
.github/workflows/main.yml (1)
227-239:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify toml-cli command syntax and handle potential quoted output.
The same MSRV detection concern from the
lowest_supportedjob applies here. Thetoml getcommand may return quoted strings that would break toolchain setup.🛡️ Potential fix to handle quoted output
- name: Get package MSRV id: msrv run: | - PATH="$PATH:$(python3 -m site --user-base)/bin" MSRV=$(toml get --toml-path Cargo.toml "package.rust-version") + PATH="$PATH:$(python3 -m site --user-base)/bin" MSRV=$(toml get --toml-path Cargo.toml "package.rust-version" | tr -d '"') echo "MSRV=$MSRV" >> "$GITHUB_OUTPUT"🤖 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 @.github/workflows/main.yml around lines 227 - 239, The MSRV step (id: msrv) uses `toml get` which can return a quoted string and break the toolchain setup; update the run block that sets MSRV so it normalizes/strips surrounding quotes (e.g., trim whitespace and remove leading/trailing double quotes) after invoking `toml get` and before exporting to GITHUB_OUTPUT, and keep using that cleaned variable when passing toolchain: ${{ steps.msrv.outputs.MSRV }} to actions-rust-lang/setup-rust-toolchain@v1.
🤖 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 @.github/workflows/main.yml:
- Line 226: The workflow step that runs the package install uses the wrong
Fedora package name ("dnf install -y git pip"); update that run command to
install python3-pip instead of pip so the correct Fedora package is used
(replace the occurrence of "pip" in the run command with "python3-pip").
---
Duplicate comments:
In @.github/workflows/main.yml:
- Around line 227-239: The MSRV step (id: msrv) uses `toml get` which can return
a quoted string and break the toolchain setup; update the run block that sets
MSRV so it normalizes/strips surrounding quotes (e.g., trim whitespace and
remove leading/trailing double quotes) after invoking `toml get` and before
exporting to GITHUB_OUTPUT, and keep using that cleaned variable when passing
toolchain: ${{ steps.msrv.outputs.MSRV }} to
actions-rust-lang/setup-rust-toolchain@v1.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f60e6a76-8799-47f7-a31d-252761846ede
📒 Files selected for processing (4)
.github/workflows/cargo.yml.github/workflows/main.yml.github/workflows/nightly.ymlrust-toolchain.toml
| steps: | ||
| - name: Install git | ||
| run: dnf install -y git | ||
| run: dnf install -y git pip |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the correct DNF package name for pip on Fedora 33, 34, and 38?
💡 Result:
The correct DNF package name for pip on Fedora 33, 34, and 38 is python3-pip [1][2][3]. You can install it using the following command: sudo dnf install python3-pip Once installed, it is standard practice on Fedora to use the pip3 command or the python3 -m pip module to interact with pip for Python 3 [4][3][5].
Citations:
- 1: https://linuxhint.com/install-python-package-installer-pip-fedora-38/
- 2: https://bugzilla.redhat.com/show_bug.cgi?id=1999035
- 3: https://idroot.us/install-python-pip-fedora-38/
- 4: https://superuser.com/questions/941418/run-pip-for-python3-on-fedora
- 5: https://www.atlantic.net/vps-hosting/how-to-install-and-use-pip-package-manager-on-fedora/
Incorrect package name for pip on Fedora.
The command dnf install -y git pip uses pip as the package name, but on Fedora the correct package is python3-pip.
🐛 Proposed fix
run: dnf install -y git pip
+ run: dnf install -y git python3-pip📝 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.
| run: dnf install -y git pip | |
| run: dnf install -y git python3-pip |
🤖 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 @.github/workflows/main.yml at line 226, The workflow step that runs the
package install uses the wrong Fedora package name ("dnf install -y git pip");
update that run command to install python3-pip instead of pip so the correct
Fedora package is used (replace the occurrence of "pip" in the run command with
"python3-pip").
92e3714 to
eb7af5f
Compare
Use the values specified in these files to set toolchain versions in CI. Signed-off-by: mulhern <amulhern@redhat.com>
eb7af5f to
e8fd0de
Compare
Related stratis-storage/project#819